From 20907e5d86166c0840687adb4cd99f9cfe638e98 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Sat, 7 May 2022 14:57:52 +0200 Subject: [PATCH 1/7] core/device: Add sysfs argument to device_process_new() Instead of retrieving the new sysfs path in device_process_new(), let's pass the syspath we retrieved earlier to device_process_new() similar to how we do for other functions in core/device.c. --- src/core/device.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/core/device.c b/src/core/device.c index f65dbfcb1e..d886119ffe 100644 --- a/src/core/device.c +++ b/src/core/device.c @@ -552,16 +552,14 @@ static int device_setup_unit(Manager *m, sd_device *dev, const char *path, bool return 0; } -static void device_process_new(Manager *m, sd_device *dev) { - const char *sysfs, *dn, *alias; +static void device_process_new(Manager *m, sd_device *dev, const char *sysfs) { + const char *dn, *alias; dev_t devnum; int r; assert(m); assert(dev); - - if (sd_device_get_syspath(dev, &sysfs) < 0) - return; + assert(sysfs); /* Add the main unit named after the sysfs path. If this one fails, don't bother with the rest, as * this one shall be the main device unit the others just follow. (Compare with how @@ -837,11 +835,13 @@ static void device_enumerate(Manager *m) { if (!device_is_ready(dev)) continue; - device_process_new(m, dev); - - if (sd_device_get_syspath(dev, &sysfs) < 0) + r = sd_device_get_syspath(dev, &sysfs); + if (r < 0) { + log_device_debug_errno(dev, r, "Couldn't get syspath from device, ignoring: %m"); continue; + } + device_process_new(m, dev, sysfs); device_update_found_by_sysfs(m, sysfs, DEVICE_FOUND_UDEV, DEVICE_FOUND_UDEV); } @@ -928,7 +928,7 @@ static int device_dispatch_io(sd_device_monitor *monitor, sd_device *dev, void * } else if (device_is_ready(dev)) { - device_process_new(m, dev); + device_process_new(m, dev, sysfs); r = swap_process_device_new(m, dev); if (r < 0) From 6cc4da5eb5346b99a66675d81d7251b6bb631ac4 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Sat, 7 May 2022 16:15:00 +0200 Subject: [PATCH 2/7] core/device: Log on every event received from udev We already debug log on device unit state changes but additionally, let's debug log for every event received from udev as well. --- src/core/device.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/core/device.c b/src/core/device.c index d886119ffe..5bee0b5c16 100644 --- a/src/core/device.c +++ b/src/core/device.c @@ -896,6 +896,8 @@ static int device_dispatch_io(sd_device_monitor *monitor, sd_device *dev, void * assert(dev); + log_device_uevent(dev, "Processing udev action"); + r = sd_device_get_syspath(dev, &sysfs); if (r < 0) { log_device_error_errno(dev, r, "Failed to get device sys path: %m"); From e9222bfcfa1665a777f280ef4deff98ec533ee30 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Sat, 7 May 2022 15:18:32 +0200 Subject: [PATCH 3/7] core/device: Log if we fail to open a device We also shorten the logic by getting rid of the validate_node() function. An extra check is added to verify we're dealing with a device before calling sd_device_new_from_devname() since that will return -EINVAL if anything other than a device is passed. --- src/core/device.c | 36 ++++++++++++------------------------ 1 file changed, 12 insertions(+), 24 deletions(-) diff --git a/src/core/device.c b/src/core/device.c index 5bee0b5c16..1c7dde2f22 100644 --- a/src/core/device.c +++ b/src/core/device.c @@ -948,29 +948,9 @@ static int device_dispatch_io(sd_device_monitor *monitor, sd_device *dev, void * return 0; } -static int validate_node(const char *node, sd_device **ret) { - _cleanup_(sd_device_unrefp) sd_device *dev = NULL; +void device_found_node(Manager *m, const char *node, DeviceFound found, DeviceFound mask) { int r; - assert(node); - assert(ret); - - /* Validates a device node that showed up in /proc/swaps or /proc/self/mountinfo if it makes sense for us to - * track. Note that this validator is fine within missing device nodes, but not with badly set up ones! */ - - r = sd_device_new_from_devname(&dev, node); - if (r == -ENODEV) { - *ret = NULL; - return 0; /* good! (though missing) */ - } - if (r < 0) - return r; /* bad! */ - - *ret = TAKE_PTR(dev); - return 1; /* good! */ -} - -void device_found_node(Manager *m, const char *node, DeviceFound found, DeviceFound mask) { assert(m); assert(node); assert(!FLAGS_SET(mask, DEVICE_FOUND_UDEV)); @@ -994,10 +974,18 @@ void device_found_node(Manager *m, const char *node, DeviceFound found, DeviceFo /* If the device is known in the kernel and newly appeared, then we'll create a device unit for it, * under the name referenced in /proc/swaps or /proc/self/mountinfo. But first, let's validate if - * everything is alright with the device node. */ + * everything is alright with the device node. Note that we're fine with missing device nodes, + * but not with badly set up ones. */ - if (validate_node(node, &dev) < 0) - return; /* Don't create a device unit for this if the device node is borked. */ + r = sd_device_new_from_devname(&dev, node); + if (r == -ENODEV) + log_debug("Could not find device for %s, continuing without device node", node); + else if (r < 0) { + /* Reduce log noise from nodes which are not device nodes by skipping EINVAL. */ + if (r != -EINVAL) + log_error_errno(r, "Failed to open %s device, ignoring: %m", node); + return; + } (void) device_setup_unit(m, dev, node, false); /* 'dev' may be NULL. */ } From 2f8287455891744b04b86a856434247cf9c7a5cb Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Sat, 7 May 2022 15:53:52 +0200 Subject: [PATCH 4/7] core/device: Log when we skip device unit creation for symlink --- src/core/device.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/core/device.c b/src/core/device.c index 1c7dde2f22..158a61b49c 100644 --- a/src/core/device.c +++ b/src/core/device.c @@ -588,8 +588,10 @@ static void device_process_new(Manager *m, sd_device *dev, const char *sysfs) { * node major/minor */ if (stat(p, &st) >= 0 && ((!S_ISBLK(st.st_mode) && !S_ISCHR(st.st_mode)) || - st.st_rdev != devnum)) + st.st_rdev != devnum)) { + log_device_debug(dev, "Skipping device unit creation for symlink %s not owned by device", p); continue; + } (void) device_setup_unit(m, dev, p, false); } From c34511086f84a416a5650437d58aeb7fe961f521 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Sat, 7 May 2022 15:56:34 +0200 Subject: [PATCH 5/7] core/device: Log when we can't retrieve SYSTEMD_ALIAS property --- src/core/device.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/core/device.c b/src/core/device.c index 158a61b49c..30f7cd64c8 100644 --- a/src/core/device.c +++ b/src/core/device.c @@ -598,8 +598,12 @@ static void device_process_new(Manager *m, sd_device *dev, const char *sysfs) { } /* Add additional units for all explicitly configured aliases */ - if (sd_device_get_property_value(dev, "SYSTEMD_ALIAS", &alias) < 0) + r = sd_device_get_property_value(dev, "SYSTEMD_ALIAS", &alias); + if (r < 0) { + if (r != -ENOENT) + log_device_error_errno(dev, r, "Failed to get SYSTEMD_ALIAS property, ignoring: %m"); return; + } for (;;) { _cleanup_free_ char *word = NULL; From f0fdcb8cfae357e8648f06d0301d3c4ac9b0aa07 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Sat, 7 May 2022 16:20:20 +0200 Subject: [PATCH 6/7] core/device: Improve device_dispatch_io() logging messages --- src/core/device.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/device.c b/src/core/device.c index 30f7cd64c8..e28108ef5c 100644 --- a/src/core/device.c +++ b/src/core/device.c @@ -906,13 +906,13 @@ static int device_dispatch_io(sd_device_monitor *monitor, sd_device *dev, void * r = sd_device_get_syspath(dev, &sysfs); if (r < 0) { - log_device_error_errno(dev, r, "Failed to get device sys path: %m"); + log_device_error_errno(dev, r, "Failed to get device syspath, ignoring: %m"); return 0; } r = sd_device_get_action(dev, &action); if (r < 0) { - log_device_error_errno(dev, r, "Failed to get udev action: %m"); + log_device_error_errno(dev, r, "Failed to get udev action, ignoring: %m"); return 0; } From c189f91f1f6e9357b0f4c3393f62da3716ef90b6 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Sat, 7 May 2022 17:52:44 +0200 Subject: [PATCH 7/7] core/device: Add logging to device_is_ready() --- src/core/device.c | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/src/core/device.c b/src/core/device.c index e28108ef5c..4885fa9295 100644 --- a/src/core/device.c +++ b/src/core/device.c @@ -709,16 +709,34 @@ static void device_update_found_by_name(Manager *m, const char *path, DeviceFoun } static bool device_is_ready(sd_device *dev) { + int r; + assert(dev); - if (device_is_renaming(dev) > 0) + r = device_is_renaming(dev); + if (r < 0) + log_device_warning_errno(dev, r, "Failed to check if device is renaming, assuming device is not renaming: %m"); + if (r > 0) { + log_device_debug(dev, "Device busy: device is renaming"); return false; + } /* Is it really tagged as 'systemd' right now? */ - if (sd_device_has_current_tag(dev, "systemd") <= 0) + r = sd_device_has_current_tag(dev, "systemd"); + if (r < 0) + log_device_warning_errno(dev, r, "Failed to check if device has \"systemd\" tag, assuming device is not tagged with \"systemd\": %m"); + if (r == 0) + log_device_debug(dev, "Device busy: device is not tagged with \"systemd\""); + if (r <= 0) return false; - return device_get_property_bool(dev, "SYSTEMD_READY") != 0; + r = device_get_property_bool(dev, "SYSTEMD_READY"); + if (r < 0 && r != -ENOENT) + log_device_warning_errno(dev, r, "Failed to get device SYSTEMD_READY property, assuming device does not have \"SYSTEMD_READY\" property: %m"); + if (r == 0) + log_device_debug(dev, "Device busy: SYSTEMD_READY property from device is false"); + + return r != 0; } static Unit *device_following(Unit *u) {