Merge pull request #24522 from yuwata/core-device-drop-nonexistent-devlink-unit

core/device: drop nonexistent devlink units

3 files changed, 390 insertions, 191 deletions

diff --git a/src/core/device.c b/src/core/device.c
index d6710262a9..b3cf77d50c 100644
--- a/src/core/device.c
+++ b/src/core/device.c
@@ -32,6 +32,27 @@ static const UnitActiveState state_translation_table[_DEVICE_STATE_MAX] = {
 
 static int device_dispatch_io(sd_device_monitor *monitor, sd_device *dev, void *userdata);
 
+static int device_by_path(Manager *m, const char *path, Unit **ret) {
+        _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 r;
+
+        u = manager_get_unit(m, e);
+        if (!u)
+                return -ENOENT;
+
+        if (ret)
+                *ret = u;
+        return 0;
+}
+
 static void device_unset_sysfs(Device *d) {
         Hashmap *devices;
         Device *first;
@@ -214,9 +235,7 @@ static void device_update_found_by_sysfs(Manager *m, const char *sysfs, DeviceFo
 }
 
 static void device_update_found_by_name(Manager *m, const char *path, DeviceFound found, DeviceFound mask) {
-        _cleanup_free_ char *e = NULL;
         Unit *u;
-        int r;
 
         assert(m);
         assert(path);
@@ -224,12 +243,7 @@ static void device_update_found_by_name(Manager *m, const char *path, DeviceFoun
         if (mask == 0)
                 return;
 
-        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)
+        if (device_by_path(m, path, &u) < 0)
                 return;
 
         device_update_found_one(DEVICE(u), found, mask);
@@ -606,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;
@@ -683,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;
 }
@@ -692,6 +712,9 @@ static bool device_is_ready(sd_device *dev) {
 
         assert(dev);
 
+        if (device_for_action(dev, SD_DEVICE_REMOVE))
+                return false;
+
         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");
@@ -718,153 +741,176 @@ 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, 0, 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_safe(*alias)) {
+                        log_device_warning(dev, "The alias \"%s\" specified in SYSTEMD_ALIAS is not safe, 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);
+
+        /* Safety check: no unit should be in ready_units and not_ready_units simultaneously. */
+        Unit *u;
+        SET_FOREACH(u, *not_ready_units)
+                if (set_remove(*ready_units, u))
+                        log_unit_error(u, "Cannot active and deactive simultaneously. Deactivating.");
+
+        return 0;
 }
 
 static Unit *device_following(Unit *u) {
@@ -982,28 +1028,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;
@@ -1012,46 +1046,18 @@ fail:
         device_shutdown(m);
 }
 
-static void device_propagate_reload_by_sysfs(Manager *m, const char *sysfs) {
-        Device *l;
-        int r;
-
-        assert(m);
-        assert(sysfs);
-
-        l = hashmap_get(m->devices_by_sysfs, sysfs);
-        LIST_FOREACH(same_sysfs, d, l) {
-                if (d->state == DEVICE_DEAD)
-                        continue;
-
-                r = manager_propagate_reload(m, UNIT(d), JOB_REPLACE, NULL);
-                if (r < 0)
-                        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;
+static void device_propagate_reload(Manager *m, Device *d) {
         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;
+        assert(d);
 
-        if (DEVICE(u)->state == DEVICE_DEAD)
+        if (d->state == DEVICE_DEAD)
                 return;
 
-        r = manager_propagate_reload(m, u, JOB_REPLACE, NULL);
+        r = manager_propagate_reload(m, UNIT(d), JOB_REPLACE, NULL);
         if (r < 0)
-                log_unit_warning_errno(u, r, "Failed to propagate reload, ignoring: %m");
+                log_unit_warning_errno(UNIT(d), r, "Failed to propagate reload, ignoring: %m");
 }
 
 static void device_remove_old_on_move(Manager *m, sd_device *dev) {
@@ -1074,11 +1080,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);
@@ -1103,54 +1110,42 @@ static int device_dispatch_io(sd_device_monitor *monitor, sd_device *dev, void *
         /* A change event can signal that a device is becoming ready, in particular if the device is using
          * the SYSTEMD_READY logic in udev so we need to reach the else block of the following if, even for
          * 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");
-
-                ready = false;
-
-                (void) device_setup_devlink_units_on_remove(m, dev, &ready_devlinks);
-
-        } else {
-                ready = device_is_ready(dev);
-
-                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);
+        if (!IN_SET(action, SD_DEVICE_ADD, SD_DEVICE_REMOVE, SD_DEVICE_MOVE))
+                SET_FOREACH(d, ready_units)
+                        device_propagate_reload(m, d);
 
-                STRV_FOREACH(devlink, ready_devlinks)
-                        device_propagate_reload_by_name(m, *devlink);
-        }
-
-        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 */
-                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
-                /* 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);
+                 * 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);
+
+        /* 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);
 
-        STRV_FOREACH(devlink, ready_devlinks)
-                device_update_found_by_name(m, *devlink, 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;
 }
@@ -1194,7 +1189,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 */
diff --git a/test/units/testsuite-17.08.sh b/test/units/testsuite-17.08.sh
new file mode 100755
index 0000000000..f740b337f7
--- /dev/null
+++ b/test/units/testsuite-17.08.sh
@@ -0,0 +1,72 @@
+#!/usr/bin/env bash
+# SPDX-License-Identifier: LGPL-2.1-or-later
+set -ex
+set -o pipefail
+
+# shellcheck source=test/units/assert.sh
+. "$(dirname "$0")"/assert.sh
+
+# This is a test for issue #24518.
+
+mkdir -p /run/udev/rules.d/
+cat >/run/udev/rules.d/50-testsuite.rules <<EOF
+SUBSYSTEM=="mem", KERNEL=="null", OPTIONS="log_level=debug", TAG+="systemd"
+SUBSYSTEM=="mem", KERNEL=="null", ACTION=="add",    SYMLINK+="test/symlink-to-null-on-add",    ENV{SYSTEMD_ALIAS}+="/sys/test/alias-to-null-on-add"
+SUBSYSTEM=="mem", KERNEL=="null", ACTION=="change", SYMLINK+="test/symlink-to-null-on-change", ENV{SYSTEMD_ALIAS}+="/sys/test/alias-to-null-on-change"
+EOF
+
+udevadm control --reload
+
+udevadm trigger --settle --action add /dev/null
+for ((i = 0; i < 20; i++)); do
+    ((i == 0)) || sleep .5
+
+    (
+        systemctl -q is-active /dev/test/symlink-to-null-on-add
+        ! systemctl -q is-active /dev/test/symlink-to-null-on-change
+        systemctl -q is-active /sys/test/alias-to-null-on-add
+        ! systemctl -q is-active /sys/test/alias-to-null-on-change
+    ) && break
+done
+assert_rc 0 systemctl -q is-active /dev/test/symlink-to-null-on-add
+assert_rc 3 systemctl -q is-active /dev/test/symlink-to-null-on-change
+assert_rc 0 systemctl -q is-active /sys/test/alias-to-null-on-add
+assert_rc 3 systemctl -q is-active /sys/test/alias-to-null-on-change
+
+udevadm trigger --settle --action change /dev/null
+for ((i = 0; i < 20; i++)); do
+    ((i == 0)) || sleep .5
+
+    (
+        ! systemctl -q is-active /dev/test/symlink-to-null-on-add
+        systemctl -q is-active /dev/test/symlink-to-null-on-change
+        ! systemctl -q is-active /sys/test/alias-to-null-on-add
+        systemctl -q is-active /sys/test/alias-to-null-on-change
+    ) && break
+done
+assert_rc 3 systemctl -q is-active /dev/test/symlink-to-null-on-add
+assert_rc 0 systemctl -q is-active /dev/test/symlink-to-null-on-change
+assert_rc 3 systemctl -q is-active /sys/test/alias-to-null-on-add
+assert_rc 0 systemctl -q is-active /sys/test/alias-to-null-on-change
+
+udevadm trigger --settle --action add /dev/null
+for ((i = 0; i < 20; i++)); do
+    ((i == 0)) || sleep .5
+
+    (
+        systemctl -q is-active /dev/test/symlink-to-null-on-add
+        ! systemctl -q is-active /dev/test/symlink-to-null-on-change
+        systemctl -q is-active /sys/test/alias-to-null-on-add
+        ! systemctl -q is-active /sys/test/alias-to-null-on-change
+    ) && break
+done
+assert_rc 0 systemctl -q is-active /dev/test/symlink-to-null-on-add
+assert_rc 3 systemctl -q is-active /dev/test/symlink-to-null-on-change
+assert_rc 0 systemctl -q is-active /sys/test/alias-to-null-on-add
+assert_rc 3 systemctl -q is-active /sys/test/alias-to-null-on-change
+
+# cleanup
+rm -f /run/udev/rules.d/50-testsuite.rules
+udevadm control --reload
+
+exit 0
diff --git a/test/units/testsuite-64.sh b/test/units/testsuite-64.sh
index 9a2540ad0c..1bf894fa32 100755
--- a/test/units/testsuite-64.sh
+++ b/test/units/testsuite-64.sh
@@ -83,6 +83,93 @@ helper_check_udev_watch() {(
     done < <(find /run/udev/watch -type l)
 )}
 
+check_device_unit() {(
+    set +x
+
+    local log_level link links path syspath unit
+
+    log_level="${1?}"
+    path="${2?}"
+    unit=$(systemd-escape --path --suffix=device "$path")
+
+    syspath=$(systemctl show --value --property SysFSPath "$unit" 2>/dev/null)
+    if [[ -z "$syspath" ]]; then
+        [[ "$log_level" == 1 ]] && echo >&2 "ERROR: $unit not found."
+        return 1
+    fi
+
+    if [[ ! -L "$path" ]]; then
+        if [[ ! -d "$syspath" ]]; then
+            [[ "$log_level" == 1 ]] && echo >&2 "ERROR: $unit exists for $syspath but it does not exist."
+            return 1
+        fi
+        return 0
+    fi
+
+    if [[ ! -b "$path" && ! -c "$path" ]]; then
+        [[ "$log_level" == 1 ]] && echo >&2 "ERROR: invalid file type $path"
+        return 1
+    fi
+
+    read -r -a links < <(udevadm info -q symlink "$syspath" 2>/dev/null)
+    for link in "${links[@]}"; do
+        if [[ "/dev/$link" == "$path" ]]; then # DEVLINKS= given by -q symlink are relative to /dev
+            return 0
+        fi
+    done
+
+    read -r -a links < <(udevadm info "$syspath" | sed -ne '/SYSTEMD_ALIAS=/ { s/^E: SYSTEMD_ALIAS=//; p }' 2>/dev/null)
+    for link in "${links[@]}"; do
+        if [[ "$link" == "$path" ]]; then # SYSTEMD_ALIAS= are absolute
+            return 0
+        fi
+    done
+
+    [[ "$log_level" == 1 ]] && echo >&2 "ERROR: $unit exists for $syspath but it does not have the corresponding DEVLINKS or SYSTEMD_ALIAS."
+    return 1
+)}
+
+check_device_units() {(
+    set +x
+
+    local log_level path paths
+
+    log_level="${1?}"
+    shift
+    paths=("$@")
+
+    for path in "${paths[@]}"; do
+        if ! check_device_unit "$log_level" "$path"; then
+           return 1
+        fi
+    done
+
+    while read -r unit _; do
+        path=$(systemd-escape --path --unescape "$unit")
+        if ! check_device_unit "$log_level" "$path"; then
+           return 1
+        fi
+    done < <(systemctl list-units --all --type=device --no-legend dev-* | awk '{ print $1 }' | sed -e 's/\.device$//')
+
+    return 0
+)}
+
+helper_check_device_units() {(
+    set +x
+
+    local i
+
+    for ((i = 0; i < 20; i++)); do
+        (( i == 0 )) || sleep .5
+
+        if check_device_units 0 "$@"; then
+            return 0
+        fi
+    done
+
+    check_device_units 1 "$@"
+)}
+
 testcase_megasas2_basic() {
     lsblk -S
     [[ "$(lsblk --scsi --noheadings | wc -l)" -ge 128 ]]
@@ -160,6 +247,7 @@ EOF
         "/dev/disk/by-uuid/deadbeef-dead-dead-beef-111111111111"
     )
     udevadm wait --settle --timeout=30 "${part_links[@]}"
+    helper_check_device_units "${part_links[@]}"
 
     # Choose a random symlink to the failover data partition each time, for
     # a better coverage
@@ -191,6 +279,7 @@ EOF
 
         # Make sure all symlinks are still valid
         udevadm wait --settle --timeout=30 "${part_links[@]}"
+        helper_check_device_units "${part_links[@]}"
     done
 
     multipath -l "$path"
@@ -287,6 +376,7 @@ EOF
         fi
     done
 
+    helper_check_device_units
     rm -f "$rule" "$partscript"
 
     udevadm control --reload
@@ -317,16 +407,35 @@ testcase_lvm_basic() {
     mkfs.ext4 -L mylvpart1 "/dev/$vgroup/mypart1"
     udevadm wait --settle --timeout="$timeout" "/dev/disk/by-label/mylvpart1"
     helper_check_device_symlinks "/dev/disk" "/dev/$vgroup"
+    helper_check_device_units
+
+    # Rename partitions (see issue #24518)
+    lvm lvrename "/dev/$vgroup/mypart1" renamed1
+    lvm lvrename "/dev/$vgroup/mypart2" renamed2
+    udevadm wait --settle --timeout="$timeout" --removed "/dev/$vgroup/mypart1" "/dev/$vgroup/mypart2"
+    udevadm wait --settle --timeout="$timeout" "/dev/$vgroup/renamed1" "/dev/$vgroup/renamed2"
+    helper_check_device_symlinks "/dev/disk" "/dev/$vgroup"
+    helper_check_device_units
+
+    # Rename them back
+    lvm lvrename "/dev/$vgroup/renamed1" mypart1
+    lvm lvrename "/dev/$vgroup/renamed2" mypart2
+    udevadm wait --settle --timeout="$timeout" --removed "/dev/$vgroup/renamed1" "/dev/$vgroup/renamed2"
+    udevadm wait --settle --timeout="$timeout" "/dev/$vgroup/mypart1" "/dev/$vgroup/mypart2"
+    helper_check_device_symlinks "/dev/disk" "/dev/$vgroup"
+    helper_check_device_units
 
     # Disable the VG and check symlinks...
     lvm vgchange -an "$vgroup"
     udevadm wait --settle --timeout="$timeout" --removed "/dev/$vgroup" "/dev/disk/by-label/mylvpart1"
     helper_check_device_symlinks "/dev/disk"
+    helper_check_device_units
 
     # reenable the VG and check the symlinks again if all LVs are properly activated
     lvm vgchange -ay "$vgroup"
     udevadm wait --settle --timeout="$timeout" "/dev/$vgroup/mypart1" "/dev/$vgroup/mypart2" "/dev/disk/by-label/mylvpart1"
     helper_check_device_symlinks "/dev/disk" "/dev/$vgroup"
+    helper_check_device_units
 
     # Same as above, but now with more "stress"
     [[ -n "${ASAN_OPTIONS:-}" ]] && iterations=10 || iterations=50
@@ -337,6 +446,7 @@ testcase_lvm_basic() {
         if ((i % 5 == 0)); then
             udevadm wait --settle --timeout="$timeout" "/dev/$vgroup/mypart1" "/dev/$vgroup/mypart2" "/dev/disk/by-label/mylvpart1"
             helper_check_device_symlinks "/dev/disk" "/dev/$vgroup"
+            helper_check_device_units
         fi
     done
 
@@ -345,6 +455,7 @@ testcase_lvm_basic() {
     udevadm wait --settle --timeout="$timeout" --removed "/dev/$vgroup/mypart1"
     udevadm wait --timeout=0 "/dev/$vgroup/mypart2"
     helper_check_device_symlinks "/dev/disk" "/dev/$vgroup"
+    helper_check_device_units
 
     # Create & remove LVs in a loop, i.e. with more "stress"
     [[ -n "${ASAN_OPTIONS:-}" ]] && iterations=8 || iterations=16
@@ -364,6 +475,7 @@ testcase_lvm_basic() {
                 udevadm wait --settle --timeout="$timeout" --removed "/dev/$vgroup/looppart$part"
             done
             helper_check_device_symlinks "/dev/disk" "/dev/$vgroup"
+            helper_check_device_units
         fi
     done
 }
@@ -383,6 +495,7 @@ testcase_btrfs_basic() {
     udevadm wait --settle --timeout=30 "${devices[0]}" "/dev/disk/by-uuid/$uuid" "/dev/disk/by-label/$label"
     btrfs filesystem show
     helper_check_device_symlinks
+    helper_check_device_units
 
     echo "Multiple devices: using partitions, data: single, metadata: raid1"
     uuid="deadbeef-dead-dead-beef-000000000001"
@@ -400,6 +513,7 @@ EOF
     udevadm wait --settle --timeout=30 "/dev/disk/by-uuid/$uuid" "/dev/disk/by-label/$label"
     btrfs filesystem show
     helper_check_device_symlinks
+    helper_check_device_units
     wipefs -a -f "${devices[0]}"
     udevadm wait --settle --timeout=30 --removed /dev/disk/by-partlabel/diskpart{1..4}
 
@@ -415,6 +529,7 @@ EOF
     udevadm wait --settle --timeout=30 "/dev/disk/by-uuid/$uuid" "/dev/disk/by-label/$label"
     btrfs filesystem show
     helper_check_device_symlinks
+    helper_check_device_units
 
     echo "Multiple devices: using LUKS encrypted disks, data: raid1, metadata: raid1, mixed mode"
     uuid="deadbeef-dead-dead-beef-000000000003"
@@ -441,6 +556,7 @@ EOF
         systemctl start "systemd-cryptsetup@encbtrfs$i"
     done
     helper_check_device_symlinks
+    helper_check_device_units
     # Check if we have all necessary DM devices
     ls -l /dev/mapper/encbtrfs{0..3}
     # Create a multi-device btrfs filesystem on the LUKS devices
@@ -453,6 +569,7 @@ EOF
     udevadm wait --settle --timeout=30 "/dev/disk/by-uuid/$uuid" "/dev/disk/by-label/$label"
     btrfs filesystem show
     helper_check_device_symlinks
+    helper_check_device_units
     # Mount it and write some data to it we can compare later
     mount -t btrfs /dev/mapper/encbtrfs0 "$mpoint"
     echo "hello there" >"$mpoint/test"
@@ -461,6 +578,7 @@ EOF
     systemctl stop systemd-cryptsetup@encbtrfs{0..3}
     udevadm wait --settle --timeout=30 --removed "/dev/disk/by-uuid/$uuid"
     helper_check_device_symlinks
+    helper_check_device_units
     # Add the mount point to /etc/fstab and check if the device can be put together
     # automagically. The source device is the DM name of the first LUKS device
     # (from /etc/crypttab). We have to specify all LUKS devices manually, as
@@ -477,6 +595,7 @@ EOF
     udevadm wait --settle --timeout=30 "/dev/disk/by-uuid/$uuid" "/dev/disk/by-label/$label"
     btrfs filesystem show
     helper_check_device_symlinks
+    helper_check_device_units
     grep "hello there" "$mpoint/test"
     # Cleanup
     systemctl stop "${mpoint##*/}.mount"
@@ -525,6 +644,7 @@ testcase_iscsi_lvm() {
     iscsiadm --mode node --targetname "$target_name" --portal "$target_ip:$target_port" --login
     udevadm wait --settle --timeout=30 "${expected_symlinks[@]}"
     helper_check_device_symlinks
+    helper_check_device_units
     # Cleanup
     iscsiadm --mode node --targetname "$target_name" --portal "$target_ip:$target_port" --logout
     tgtadm --lld iscsi --op delete --mode target --tid=1
@@ -560,6 +680,7 @@ testcase_iscsi_lvm() {
     iscsiadm --mode node --targetname "$target_name" --portal "$target_ip:$target_port" --login
     udevadm wait --settle --timeout=30 "${expected_symlinks[@]}"
     helper_check_device_symlinks
+    helper_check_device_units
     # Add all iSCSI devices into a LVM volume group, create two logical volumes,
     # and check if necessary symlinks exist (and are valid)
     lvm pvcreate -y "${expected_symlinks[@]}"
@@ -574,6 +695,7 @@ testcase_iscsi_lvm() {
     mkfs.ext4 -L mylvpart1 "/dev/$vgroup/mypart1"
     udevadm wait --settle --timeout=30 "/dev/disk/by-label/mylvpart1"
     helper_check_device_symlinks "/dev/disk" "/dev/$vgroup"
+    helper_check_device_units
     # Disconnect the iSCSI devices and check all the symlinks
     iscsiadm --mode node --targetname "$target_name" --portal "$target_ip:$target_port" --logout
     # "Reset" the DM state, since we yanked the backing storage from under the LVM,
@@ -582,11 +704,13 @@ testcase_iscsi_lvm() {
     # The LVM and iSCSI related symlinks should be gone
     udevadm wait --settle --timeout=30 --removed "/dev/$vgroup" "/dev/disk/by-label/mylvpart1" "${expected_symlinks[@]}"
     helper_check_device_symlinks "/dev/disk"
+    helper_check_device_units
     # Reconnect the iSCSI devices and check if everything get detected correctly
     iscsiadm --mode discoverydb --type sendtargets --portal "$target_ip" --discover
     iscsiadm --mode node --targetname "$target_name" --portal "$target_ip:$target_port" --login
     udevadm wait --settle --timeout=30 "${expected_symlinks[@]}" "/dev/$vgroup/mypart1" "/dev/$vgroup/mypart2" "/dev/disk/by-label/mylvpart1"
     helper_check_device_symlinks "/dev/disk" "/dev/$vgroup"
+    helper_check_device_units
     # Cleanup
     iscsiadm --mode node --targetname "$target_name" --portal "$target_ip:$target_port" --logout
     tgtadm --lld iscsi --op delete --mode target --tid=2
@@ -687,6 +811,7 @@ testcase_mdadm_basic() {
         udevadm wait --settle --timeout=30 "${expected_symlinks[@]}"
     done
     helper_check_device_symlinks
+    helper_check_device_units
     # Cleanup
     mdadm -v --stop "$raid_dev"
     udevadm wait --settle --timeout=30 --removed "${expected_symlinks[@]}"
@@ -715,9 +840,11 @@ testcase_mdadm_basic() {
         udevadm wait --settle --timeout=30 "${expected_symlinks[@]}"
     done
     helper_check_device_symlinks
+    helper_check_device_units
     # Cleanup
     mdadm -v --stop "$raid_dev"
     udevadm wait --settle --timeout=30 --removed "${expected_symlinks[@]}"
+    helper_check_device_units
 
     echo "Mirror + parity raid (RAID 10) + multiple partitions"
     raid_name="mdmirpar"
@@ -763,10 +890,12 @@ EOF
         udevadm wait --settle --timeout=30 "${expected_symlinks[@]}"
     done
     helper_check_device_symlinks
+    helper_check_device_units
     # Cleanup
     mdadm -v --stop "$raid_dev"
     # Check if all expected symlinks were removed after the cleanup
     udevadm wait --settle --timeout=30 --removed "${expected_symlinks[@]}"
+    helper_check_device_units
 }
 
 testcase_mdadm_lvm() {
@@ -811,15 +940,18 @@ testcase_mdadm_lvm() {
     mdadm -v --stop "$raid_dev"
     udevadm wait --settle --timeout=30 --removed "${expected_symlinks[@]}"
     helper_check_device_symlinks
+    helper_check_device_units
     # Reassemble it and check if all required symlinks exist
     mdadm --assemble "$raid_dev" --name "$raid_name" -v
     udevadm wait --settle --timeout=30 "${expected_symlinks[@]}"
     helper_check_device_symlinks
+    helper_check_device_units
     # Cleanup
     lvm vgchange -an "$vgroup"
     mdadm -v --stop "$raid_dev"
     # Check if all expected symlinks were removed after the cleanup
     udevadm wait --settle --timeout=30 --removed "${expected_symlinks[@]}"
+    helper_check_device_units
 }
 
 : >/failed