core/device: always update existing devlink or alias units on uevent

Previously, existing device units for devlinks or aliases were not
removed unless the main device unit is removed. This makes all existing
device units for devlinks and aliases are checked if they are still
required, and remove if not necessary anymore.

Fixes #24518.

1 files changed, 146 insertions, 169 deletions

diff --git a/src/core/device.c b/src/core/device.c
index 442efa6ba2..ebe1a934f4 100644
--- a/src/core/device.c
+++ b/src/core/device.c
@@ -620,7 +620,7 @@ static void device_upgrade_mount_deps(Unit *u) {
         }
 }
 
-static int device_setup_unit(Manager *m, sd_device *dev, const char *path, bool main) {
+static int device_setup_unit(Manager *m, sd_device *dev, const char *path, bool main, Set **units) {
         _cleanup_(unit_freep) Unit *new_unit = NULL;
         _cleanup_free_ char *e = NULL;
         const char *sysfs = NULL;
@@ -697,6 +697,12 @@ static int device_setup_unit(Manager *m, sd_device *dev, const char *path, bool
         if (dev && device_is_bound_by_mounts(DEVICE(u), dev))
                 device_upgrade_mount_deps(u);
 
+        if (units) {
+                r = set_ensure_put(units, NULL, DEVICE(u));
+                if (r < 0)
+                        return log_unit_error_errno(u, r, "Failed to store unit: %m");
+        }
+
         TAKE_PTR(new_unit);
         return 0;
 }
@@ -735,153 +741,169 @@ 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) {
+static int device_setup_devlink_unit_one(Manager *m, const char *devlink, Set **ready_units, Set **not_ready_units) {
         _cleanup_(sd_device_unrefp) sd_device *dev = NULL;
-        int r;
+        Unit *u;
 
         assert(m);
         assert(devlink);
+        assert(ready_units);
+        assert(not_ready_units);
 
-        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, DEVICE_NOT_FOUND, DEVICE_FOUND_UDEV);
-                *ret = NULL;
-                return 0; /* not ready */
-        }
+        if (sd_device_new_from_devname(&dev, devlink) >= 0 && device_is_ready(dev))
+                return device_setup_unit(m, dev, devlink, /* main = */ false, ready_units);
 
-        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);
+        /* the devlink is already removed or not ready */
+        if (device_by_path(m, devlink, &u) < 0)
+                return 0; /* The corresponding .device unit not found. That's fine. */
 
-        *ret = TAKE_PTR(dev);
-        return 1; /* ready */
+        return set_ensure_put(not_ready_units, NULL, DEVICE(u));
 }
 
-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;
+static int device_setup_extra_units(Manager *m, sd_device *dev, Set **ready_units, Set **not_ready_units) {
+        _cleanup_strv_free_ char **aliases = NULL;
+        const char *devlink, *syspath, *devname = NULL;
+        Device *l;
         int r;
 
         assert(m);
         assert(dev);
-        assert(ret_ready_devlinks);
+        assert(ready_units);
+        assert(not_ready_units);
 
         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;
+        (void) sd_device_get_devname(dev, &devname);
 
+        /* devlink units */
+        FOREACH_DEVICE_DEVLINK(dev, devlink) {
+                /* These are a kind of special devlink. They should be always unique, but neither persistent
+                 * nor predictable. Hence, let's refuse them. See also the comments for alias units below. */
                 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;
+                (void) device_setup_devlink_unit_one(m, devlink, ready_units, not_ready_units);
+        }
 
-                if (path_equal(s, syspath))
-                        continue;
+        if (device_is_ready(dev)) {
+                const char *s;
 
-                r = strv_extend(&ready_devlinks, devlink);
-                if (r < 0)
-                        return -ENOMEM;
+                r = sd_device_get_property_value(dev, "SYSTEMD_ALIAS", &s);
+                if (r < 0 && r != -ENOENT)
+                        log_device_warning_errno(dev, r, "Failed to get SYSTEMD_ALIAS property, ignoring: %m");
+                if (r >= 0) {
+                        r = strv_split_full(&aliases, s, NULL, EXTRACT_UNQUOTE);
+                        if (r < 0)
+                                log_device_warning_errno(dev, r, "Failed to parse SYSTEMD_ALIAS property, ignoring: %m");
+                }
         }
 
-        *ret_ready_devlinks = TAKE_PTR(ready_devlinks);
-        return 0;
-}
+        /* alias units */
+        STRV_FOREACH(alias, aliases) {
+                if (!path_is_absolute(*alias)) {
+                        log_device_warning(dev, "The alias \"%s\" specified in SYSTEMD_ALIAS is not an absolute path, ignoring.", *alias);
+                        continue;
+                }
 
-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;
+                if (!path_is_normalized(*alias)) {
+                        log_device_warning(dev, "The alias \"%s\" specified in SYSTEMD_ALIAS is not a normalized path, ignoring.", *alias);
+                        continue;
+                }
 
-        assert(m);
-        assert(dev);
-        assert(ret_ready_devlinks);
+                /* Note, even if the devlink is not persistent, LVM expects /dev/block/ symlink units exist.
+                 * To achieve that, they set the path to SYSTEMD_ALIAS. Hence, we cannot refuse aliases start
+                 * with /dev/, unfortunately. */
 
-        r = sd_device_get_syspath(dev, &syspath);
-        if (r < 0)
-                return r;
+                (void) device_setup_unit(m, dev, *alias, /* main = */ false, ready_units);
+        }
 
         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 (path_equal(d->path, syspath))
