Ubuntu
systemd package

Merge ~enr0n/ubuntu/+source/systemd:ubuntu-jammy into ~ubuntu-core-dev/ubuntu/+source/systemd:ubuntu-jammy

  1. Git
  2. lp:~enr0n/ubuntu/+source/systemd
  3. ubuntu-jammy
  4. Merge into ubuntu-jammy
Proposed by Nick Rosbrook
Status: Merged
Merged at revision: 5a33378dde845d7b7716b90c94ed0e54a23968e8
Proposed branch: ~enr0n/ubuntu/+source/systemd:ubuntu-jammy
Merge into: ~ubuntu-core-dev/ubuntu/+source/systemd:ubuntu-jammy
Diff against target: 351 lines (+311/-0)
6 files modified
debian/changelog (+12/-0)
debian/patches/lp2002445/core-device-ignore-failed-uevents.patch (+51/-0)
debian/patches/lp2002445/sd-device-introduce-device_get_property_int.patch (+56/-0)
debian/patches/lp2002445/sd-device-make-device_set_syspath-clear-sysname-and-sysnu.patch (+29/-0)
debian/patches/lp2002445/udev-restore-syspath-and-properties-on-failure.patch (+159/-0)
debian/patches/series (+4/-0)
Reviewer Review Type Date Requested Status
Lukas Märdian Approve
Review via email: mp+439253@code.launchpad.net

Description of the change

The fix for LP: #2002445 is incomplete without these patches. In particular, udev needs to properly clean up modified device properties after failed renaming attempts. The full upstream PR is here: https://github.com/systemd/systemd/pull/25980.

Only the necessary commits from that PR are backported here.

PPA: https://launchpad.net/~enr0n/+archive/ubuntu/systemd-249
autopkgtest:

systemd 249.11-0ubuntu3.9~ppa1 (amd64) -- Fail: https://autopkgtest.ubuntu.com/results/autopkgtest-jammy-enr0n-systemd-249/jammy/amd64/s/systemd/20230318_022614_2a0e9@/log.gz
systemd 249.11-0ubuntu3.9~ppa1 (arm64) -- Pass: https://autopkgtest.ubuntu.com/results/autopkgtest-jammy-enr0n-systemd-249/jammy/arm64/s/systemd/20230317_220446_9d627@/log.gz
systemd 249.11-0ubuntu3.9~ppa1 (armhf) -- Pass: https://autopkgtest.ubuntu.com/results/autopkgtest-jammy-enr0n-systemd-249/jammy/armhf/s/systemd/20230317_205813_0b841@/log.gz
systemd 249.11-0ubuntu3.9~ppa1 (ppc64el) -- Fail: https://autopkgtest.ubuntu.com/results/autopkgtest-jammy-enr0n-systemd-249/jammy/ppc64el/s/systemd/20230317_211917_e3ab0@/log.gz
systemd 249.11-0ubuntu3.9~ppa1 (s390x) -- Fail: https://autopkgtest.ubuntu.com/results/autopkgtest-jammy-enr0n-systemd-249/jammy/s390x/s/systemd/20230317_215320_04b93@/log.gz

The s390x failure was a test bed failure. The ppc64el and amd64 failures are both familiar flaky tests.

To post a comment you must log in.
Revision history for this message
Lukas Märdian (slyon) wrote :

Thanks, refreshed/backported patches lgtm.

praise: I really like how you provide full context in the commit messages and merge-proposal (test-builds, autopkgtest logs + explanation). Kudos!

review: Approve
Revision history for this message
Lukas Märdian (slyon) wrote :

Sponsored as `debuild -S -d -v249.11-0ubuntu3.7`, to keep the previous 249.11-0ubuntu3.8 SRU changelog available.

