From: Yu Watanabe <watanabe.yu+github@gmail.com>
Date: Mon, 25 Nov 2024 16:19:35 +0000 (+0900)
Subject: core/device: handle ID_PROCESSING udev property
X-Git-Tag: v258-rc1~1800^2~1
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=ad920b4cb318f9030300df8e93fdd85ca1b055db;p=thirdparty%2Fsystemd.git

core/device: handle ID_PROCESSING udev property

If an enumerated device has ID_PROCESSING=1 property, and the service
manager does not know if the device has been processed by udevd
previously (that is, Device.deserialized_found does not have
DEVICE_FOUND_UDEV), then drop DEVICE_FOUND_UDEV flag from the device and
make the device not enter the active state.

Follow-up for 405be62f05d76f1845f347737b5972158c79dd3e, which was
reverted by c4fc22c4defc5983e53f4ce048e15ea7d31e6a75.
---

diff --git a/src/core/device.c b/src/core/device.c
index 85b94e2d60c..e201d099768 100644
--- a/src/core/device.c
+++ b/src/core/device.c
@@ -334,6 +334,20 @@ static void device_catchup(Unit *u) {
         Device *d = ASSERT_PTR(DEVICE(u));
 
         /* Second, let's update the state with the enumerated state */
+
+        /* If Device.found (set from Device.deserialized_found) does not have DEVICE_FOUND_UDEV, and the
+         * device has not been processed by udevd while enumeration, it indicates the unit was never active
+         * before reexecution, hence we can safely drop the flag from Device.enumerated_found. The device
+         * will be set up later when udev finishes processing (see also comment in
+         * device_setup_devlink_unit_one()).
+         *
+         * NB: 💣💣💣 If Device.found already contains udev, i.e. the unit was fully ready before
+         * reexecution, do not unset the flag. Otherwise, e.g. if systemd-udev-trigger.service is started
+         * just before reexec, reload, and so on, devices being reprocessed (carrying ID_PROCESSING=1
+         * property) on enumeration and will enter dead state. See issue #35329. */
+        if (!FLAGS_SET(d->found, DEVICE_FOUND_UDEV) && !d->processed)
+                d->enumerated_found &= ~DEVICE_FOUND_UDEV;
+
         device_update_found_one(d, d->enumerated_found, _DEVICE_FOUND_MASK);
 }
 
@@ -779,8 +793,16 @@ static int device_setup_devlink_unit_one(Manager *m, const char *devlink, Set **
         assert(ready_units);
         assert(not_ready_units);
 
-        if (sd_device_new_from_devname(&dev, devlink) >= 0 && device_is_ready(dev))
+        if (sd_device_new_from_devname(&dev, devlink) >= 0 && device_is_ready(dev)) {
+                if (MANAGER_IS_RUNNING(m) && device_is_processed(dev) <= 0)
+                        /* The device is being processed by udevd. We will receive relevant uevent for the
+                         * device later when completed. Let's ignore the device now. */
+                        return 0;
+
+                /* Note, even if the device is being processed by udevd, setup the unit on enumerate.
+                 * See also the comments in device_catchup(). */
                 return device_setup_unit(m, dev, devlink, /* main = */ false, ready_units);
+        }
 
         /* the devlink is already removed or not ready */
         if (device_by_path(m, devlink, &u) < 0)
@@ -1051,13 +1073,32 @@ static void device_enumerate(Manager *m) {
 
         FOREACH_DEVICE(e, dev) {
                 _cleanup_set_free_ Set *ready_units = NULL, *not_ready_units = NULL;
+                const char *syspath;
+                bool processed;
                 Device *d;
 
+                r = sd_device_get_syspath(dev, &syspath);
+                if (r < 0) {
+                        log_device_debug_errno(dev, r, "Failed to get syspath of enumerated device, ignoring: %m");
+                        continue;
+                }
+
+                r = device_is_processed(dev);
+                if (r < 0)
+                        log_device_debug_errno(dev, r, "Failed to check if device is processed by udevd, assuming not: %m");
+                processed = r > 0;
+
                 if (device_setup_units(m, dev, &ready_units, &not_ready_units) < 0)
                         continue;
 
-                SET_FOREACH(d, ready_units)
+                SET_FOREACH(d, ready_units) {
                         device_update_found_one(d, DEVICE_FOUND_UDEV, DEVICE_FOUND_UDEV);
+
+                        /* Why we need to check the syspath here? Because the device unit may be generated by
+                         * a devlink, and the syspath may be different from the one of the original device. */
+                        if (path_equal(d->sysfs, syspath))
+                                d->processed = processed;
+                }
                 SET_FOREACH(d, not_ready_units)
                         device_update_found_one(d, DEVICE_NOT_FOUND, DEVICE_FOUND_UDEV);
         }
diff --git a/src/core/device.h b/src/core/device.h
index 4eb656777ae..631f45d45eb 100644
--- a/src/core/device.h
+++ b/src/core/device.h
@@ -29,7 +29,9 @@ struct Device {
 
         DeviceState state, deserialized_state;
         DeviceFound found, deserialized_found, enumerated_found;
-
+        bool processed; /* Whether udevd has done processing the device, i.e. the device has database and
+                         * ID_PROCESSING=1 udev property is not set. This is used only by enumeration and
+                         * subsequent catchup process. */
         bool bind_mounts;
 
         /* The SYSTEMD_WANTS udev property for this device the last time we saw it */