+                        continue; /* This is the main unit. */
 
-                if (device_setup_devlink_unit_one(m, d->path, &assigned) <= 0)
-                        continue;
+                if (devname && path_equal(d->path, devname))
+                        continue; /* This is the real device node. */
 
-                if (sd_device_get_syspath(assigned, &s) < 0)
-                        continue;
+                if (device_has_devlink(dev, d->path))
+                        continue; /* The devlink was already processed in the above loop. */
 
-                if (path_equal(s, syspath))
-                        continue;
+                if (strv_contains(aliases, d->path))
+                        continue; /* This is already processed in the above, and ready. */
 
-                r = strv_extend(&ready_devlinks, d->path);
-                if (r < 0)
-                        return -ENOMEM;
+                if (path_startswith(d->path, "/dev/"))
+                        /* This is a devlink unit. Check existence and update syspath. */
+                        (void) device_setup_devlink_unit_one(m, d->path, ready_units, not_ready_units);
+                else
+                        /* This is an alias unit of dropped or not ready device. */
+                        (void) set_ensure_put(not_ready_units, NULL, d);
         }
 
-        *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;
+static int device_setup_units(Manager *m, sd_device *dev, Set **ready_units, Set **not_ready_units) {
+        const char *syspath, *devname = NULL;
         int r;
 
         assert(m);
         assert(dev);
-        assert(sysfs);
+        assert(ready_units);
+        assert(not_ready_units);
 
-        /* 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
-         * device_following() is implemented, see below, which looks for the sysfs device.) */
-        if (device_setup_unit(m, dev, sysfs, /* main = */ true) < 0)
-                return;
+        r = sd_device_get_syspath(dev, &syspath);
+        if (r < 0)
+                return log_device_debug_errno(dev, r, "Couldn't get syspath from device, ignoring: %m");
 
-        /* Add an additional unit for the device node */
-        if (sd_device_get_devname(dev, &dn) >= 0)
-                (void) device_setup_unit(m, dev, dn, /* main = */ false);
+        /* First, process the main (that is, points to the syspath) and (real, not symlink) devnode units. */
+        if (device_for_action(dev, SD_DEVICE_REMOVE))
+                /* If the device is removed, the main and devnode units units will be removed by
+                 * device_update_found_by_sysfs() in device_dispatch_io(). Hence, it is not necessary to
+                 * store them to not_ready_units, and we have nothing to do here.
+                 *
+                 * Note, still we need to process devlink units below, as a devlink previously points to this
+                 * device may still exist and now point to another device node. That is, do not forget to
+                 * call device_setup_extra_units(). */
+                ;
+        else if (device_is_ready(dev)) {
+                /* Add the main unit named after the syspath. 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
+                 * device_following() is implemented, see below, which looks for the sysfs device.) */
+                r = device_setup_unit(m, dev, syspath, /* main = */ true, ready_units);
+                if (r < 0)
+                        return r;
 
-        /* Add additional units for all explicitly configured aliases */
-        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;
-        }
+                /* Add an additional unit for the device node */
+                if (sd_device_get_devname(dev, &devname) >= 0)
+                        (void) device_setup_unit(m, dev, devname, /* main = */ false, ready_units);
 
-        for (;;) {
-                _cleanup_free_ char *word = NULL;
+        } else {
+                Unit *u;
 
-                r = extract_first_word(&alias, &word, NULL, EXTRACT_UNQUOTE);
-                if (r == 0)
-                        break;
-                if (r == -ENOMEM)
-                        return (void) log_oom();
-                if (r < 0)
-                        return (void) log_device_warning_errno(dev, r, "Failed to parse SYSTEMD_ALIAS property, ignoring: %m");
+                /* If the device exists but not ready, then save the units and unset udev bits later. */
 
-                if (!path_is_absolute(word))
-                        log_device_warning(dev, "SYSTEMD_ALIAS is not an absolute path, ignoring: %s", word);
-                else if (!path_is_normalized(word))
-                        log_device_warning(dev, "SYSTEMD_ALIAS is not a normalized path, ignoring: %s", word);
-                else
-                        (void) device_setup_unit(m, dev, word, /* main = */ false);
+                if (device_by_path(m, syspath, &u) >= 0) {
+                        r = set_ensure_put(not_ready_units, NULL, DEVICE(u));
+                        if (r < 0)
+                                log_unit_debug_errno(u, r, "Failed to store unit, ignoring: %m");
+                }
+
+                if (sd_device_get_devname(dev, &devname) >= 0 &&
+                    device_by_path(m, devname, &u) >= 0) {
+                        r = set_ensure_put(not_ready_units, NULL, DEVICE(u));
+                        if (r < 0)
+                                log_unit_debug_errno(u, r, "Failed to store unit, ignoring: %m");
+                }
         }
+
+        /* Next, add/update additional .device units point to aliases and symlinks. */
+        (void) device_setup_extra_units(m, dev, ready_units, not_ready_units);
+        return 0;
 }
 
 static Unit *device_following(Unit *u) {
@@ -999,28 +1021,16 @@ static void device_enumerate(Manager *m) {
         }
 
         FOREACH_DEVICE(e, dev) {
-                _cleanup_strv_free_ char **ready_devlinks = NULL;
-                const char *sysfs;
-                bool ready;
+                _cleanup_set_free_ Set *ready_units = NULL, *not_ready_units = NULL;
+                Device *d;
 
-                r = sd_device_get_syspath(dev, &sysfs);
-                if (r < 0) {
-                        log_device_debug_errno(dev, r, "Couldn't get syspath from device, ignoring: %m");
+                if (device_setup_units(m, dev, &ready_units, &not_ready_units) < 0)
                         continue;
-                }
-
-                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);
+                SET_FOREACH(d, ready_units)
+                        device_update_found_one(d, DEVICE_FOUND_UDEV, DEVICE_FOUND_UDEV);
+                SET_FOREACH(d, not_ready_units)
+                        device_update_found_one(d, DEVICE_NOT_FOUND, DEVICE_FOUND_UDEV);
         }
 
         return;