https://launchpad.net/ubuntu/jammy/+queue?queue_state=1&queue_text=systemd

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/debian/changelog b/debian/changelog
index 238b809..ec1d8ab 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,15 @@
1systemd (249.11-0ubuntu3.9) jammy; urgency=medium
2
3 * udev: gracefully handle rename failures (LP: #2002445)
4 Files:
5 - debian/patches/lp2002445/core-device-ignore-failed-uevents.patch
6 - debian/patches/lp2002445/sd-device-introduce-device_get_property_int.patch
7 - debian/patches/lp2002445/sd-device-make-device_set_syspath-clear-sysname-and-sysnu.patch
8 - debian/patches/lp2002445/udev-restore-syspath-and-properties-on-failure.patch
9 https://git.launchpad.net/~ubuntu-core-dev/ubuntu/+source/systemd/commit/?id=a7ad4a9fc708500c61e3b8127f112d8c90049b2c
10
11 -- Nick Rosbrook <nick.rosbrook@canonical.com> Mon, 20 Mar 2023 10:32:08 -0400
12
1systemd (249.11-0ubuntu3.8) jammy; urgency=medium13systemd (249.11-0ubuntu3.8) jammy; urgency=medium
214
3 * network/dhcp4: accept local subnet routes from DHCP (LP: #2004478)15 * network/dhcp4: accept local subnet routes from DHCP (LP: #2004478)
diff --git a/debian/patches/lp2002445/core-device-ignore-failed-uevents.patch b/debian/patches/lp2002445/core-device-ignore-failed-uevents.patch
4new file mode 10064416new file mode 100644
index 0000000..1a2f9d1
--- /dev/null
+++ b/debian/patches/lp2002445/core-device-ignore-failed-uevents.patch
@@ -0,0 +1,51 @@
1From: Yu Watanabe <watanabe.yu+github@gmail.com>
2Date: Mon, 9 Jan 2023 16:11:52 +0900
3Subject: core/device: ignore failed uevents
4
5Origin: upstream, https://github.com/systemd/systemd/commit/e9336d6ac346df38d96c91ba0447b3c76ee6697b
6Bug-Ubuntu: https://launchpad.net/bugs/2002445
7
8When udevd failed to process the device, SYSTEMD_ALIAS or any other
9properties may contain invalid values. Let's refuse to handle the uevent.
10---
11 src/core/device.c | 20 ++++++++++++++++++++
12 1 file changed, 20 insertions(+)
13
14diff --git a/src/core/device.c b/src/core/device.c
15index 5ed5ceb..e138754 100644
16--- a/src/core/device.c
17+++ b/src/core/device.c
18@@ -9,6 +9,7 @@
19 #include "bus-error.h"
20 #include "dbus-device.h"
21 #include "dbus-unit.h"
22+#include "device-private.h"
23 #include "device-util.h"
24 #include "device.h"
25 #include "log.h"
26@@ -960,6 +961,25 @@ static int device_dispatch_io(sd_device_monitor *monitor, sd_device *dev, void *
27 if (action == SD_DEVICE_MOVE)
28 device_remove_old_on_move(m, dev);
29
30+ /* When udevd failed to process the device, SYSTEMD_ALIAS or any other properties may contain invalid
31+ * values. Let's refuse to handle the uevent. */
32+ if (sd_device_get_property_value(dev, "UDEV_WORKER_FAILED", NULL) >= 0) {
33+ int v;
34+
35+ if (device_get_property_int(dev, "UDEV_WORKER_ERRNO", &v) >= 0)
36+ log_device_warning_errno(dev, v, "systemd-udevd failed to process the device, ignoring: %m");
37+ else if (device_get_property_int(dev, "UDEV_WORKER_EXIT_STATUS", &v) >= 0)
38+ log_device_warning(dev, "systemd-udevd failed to process the device with exit status %i, ignoring.", v);
39+ else if (device_get_property_int(dev, "UDEV_WORKER_SIGNAL", &v) >= 0) {
40+ const char *s;
41+ (void) sd_device_get_property_value(dev, "UDEV_WORKER_SIGNAL_NAME", &s);
42+ log_device_warning(dev, "systemd-udevd failed to process the device with signal %i(%s), ignoring.", v, strna(s));
43+ } else
44+ log_device_warning(dev, "systemd-udevd failed to process the device with unknown result, ignoring.");
45+
46+ return 0;
47+ }
48+
49 /* A change event can signal that a device is becoming ready, in particular if the device is using
50 * the SYSTEMD_READY logic in udev so we need to reach the else block of the following if, even for
51 * change events */
diff --git a/debian/patches/lp2002445/sd-device-introduce-device_get_property_int.patch b/debian/patches/lp2002445/sd-device-introduce-device_get_property_int.patch
0new file mode 10064452new file mode 100644
index 0000000..3dcdf4a
--- /dev/null
+++ b/debian/patches/lp2002445/sd-device-introduce-device_get_property_int.patch
@@ -0,0 +1,56 @@
1From: Yu Watanabe <watanabe.yu+github@gmail.com>
2Date: Mon, 9 Jan 2023 16:44:11 +0900
3Subject: sd-device: introduce device_get_property_int()
4
5Origin: upstream, https://github.com/systemd/systemd/commit/eedfef0f0d2654fcde2a3b694e62518d688af827
6Bug-Ubuntu: https://launchpad.net/bugs/2002445
7
8(modified to apply to v249.11)
9---
10 src/libsystemd/sd-device/device-private.h | 1 +
11 src/libsystemd/sd-device/sd-device.c | 20 ++++++++++++++++++++
12 2 files changed, 21 insertions(+)
13
14diff --git a/src/libsystemd/sd-device/device-private.h b/src/libsystemd/sd-device/device-private.h
15index 9bb5eff..0ed7807 100644
16--- a/src/libsystemd/sd-device/device-private.h
17+++ b/src/libsystemd/sd-device/device-private.h
18@@ -17,6 +17,7 @@ static inline int device_new_from_watch_handle(sd_device **ret, int wd) {
19 return device_new_from_watch_handle_at(ret, -1, wd);
20 }
21
22+int device_get_property_int(sd_device *device, const char *key, int *ret);
23 int device_get_device_id(sd_device *device, const char **ret);
24
25 int device_get_devlink_priority(sd_device *device, int *priority);
26diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c
27index 49f56ca..2b9be24 100644
28--- a/src/libsystemd/sd-device/sd-device.c
29+++ b/src/libsystemd/sd-device/sd-device.c
30@@ -1904,6 +1904,26 @@ _public_ int sd_device_get_property_value(sd_device *device, const char *key, co
31 return 0;
32 }
33
34+int device_get_property_int(sd_device *device, const char *key, int *ret) {
35+ const char *value;
36+ int r, v;
37+
38+ assert(device);
39+ assert(key);
40+
41+ r = sd_device_get_property_value(device, key, &value);
42+ if (r < 0)
43+ return r;
44+
45+ r = safe_atoi(value, &v);
46+ if (r < 0)
47+ return r;
48+
49+ if (ret)
50+ *ret = v;
51+ return 0;
52+}
53+
54 _public_ int sd_device_get_trigger_uuid(sd_device *device, sd_id128_t *ret) {
55 const char *s;
56 sd_id128_t id;
diff --git a/debian/patches/lp2002445/sd-device-make-device_set_syspath-clear-sysname-and-sysnu.patch b/debian/patches/lp2002445/sd-device-make-device_set_syspath-clear-sysname-and-sysnu.patch
0new file mode 10064457new file mode 100644
index 0000000..82c82e3
--- /dev/null
+++ b/debian/patches/lp2002445/sd-device-make-device_set_syspath-clear-sysname-and-sysnu.patch
@@ -0,0 +1,29 @@
1From: Yu Watanabe <watanabe.yu+github@gmail.com>
2Date: Mon, 9 Jan 2023 14:00:09 +0900
3Subject: sd-device: make device_set_syspath() clear sysname and sysnum
4
5Origin: upstream, https://github.com/systemd/systemd/commit/9a26098e90116fdb5fcffa03485b375ee0c82b6a
6Bug-Ubuntu: https://launchpad.net/bugs/2002445
7
8Otherwise, when a new syspath is assigned to the sd-device object,
9sd_device_get_sysname() or _sysnum() will provide an outdated device
10name or number.
11---
12 src/libsystemd/sd-device/sd-device.c | 4 ++++
13 1 file changed, 4 insertions(+)
14
15diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c
16index 418a5b1..49f56ca 100644
17--- a/src/libsystemd/sd-device/sd-device.c
18+++ b/src/libsystemd/sd-device/sd-device.c
19@@ -213,6 +213,10 @@ int device_set_syspath(sd_device *device, const char *_syspath, bool verify) {
20
21 free_and_replace(device->syspath, syspath);
22 device->devpath = devpath;
23+
24+ /* Unset sysname and sysnum, they will be assigned when requested. */
25+ device->sysnum = NULL;
26+ device->sysname = mfree(device->sysname);
27 return 0;
28 }
29
diff --git a/debian/patches/lp2002445/udev-restore-syspath-and-properties-on-failure.patch b/debian/patches/lp2002445/udev-restore-syspath-and-properties-on-failure.patch
0new file mode 10064430new file mode 100644
index 0000000..c7f5810
--- /dev/null
+++ b/debian/patches/lp2002445/udev-restore-syspath-and-properties-on-failure.patch
@@ -0,0 +1,159 @@
1From: Yu Watanabe <watanabe.yu+github@gmail.com>
2Date: Mon, 9 Jan 2023 14:58:58 +0900
3Subject: udev: restore syspath and properties on failure
4
5Origin: upstream, https://github.com/systemd/systemd/commit/210033847c340c43dd6835520f21f8b23ba29579
6Bug-Ubuntu: https://launchpad.net/bugs/2002445
7
8Otherwise, invalid sysname or properties may be broadcast to udev
9listeners.
10
11(modified to apply to v249.11)
12---
13 src/udev/udev-event.c | 92 ++++++++++++++++++++++++++++++++++++---------------
14 1 file changed, 65 insertions(+), 27 deletions(-)
15
16diff --git a/src/udev/udev-event.c b/src/udev/udev-event.c
17index 84822d9..d538377 100644
18--- a/src/udev/udev-event.c
19+++ b/src/udev/udev-event.c
20@@ -12,6 +12,7 @@
21 #include "sd-event.h"
22
23 #include "alloc-util.h"
24+#include "device-internal.h"
25 #include "device-private.h"
26 #include "device-util.h"
27 #include "fd-util.h"
28@@ -827,19 +828,13 @@ int udev_event_spawn(UdevEvent *event,
29
30 static int rename_netif(UdevEvent *event) {
31 sd_device *dev = event->dev;
32- const char *oldname;
33+ _cleanup_free_ char *old_syspath = NULL, *old_sysname = NULL;
34+ const char *s;
35 int ifindex, r;
36
37 if (!event->name)
38 return 0; /* No new name is requested. */
39
40- r = sd_device_get_sysname(dev, &oldname);
41- if (r < 0)
42- return log_device_error_errno(dev, r, "Failed to get sysname: %m");
43-
44- if (streq(event->name, oldname))
45- return 0; /* The interface name is already requested name. */
46-
47 if (!device_for_action(dev, SD_DEVICE_ADD))
48 return 0; /* Rename the interface only when it is added. */
49
50@@ -847,7 +842,7 @@ static int rename_netif(UdevEvent *event) {
51 if (r == -ENOENT)
52 return 0; /* Device is not a network interface. */
53 if (r < 0)
54- return log_device_error_errno(dev, r, "Failed to get ifindex: %m");
55+ return log_device_warning_errno(dev, r, "Failed to get ifindex: %m");
56
57 if (naming_scheme_has(NAMING_REPLACE_STRICTLY) &&
58 !ifname_valid(event->name)) {
59@@ -855,39 +850,82 @@ static int rename_netif(UdevEvent *event) {
60 return 0;
61 }
62
63- /* Set ID_RENAMING boolean property here, and drop it in the corresponding move uevent later. */
64- r = device_add_property(dev, "ID_RENAMING", "1");
65+ r = sd_device_get_sysname(dev, &s);
66 if (r < 0)
67- return log_device_warning_errno(dev, r, "Failed to add 'ID_RENAMING' property: %m");
68+ return log_device_warning_errno(dev, r, "Failed to get sysname: %m");
69
70- r = device_rename(dev, event->name);
71+ if (streq(event->name, s))
72+ return 0; /* The interface name is already requested name. */
73+
74+ old_sysname = strdup(s);
75+ if (!old_sysname)
76+ return -ENOMEM;
77+
78+ r = sd_device_get_syspath(dev, &s);
79 if (r < 0)
80- return log_device_warning_errno(dev, r, "Failed to update properties with new name '%s': %m", event->name);
81+ return log_device_warning_errno(dev, r, "Failed to get syspath: %m");
82+
83+ old_syspath = strdup(s);
84+ if (!old_syspath)
85+ return -ENOMEM;
86+
87+ r = device_rename(dev, event->name);
88+ if (r < 0) {
89+ log_device_warning_errno(dev, r, "Failed to update properties with new name '%s': %m", event->name);
90+ goto revert;
91+ }
92+
93+ /* Set ID_RENAMING boolean property here. It will be dropped when the corresponding move uevent is processed. */
94+ r = device_add_property(dev, "ID_RENAMING", "1");
95+ if (r < 0) {
96+ log_device_warning_errno(dev, r, "Failed to add 'ID_RENAMING' property: %m");
97+ goto revert;
98+ }
99
100 /* Also set ID_RENAMING boolean property to cloned sd_device object and save it to database
101 * before calling rtnl_set_link_name(). Otherwise, clients (e.g., systemd-networkd) may receive
102 * RTM_NEWLINK netlink message before the database is updated. */
103 r = device_add_property(event->dev_db_clone, "ID_RENAMING", "1");
104- if (r < 0)
105- return log_device_warning_errno(event->dev_db_clone, r, "Failed to add 'ID_RENAMING' property: %m");
106+ if (r < 0) {
107+ log_device_warning_errno(event->dev_db_clone, r, "Failed to add 'ID_RENAMING' property: %m");
108+ goto revert;
109+ }
110
111 r = device_update_db(event->dev_db_clone);
112- if (r < 0)
113- return log_device_debug_errno(event->dev_db_clone, r, "Failed to update database under /run/udev/data/: %m");
114+ if (r < 0) {
115+ log_device_debug_errno(event->dev_db_clone, r, "Failed to update database under /run/udev/data/: %m");
116+ goto revert;
117+ }
118
119 r = rtnl_set_link_name(&event->rtnl, ifindex, event->name);
120- if (r == -EBUSY) {
121- log_device_info(dev, "Network interface '%s' is already up, cannot rename to '%s'.",
122- oldname, event->name);
123- return 0;
124+ if (r < 0) {
125+ if (r == -EBUSY) {
126+ log_device_info(dev, "Network interface '%s' is already up, cannot rename to '%s'.",
127+ old_sysname, event->name);
128+ r = 0;
129+ } else
130+ log_device_error_errno(dev, r, "Failed to rename network interface %i from '%s' to '%s': %m",
131+ ifindex, old_sysname, event->name);
132+ goto revert;
133 }
134- if (r < 0)
135- return log_device_error_errno(dev, r, "Failed to rename network interface %i from '%s' to '%s': %m",
136- ifindex, oldname, event->name);
137-
138- log_device_debug(dev, "Network interface %i is renamed from '%s' to '%s'", ifindex, oldname, event->name);
139
140+ log_device_debug(dev, "Network interface %i is renamed from '%s' to '%s'", ifindex, old_sysname, event->name);
141 return 1;
142+
143+revert:
144+ /* Restore 'dev_db_clone' */
145+ (void) device_add_property(event->dev_db_clone, "ID_RENAMING", NULL);
146+ (void) device_update_db(event->dev_db_clone);
147+
148+ /* Restore 'dev' */
149+ (void) device_set_syspath(dev, old_syspath, /* verify = */ false);
150+ if (sd_device_get_property_value(dev, "INTERFACE_OLD", &s) >= 0) {
151+ (void) device_add_property_internal(dev, "INTERFACE", s);
152+ (void) device_add_property_internal(dev, "INTERFACE_OLD", NULL);
153+ }
154+ (void) device_add_property(dev, "ID_RENAMING", NULL);
155+
156+ return r;
157 }
158
159 static int update_devnode(UdevEvent *event) {
diff --git a/debian/patches/series b/debian/patches/series
index 44b471e..c98336a 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -109,3 +109,7 @@ lp2002445/udev-attempt-device-rename-even-if-interface-is-up.patch
109lp2002445/sd-netlink-add-a-test-for-rtnl_set_link_name.patch109lp2002445/sd-netlink-add-a-test-for-rtnl_set_link_name.patch
110lp2000880-network-create-stacked-netdevs-after-the-underlying-link-.patch110lp2000880-network-create-stacked-netdevs-after-the-underlying-link-.patch
111lp2009502-Enable-dev-sgx_vepc-access-for-the-group-sgx.patch111lp2009502-Enable-dev-sgx_vepc-access-for-the-group-sgx.patch
112lp2002445/sd-device-make-device_set_syspath-clear-sysname-and-sysnu.patch
113lp2002445/udev-restore-syspath-and-properties-on-failure.patch
114lp2002445/sd-device-introduce-device_get_property_int.patch
115lp2002445/core-device-ignore-failed-uevents.patch

Subscribers

People subscribed via source and target branches