core/device: always accept syspath change

When multiple devices have the same devlink, then
adding/updating/removing one of the device may cause syspath change.

Fixes the following issue in
https://github.com/systemd/systemd/issues/23208#issue-1217909746
> the above shows an inconsistency between udev's and systemd's handling
> of the two different devices having the same alias. While udev replaces
> the by-uuid symlink which now points to sdh1 rather than sdd1, systemd
> keeps the previous mapping to sdd1 and emits a warning. This is not the
> problem cause but worth mentioning.

1 files changed, 179 insertions, 55 deletions

diff --git a/src/core/device.c b/src/core/device.c
index 835acd9745..e6f41588e8 100644
--- a/src/core/device.c
+++ b/src/core/device.c
@@ -632,16 +632,9 @@ static int device_setup_unit(Manager *m, sd_device *dev, const char *path, bool
                  * dependency on the mount unit which was added during the loading of the later. When the device is
                  * plugged the sysfs might not be initialized yet, as we serialize the device's state but do not
                  * serialize the sysfs path across reloads/reexecs. Hence, when coming back from a reload/restart we
-                 * might have the state valid, but not the sysfs path. Hence, let's filter out conflicting devices, but
-                 * let's accept devices in any state with no sysfs path set. */
-
-                if (DEVICE(u)->state == DEVICE_PLUGGED &&
-                    DEVICE(u)->sysfs &&
-                    sysfs &&
-                    !path_equal(DEVICE(u)->sysfs, sysfs))
-                        return log_unit_debug_errno(u, SYNTHETIC_ERRNO(EEXIST),
-                                                    "Device %s appeared twice with different sysfs paths %s and %s, ignoring the latter.",
-                                                    e, DEVICE(u)->sysfs, sysfs);
+                 * might have the state valid, but not the sysfs path. Also, there is another possibility; when multiple
+                 * devices have the same devlink (e.g. /dev/disk/by-uuid/xxxx), adding/updating/removing one of the
+                 * device causes syspath change. Hence, let's always update sysfs path.*/
 
                 /* Let's remove all dependencies generated due to udev properties. We'll re-add whatever is configured
                  * now below. */
@@ -719,9 +712,111 @@ static bool device_is_ready(sd_device *dev) {
         return r != 0;
 }
 
+static int device_setup_devlink_unit_one(Manager *m, const char *devlink, sd_device **ret) {
+        _cleanup_(sd_device_unrefp) sd_device *dev = NULL;
+        int r;
+
+        assert(m);
+        assert(devlink);
+
+        if (sd_device_new_from_devname(&dev, devlink) < 0 || !device_is_ready(dev)) {
+                /* the devlink is already removed or not ready */
+                device_update_found_by_name(m, devlink, 0, DEVICE_FOUND_UDEV);
+                *ret = NULL;
+                return 0; /* not ready */
+        }
+
+        r = device_setup_unit(m, dev, devlink, /* main = */ false);
+        if (r < 0)
+                return log_device_warning_errno(dev, r, "Failed to setup unit for '%s': %m", devlink);
+
+        *ret = TAKE_PTR(dev);
+        return 1; /* ready */
+}
+
+static int device_setup_devlink_units(Manager *m, sd_device *dev, char ***ret_ready_devlinks) {
+        _cleanup_strv_free_ char **ready_devlinks = NULL;
+        const char *devlink, *syspath;
+        int r;
+
+        assert(m);
+        assert(dev);
+        assert(ret_ready_devlinks);
+
+        r = sd_device_get_syspath(dev, &syspath);
+        if (r < 0)
+                return r;
+
+        FOREACH_DEVICE_DEVLINK(dev, devlink) {
+                _cleanup_(sd_device_unrefp) sd_device *assigned = NULL;
+                const char *s;
+
+                if (PATH_STARTSWITH_SET(devlink, "/dev/block/", "/dev/char/"))
+                        continue;
+
+                if (device_setup_devlink_unit_one(m, devlink, &assigned) <= 0)
+                        continue;
+
+                if (sd_device_get_syspath(assigned, &s) < 0)
+                        continue;
+
+                if (path_equal(s, syspath))
+                        continue;
+
+                r = strv_extend(&ready_devlinks, devlink);
+                if (r < 0)
+                        return -ENOMEM;
+        }
+
+        *ret_ready_devlinks = TAKE_PTR(ready_devlinks);
+        return 0;
+}
+
+static int device_setup_devlink_units_on_remove(Manager *m, sd_device *dev, char ***ret_ready_devlinks) {
+        _cleanup_strv_free_ char **ready_devlinks = NULL;
+        const char *syspath;
+        Device *l;
+        int r;
+
+        assert(m);
+        assert(dev);
+        assert(ret_ready_devlinks);
+
+        r = sd_device_get_syspath(dev, &syspath);
+        if (r < 0)
+                return r;
+
+        l = hashmap_get(m->devices_by_sysfs, syspath);
+        LIST_FOREACH(same_sysfs, d, l) {
+                _cleanup_(sd_device_unrefp) sd_device *assigned = NULL;
+                const char *s;
+
+                if (!d->path)
+                        continue;
+
+                if (!path_startswith(d->path, "/dev/"))
+                        continue;
+
+                if (device_setup_devlink_unit_one(m, d->path, &assigned) <= 0)
+                        continue;
+
+                if (sd_device_get_syspath(assigned, &s) < 0)
+                        continue;
+
+                if (path_equal(s, syspath))
+                        continue;
+
+                r = strv_extend(&ready_devlinks, d->path);
+                if (r < 0)
+                        return -ENOMEM;
+        }
+
+        *ret_ready_devlinks = TAKE_PTR(ready_devlinks);
+        return 0;
+}
+
 static void device_process_new(Manager *m, sd_device *dev, const char *sysfs) {
         const char *dn, *alias;
-        dev_t devnum;
         int r;
 
         assert(m);
@@ -738,32 +833,6 @@ static void device_process_new(Manager *m, sd_device *dev, const char *sysfs) {
         if (sd_device_get_devname(dev, &dn) >= 0)
                 (void) device_setup_unit(m, dev, dn, false);
 
-        /* Add additional units for all symlinks */
-        if (sd_device_get_devnum(dev, &devnum) >= 0) {
-                const char *p;
-
-                FOREACH_DEVICE_DEVLINK(dev, p) {
-                        struct stat st;
-
-                        if (PATH_STARTSWITH_SET(p, "/dev/block/", "/dev/char/"))
-                                continue;
-
-                        /* Verify that the symlink in the FS actually belongs to this device. This is useful
-                         * to deal with conflicting devices, e.g. when two disks want the same
-                         * /dev/disk/by-label/xxx link because they have the same label. We want to make sure
-                         * that the same device that won the symlink wins in systemd, so we check the device
-                         * node major/minor */
-                        if (stat(p, &st) >= 0 &&
-                            ((!S_ISBLK(st.st_mode) && !S_ISCHR(st.st_mode)) ||
-                             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);
-                }
-        }
-
         /* Add additional units for all explicitly configured aliases */
         r = sd_device_get_property_value(dev, "SYSTEMD_ALIAS", &alias);
         if (r < 0) {
@@ -907,10 +976,9 @@ static void device_enumerate(Manager *m) {
         }
 
         FOREACH_DEVICE(e, dev) {
+                _cleanup_strv_free_ char **ready_devlinks = NULL;
                 const char *sysfs;
-
-                if (!device_is_ready(dev))
-                        continue;
+                bool ready;
 
                 r = sd_device_get_syspath(dev, &sysfs);
                 if (r < 0) {
@@ -918,8 +986,18 @@ static void device_enumerate(Manager *m) {
                         continue;
                 }
 
-                device_process_new(m, dev, sysfs);
-                device_update_found_by_sysfs(m, sysfs, DEVICE_FOUND_UDEV, DEVICE_FOUND_UDEV);
+                ready = device_is_ready(dev);
+                if (ready)
+                        device_process_new(m, dev, sysfs);
+
+                /* Add additional units for all symlinks */
+                (void) device_setup_devlink_units(m, dev, &ready_devlinks);
+
+                if (ready)
+                        device_update_found_by_sysfs(m, sysfs, DEVICE_FOUND_UDEV, DEVICE_FOUND_UDEV);
+
+                STRV_FOREACH(devlink, ready_devlinks)
+                        device_update_found_by_name(m, *devlink, DEVICE_FOUND_UDEV, DEVICE_FOUND_UDEV);
         }
 
         return;
@@ -942,10 +1020,34 @@ static void device_propagate_reload_by_sysfs(Manager *m, const char *sysfs) {
 
                 r = manager_propagate_reload(m, UNIT(d), JOB_REPLACE, NULL);
                 if (r < 0)
-                        log_warning_errno(r, "Failed to propagate reload, ignoring: %m");
+                        log_unit_warning_errno(UNIT(d), r, "Failed to propagate reload, ignoring: %m");
         }
 }
 
+static void device_propagate_reload_by_name(Manager *m, const char *path) {
+        _cleanup_free_ char *e = NULL;
+        Unit *u;
+        int r;
+
+        assert(m);
+        assert(path);
+
+        r = unit_name_from_path(path, ".device", &e);
+        if (r < 0)
+                return (void) log_debug_errno(r, "Failed to generate unit name from device path, ignoring: %m");
+
+        u = manager_get_unit(m, e);
+        if (!u)
+                return;
+
+        if (DEVICE(u)->state == DEVICE_DEAD)
+                return;
+
+        r = manager_propagate_reload(m, u, JOB_REPLACE, NULL);
+        if (r < 0)
+                log_unit_warning_errno(u, r, "Failed to propagate reload, ignoring: %m");
+}
+
 static void device_remove_old_on_move(Manager *m, sd_device *dev) {
         _cleanup_free_ char *syspath_old = NULL;
         const char *devpath_old;
@@ -966,9 +1068,11 @@ static void device_remove_old_on_move(Manager *m, sd_device *dev) {
 }
 
 static int device_dispatch_io(sd_device_monitor *monitor, sd_device *dev, void *userdata) {
+        _cleanup_strv_free_ char **ready_devlinks = NULL;
         Manager *m = ASSERT_PTR(userdata);
         sd_device_action_t action;
         const char *sysfs;
+        bool ready;
         int r;
 
         assert(dev);
@@ -987,9 +1091,6 @@ static int device_dispatch_io(sd_device_monitor *monitor, sd_device *dev, void *
                 return 0;
         }
 
-        if (!IN_SET(action, SD_DEVICE_ADD, SD_DEVICE_REMOVE, SD_DEVICE_MOVE))
-                device_propagate_reload_by_sysfs(m, sysfs);
-
         if (action == SD_DEVICE_MOVE)
                 device_remove_old_on_move(m, dev);
 
@@ -1001,27 +1102,50 @@ static int device_dispatch_io(sd_device_monitor *monitor, sd_device *dev, void *
                 if (r < 0)
                         log_device_warning_errno(dev, r, "Failed to process swap device remove event, ignoring: %m");
 
-                /* If we get notified that a device was removed by udev, then it's completely gone, hence
-                 * unset all found bits */
-                device_update_found_by_sysfs(m, sysfs, DEVICE_NOT_FOUND, DEVICE_FOUND_MASK);
+                ready = false;
 
-        } else if (device_is_ready(dev)) {
+                (void) device_setup_devlink_units_on_remove(m, dev, &ready_devlinks);
 
-                device_process_new(m, dev, sysfs);
+        } else {
+                ready = device_is_ready(dev);
 
-                r = swap_process_device_new(m, dev);
-                if (r < 0)
-                        log_device_warning_errno(dev, r, "Failed to process swap device new event, ignoring: %m");
+                if (ready) {
+                        device_process_new(m, dev, sysfs);
 
+                        r = swap_process_device_new(m, dev);
+                        if (r < 0)
+                                log_device_warning_errno(dev, r, "Failed to process swap device new event, ignoring: %m");
+                }
+
+                /* Add additional units for all symlinks */
+                (void) device_setup_devlink_units(m, dev, &ready_devlinks);
+        }
+
+        if (!IN_SET(action, SD_DEVICE_ADD, SD_DEVICE_REMOVE, SD_DEVICE_MOVE)) {
+                device_propagate_reload_by_sysfs(m, sysfs);
+
+                STRV_FOREACH(devlink, ready_devlinks)
+                        device_propagate_reload_by_name(m, *devlink);
+        }
+
+        if (ready || !strv_isempty(ready_devlinks))
                 manager_dispatch_load_queue(m);
 
+        if (action == SD_DEVICE_REMOVE)
+                /* If we get notified that a device was removed by udev, then it's completely gone, hence
+                 * unset all found bits */
+                device_update_found_by_sysfs(m, sysfs, 0, DEVICE_FOUND_MASK);
+        else if (ready)
                 /* The device is found now, set the udev found bit */
                 device_update_found_by_sysfs(m, sysfs, DEVICE_FOUND_UDEV, DEVICE_FOUND_UDEV);
-        } else
+        else
                 /* The device is nominally around, but not ready for us. Hence unset the udev bit, but leave
                  * the rest around. */
                 device_update_found_by_sysfs(m, sysfs, DEVICE_NOT_FOUND, DEVICE_FOUND_UDEV);
 
+        STRV_FOREACH(devlink, ready_devlinks)
+                device_update_found_by_name(m, *devlink, DEVICE_FOUND_UDEV, DEVICE_FOUND_UDEV);
+
         return 0;
 }