@@ -1043,30 +1053,6 @@ static void device_propagate_reload(Manager *m, Device *d) {
                 log_unit_warning_errno(UNIT(d), r, "Failed to propagate reload, ignoring: %m");
 }
 
-static void device_propagate_reload_by_sysfs(Manager *m, const char *sysfs) {
-        Device *l;
-
-        assert(m);
-        assert(sysfs);
-
-        l = hashmap_get(m->devices_by_sysfs, sysfs);
-        LIST_FOREACH(same_sysfs, d, l)
-                device_propagate_reload(m, d);
-}
-
-static void device_propagate_reload_by_name(Manager *m, const char *path) {
-        Unit *u;
-        int r;
-
-        assert(m);
-        assert(path);
-
-        if (device_by_path(m, path, &u) < 0)
-                return;
-
-        device_propagate_reload(m, DEVICE(u));
-}
-
 static void device_remove_old_on_move(Manager *m, sd_device *dev) {
         _cleanup_free_ char *syspath_old = NULL;
         const char *devpath_old;
@@ -1087,11 +1073,12 @@ 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;
+        _cleanup_set_free_ Set *ready_units = NULL, *not_ready_units = NULL;
         Manager *m = ASSERT_PTR(userdata);
         sd_device_action_t action;
         const char *sysfs;
         bool ready;
+        Device *d;
         int r;
 
         assert(dev);
@@ -1118,50 +1105,40 @@ static int device_dispatch_io(sd_device_monitor *monitor, sd_device *dev, void *
          * change events */
         ready = device_is_ready(dev);
 
+        (void) device_setup_units(m, dev, &ready_units, &not_ready_units);
+
         if (action == SD_DEVICE_REMOVE) {
                 r = swap_process_device_remove(m, dev);
                 if (r < 0)
                         log_device_warning_errno(dev, r, "Failed to process swap device remove event, ignoring: %m");
-
-                (void) device_setup_devlink_units_on_remove(m, dev, &ready_devlinks);
-
-        } else {
-                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);
+        } else if (ready) {
+                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 (!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 (!IN_SET(action, SD_DEVICE_ADD, SD_DEVICE_REMOVE, SD_DEVICE_MOVE))
+                SET_FOREACH(d, ready_units)
+                        device_propagate_reload(m, d);
 
-        if (ready || !strv_isempty(ready_devlinks))
+        if (!set_isempty(ready_units))
                 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 */
+                 * unset all found bits. Note this affects all .device units still point to the removed
+                 * device. */
                 device_update_found_by_sysfs(m, sysfs, DEVICE_NOT_FOUND, 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
-                /* 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);
+        /* These devices are found and ready now, set the udev found bit. Note, this is also necessary to do
+         * on remove uevent, as some devlinks may be updated and now point to other device nodes. */
+        SET_FOREACH(d, ready_units)
+                device_update_found_one(d, DEVICE_FOUND_UDEV, DEVICE_FOUND_UDEV);
+
+        /* These devices may be nominally around, but not ready for us. Hence unset the udev bit, but leave
+         * the rest around. This may be redundant for remove uevent, but should be harmless. */
+        SET_FOREACH(d, not_ready_units)
+                device_update_found_one(d, DEVICE_NOT_FOUND, DEVICE_FOUND_UDEV);
 
         return 0;
 }
@@ -1205,7 +1182,7 @@ void device_found_node(Manager *m, const char *node, DeviceFound found, DeviceFo
                         return;
                 }
 
-                (void) device_setup_unit(m, dev, node, /* main = */ false); /* 'dev' may be NULL. */
+                (void) device_setup_unit(m, dev, node, /* main = */ false, NULL); /* 'dev' may be NULL. */
         }
 
         /* Update the device unit's state, should it exist */