From: Yu Watanabe <watanabe.yu+github@gmail.com>
Date: Fri, 15 Apr 2022 03:23:48 +0000 (+0900)
Subject: udev: make newer event also blocked by DEVPATH_OLD
X-Git-Tag: v252-rc1~740^2~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a1af9668ec78fc11a2e7ac942397eac0c8fceced;p=thirdparty%2Fsystemd.git

udev: make newer event also blocked by DEVPATH_OLD

Previously, a device has DEVPATH_OLD is blocked by a previous event
whose devpath is equivalent to the DEVPATH_OLD.

This extends the condtion.

1. an event has DEVPATH_OLD is blocked by a previous event whose
   devpath is a parent of, child of, or equivalent to the DEVPATH_OLD.

2. an event is blocked by a previous event whose DEVPATH_OLD is a
   parent of, child of, or equivalent to the devpath of the new event.

I am not sure such check is really necessary. But, the cost of the check
is expected to be extremely small, as device renaming does not occur so
frequently. Hence, it should not introduce any significant performance
regression.
---

diff --git a/src/shared/udev-util.c b/src/shared/udev-util.c
index f91ee3e1bdb..1eb5d8fb53f 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 ce016e88db3..43a1b846f48 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 d4139971895..02be7a0b3c6 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 859b169f1b6..b2291c85cae 100644
--- a/src/udev/udevd.c
+++ b/src/udev/udevd.c
@@ -856,7 +856,6 @@ static int event_run(Event *event) {
 
 static int event_is_blocked(Event *event) {
         Event *loop_event = NULL;
-        size_t devpath_len;
         int r;
 
         /* lookup event for identical, parent, child device */
@@ -903,12 +902,8 @@ static int event_is_blocked(Event *event) {
         assert(loop_event->seqnum > event->blocker_seqnum &&
                loop_event->seqnum < event->seqnum);
 
-        devpath_len = strlen(event->devpath);
-
         /* check if queue contains events we depend on */
         LIST_FOREACH(event, e, loop_event) {
-                size_t loop_devpath_len, common;
-
                 loop_event = e;
 
                 /* found ourself, no later event can block us */
@@ -918,29 +913,9 @@ static int event_is_blocked(Event *event) {
                 if (streq_ptr(loop_event->id, event->id))
                         break;
 
-                /* check our old name */
-                if (event->devpath_old && streq(event->devpath_old, loop_event->devpath))
-                        break;
-
-                loop_devpath_len = strlen(loop_event->devpath);
-
-                /* compare devpath */
-                common = MIN(devpath_len, loop_devpath_len);
-
-                /* one devpath is contained in the other? */
-                if (!strneq(event->devpath, loop_event->devpath, common))
-                        continue;
-
-                /* identical device event found */
-                if (devpath_len == loop_devpath_len)
-                        break;
-
-                /* parent device event found */
-                if (event->devpath[common] == '/')
-                        break;
-
-                /* child device event found */
-                if (loop_event->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;
         }