Merge pull request #23088 from yuwata/udev-event-blocker

udev: cleanups for event blocker

4 files changed, 59 insertions, 76 deletions

diff --git a/src/shared/udev-util.c b/src/shared/udev-util.c
index f91ee3e1bd..1eb5d8fb53 100644
--- a/src/shared/udev-util.c
+++ b/src/shared/udev-util.c
@@ -568,6 +568,19 @@ int udev_resolve_subsys_kernel(const char *string, char *result, size_t maxsize,
         return 0;
 }
 
+bool devpath_conflict(const char *a, const char *b) {
+        /* This returns true when two paths are equivalent, or one is a child of another. */
+
+        if (!a || !b)
+                return false;
+
+        for (; *a != '\0' && *b != '\0'; a++, b++)
+                if (*a != *b)
+                        return false;
+
+        return *a == '/' || *b == '/' || *a == *b;
+}
+
 int udev_queue_is_empty(void) {
         return access("/run/udev/queue", F_OK) < 0 ?
                 (errno == ENOENT ? true : -errno) : false;
diff --git a/src/shared/udev-util.h b/src/shared/udev-util.h
index ce016e88db..43a1b846f4 100644
--- a/src/shared/udev-util.h
+++ b/src/shared/udev-util.h
@@ -50,6 +50,8 @@ size_t udev_replace_ifname(char *str);
 size_t udev_replace_chars(char *str, const char *allow);
 int udev_resolve_subsys_kernel(const char *string, char *result, size_t maxsize, bool read_value);
 
+bool devpath_conflict(const char *a, const char *b);
+
 int udev_queue_is_empty(void);
 int udev_queue_init(void);
 
diff --git a/src/test/test-udev-util.c b/src/test/test-udev-util.c
index 35b20dc172..1db2dad4ff 100644
--- a/src/test/test-udev-util.c
+++ b/src/test/test-udev-util.c
@@ -155,4 +155,17 @@ TEST(udev_resolve_subsys_kernel) {
         test_udev_resolve_subsys_kernel_one("[net/lo]/address", true, 0, "00:00:00:00:00:00");
 }
 
+TEST(devpath_conflict) {
+        assert_se(!devpath_conflict(NULL, NULL));
+        assert_se(!devpath_conflict(NULL, "/devices/pci0000:00/0000:00:1c.4"));
+        assert_se(!devpath_conflict("/devices/pci0000:00/0000:00:1c.4", NULL));
+        assert_se(!devpath_conflict("/devices/pci0000:00/0000:00:1c.4", "/devices/pci0000:00/0000:00:00.0"));
+        assert_se(!devpath_conflict("/devices/virtual/net/veth99", "/devices/virtual/net/veth999"));
+
+        assert_se(devpath_conflict("/devices/pci0000:00/0000:00:1c.4", "/devices/pci0000:00/0000:00:1c.4"));
+        assert_se(devpath_conflict("/devices/pci0000:00/0000:00:1c.4", "/devices/pci0000:00/0000:00:1c.4/0000:3c:00.0"));
+        assert_se(devpath_conflict("/devices/pci0000:00/0000:00:1c.4/0000:3c:00.0/nvme/nvme0/nvme0n1",
+                                   "/devices/pci0000:00/0000:00:1c.4/0000:3c:00.0/nvme/nvme0/nvme0n1/nvme0n1p1"));
+}
+
 DEFINE_TEST_MAIN(LOG_INFO);
diff --git a/src/udev/udevd.c b/src/udev/udevd.c
index 1994b6b2d5..78c37b905e 100644
--- a/src/udev/udevd.c
+++ b/src/udev/udevd.c
@@ -131,6 +131,10 @@ typedef struct Event {
         sd_device_action_t action;
         uint64_t seqnum;
         uint64_t blocker_seqnum;
+        const char *id;
+        const char *devpath;
+        const char *devpath_old;
+        const char *devnode;
         usec_t retry_again_next_usec;
         usec_t retry_again_timeout_usec;
 
@@ -852,12 +856,8 @@ static int event_run(Event *event) {
 }
 
 static int event_is_blocked(Event *event) {
-        const char *subsystem, *devpath, *devpath_old = NULL;
-        dev_t devnum = makedev(0, 0);
         Event *loop_event = NULL;
-        size_t devpath_len;
-        int r, ifindex = 0;
-        bool is_block;
+        int r;
 
         /* lookup event for identical, parent, child device */
 
@@ -903,89 +903,23 @@ static int event_is_blocked(Event *event) {
         assert(loop_event->seqnum > event->blocker_seqnum &&
                loop_event->seqnum < event->seqnum);
 
-        r = sd_device_get_subsystem(event->dev, &subsystem);
-        if (r < 0)
-                return r;
-
-        is_block = streq(subsystem, "block");
-
-        r = sd_device_get_devpath(event->dev, &devpath);
-        if (r < 0)
-                return r;
-
-        devpath_len = strlen(devpath);
-
-        r = sd_device_get_property_value(event->dev, "DEVPATH_OLD", &devpath_old);
-        if (r < 0 && r != -ENOENT)
-                return r;
-
-        r = sd_device_get_devnum(event->dev, &devnum);
-        if (r < 0 && r != -ENOENT)
-                return r;
-
-        r = sd_device_get_ifindex(event->dev, &ifindex);
-        if (r < 0 && r != -ENOENT)
-                return r;
-
         /* check if queue contains events we depend on */
         LIST_FOREACH(event, e, loop_event) {
-                size_t loop_devpath_len, common;
-                const char *loop_devpath;
-
                 loop_event = e;
 
                 /* found ourself, no later event can block us */
                 if (loop_event->seqnum >= event->seqnum)
                         goto no_blocker;
 
-                /* check major/minor */
-                if (major(devnum) != 0) {
-                        const char *s;
-                        dev_t d;
-
-                        if (sd_device_get_subsystem(loop_event->dev, &s) < 0)
-                                continue;
-
-                        if (sd_device_get_devnum(loop_event->dev, &d) >= 0 &&
-                            devnum == d && is_block == streq(s, "block"))
-                                break;
-                }
-
-                /* check network device ifindex */
-                if (ifindex > 0) {
-                        int i;
-
-                        if (sd_device_get_ifindex(loop_event->dev, &i) >= 0 &&
-                            ifindex == i)
-                                break;
-                }
-
-                if (sd_device_get_devpath(loop_event->dev, &loop_devpath) < 0)
-                        continue;
-
-                /* check our old name */
-                if (devpath_old && streq(devpath_old, loop_devpath))
-                        break;
-
-                loop_devpath_len = strlen(loop_devpath);
-
-                /* compare devpath */
-                common = MIN(devpath_len, loop_devpath_len);
-
-                /* one devpath is contained in the other? */
-                if (!strneq(devpath, loop_devpath, common))
-                        continue;
-
-                /* identical device event found */
-                if (devpath_len == loop_devpath_len)
+                if (streq_ptr(loop_event->id, event->id))
                         break;
 
-                /* parent device event found */
-                if (devpath[common] == '/')
+                if (devpath_conflict(event->devpath, loop_event->devpath) ||
+                    devpath_conflict(event->devpath, loop_event->devpath_old) ||
+                    devpath_conflict(event->devpath_old, loop_event->devpath))
                         break;
 
-                /* child device event found */
-                if (loop_devpath[common] == '/')
+                if (event->devnode && streq_ptr(event->devnode, loop_event->devnode))
                         break;
         }
 
@@ -1134,6 +1068,7 @@ static int event_queue_assume_block_device_unlocked(Manager *manager, sd_device
 }
 
 static int event_queue_insert(Manager *manager, sd_device *dev) {
+        const char *devpath, *devpath_old = NULL, *id = NULL, *devnode = NULL;
         sd_device_action_t action;
         uint64_t seqnum;
         Event *event;
@@ -1154,6 +1089,22 @@ static int event_queue_insert(Manager *manager, sd_device *dev) {
         if (r < 0)
                 return r;
 
+        r = sd_device_get_devpath(dev, &devpath);
+        if (r < 0)
+                return r;
+
+        r = sd_device_get_property_value(dev, "DEVPATH_OLD", &devpath_old);
+        if (r < 0 && r != -ENOENT)
+                return r;
+
+        r = device_get_device_id(dev, &id);
+        if (r < 0 && r != -ENOENT)
+                return r;
+
+        r = sd_device_get_devname(dev, &devnode);
+        if (r < 0 && r != -ENOENT)
+                return r;
+
         event = new(Event, 1);
         if (!event)
                 return -ENOMEM;
@@ -1163,6 +1114,10 @@ static int event_queue_insert(Manager *manager, sd_device *dev) {
                 .dev = sd_device_ref(dev),
                 .seqnum = seqnum,
                 .action = action,
+                .id = id,
+                .devpath = devpath,
+                .devpath_old = devpath_old,
+                .devnode = devnode,
                 .state = EVENT_QUEUED,
         };