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 f91ee3e1bdb31fe205fe793acec20bc3a925553a..1eb5d8fb53f6c8d84decd2bc38f19061b334ed61 100644 (file)
--- 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 ce016e88db3a811d2e40e4ad3bbca5e4e385664f..43a1b846f48cd4487bf1b1fcb3b601e62fb05b3c 100644 (file)
--- 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 d41399718958891864fa4c911b5563ac58cde9fe..02be7a0b3c671815af54e23d142e150f1837e46e 100644 (file)
--- 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 859b169f1b64d64c20d58ee4ac98e7260690e5e6..b2291c85cae78c87e66137ba72bb75857f46fd7b 100644 (file)
--- 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;
         }