Merge pull request #24757 from yuwata/sd-device-get-child-first

sd-device: introduce sd_device_get_child_first() and _next()

src/libsystemd/libsystemd.sym

@@ -785,6 +785,8 @@ global:
         sd_bus_error_setfv;
 
         sd_device_new_child;
+        sd_device_get_child_first;
+        sd_device_get_child_next;
         sd_device_monitor_set_description;
         sd_device_monitor_get_description;
 

src/libsystemd/sd-device/device-internal.h

@@ -47,6 +47,10 @@ struct sd_device {
         uint64_t devlinks_iterator_generation; /* generation when iteration was started */
         int devlink_priority;
 
+        Hashmap *children;
+        Iterator children_iterator;
+        bool children_enumerated;
+
         int ifindex;
         char *devtype;
         char *devname;

src/libsystemd/sd-device/device-util.h

@@ -38,6 +38,17 @@
              devlink;                                   \
              devlink = sd_device_get_devlink_next(device))
 
+#define _FOREACH_DEVICE_CHILD(device, child, suffix_ptr)                \
+        for (child = sd_device_get_child_first(device, suffix_ptr);     \
+             child;                                                     \
+             child = sd_device_get_child_next(device, suffix_ptr))
+
+#define FOREACH_DEVICE_CHILD(device, child)                             \
+        _FOREACH_DEVICE_CHILD(device, child, NULL)
+
+#define FOREACH_DEVICE_CHILD_WITH_SUFFIX(device, child, suffix)         \
+        _FOREACH_DEVICE_CHILD(device, child, &suffix)
+
 #define FOREACH_DEVICE(enumerator, device)                               \
         for (device = sd_device_enumerator_get_device_first(enumerator); \
              device;                                                     \

src/libsystemd/sd-device/sd-device.c

@@ -78,6 +78,7 @@ static sd_device *device_free(sd_device *device) {
         set_free(device->all_tags);
         set_free(device->current_tags);
         set_free(device->devlinks);
+        hashmap_free(device->children);
 
         return mfree(device);
 }
@@ -870,8 +871,129 @@ _public_ int sd_device_get_syspath(sd_device *device, const char **ret) {
         return 0;
 }
 
+DEFINE_PRIVATE_HASH_OPS_FULL(
+        device_by_path_hash_ops,
+        char, path_hash_func, path_compare, free,
+        sd_device, sd_device_unref);
+
+static int device_enumerate_children_internal(sd_device *device, const char *subdir, Set **stack, Hashmap **children) {
+        _cleanup_closedir_ DIR *dir = NULL;
+        int r;
+
+        assert(device);
+        assert(stack);
+        assert(children);
+
+        r = device_opendir(device, subdir, &dir);
+        if (r < 0)
+                return r;
+
+        FOREACH_DIRENT_ALL(de, dir, return -errno) {
+                _cleanup_(sd_device_unrefp) sd_device *child = NULL;
+                _cleanup_free_ char *p = NULL;
+
+                if (dot_or_dot_dot(de->d_name))
+                        continue;
+
+                if (!IN_SET(de->d_type, DT_LNK, DT_DIR))
+                        continue;
+
+                if (subdir)
+                        p = path_join(subdir, de->d_name);
+                else
+                        p = strdup(de->d_name);
+                if (!p)
+                        return -ENOMEM;
+
+                /* Try to create child device. */
+                r = sd_device_new_child(&child, device, p);
+                if (r >= 0) {
+                        /* OK, this is a child device, saving it. */
+                        r = hashmap_ensure_put(children, &device_by_path_hash_ops, p, child);
+                        if (r < 0)
+                                return r;
+
+                        TAKE_PTR(p);
+                        TAKE_PTR(child);
+                } else if (r == -ENODEV) {
+                        /* This is not a child device. Push the sub-directory into stack, and read it later. */
+
+                        if (de->d_type == DT_LNK)
+                                /* Do not follow symlinks, otherwise, we will enter an infinite loop, e.g.,
+                                 * /sys/class/block/nvme0n1/subsystem/nvme0n1/subsystem/nvme0n1/subsystem/… */
+                                continue;
+
+                        r = set_ensure_consume(stack, &path_hash_ops_free, TAKE_PTR(p));
+                        if (r < 0)
+                                return r;
+                } else
+                        return r;
+        }
+
+        return 0;
+}
+
+static int device_enumerate_children(sd_device *device) {
+        _cleanup_hashmap_free_ Hashmap *children = NULL;
+        _cleanup_set_free_ Set *stack = NULL;
+        int r;
+
+        assert(device);
+
+        if (device->children_enumerated)
+                return 0; /* Already enumerated. */
+
+        r = device_enumerate_children_internal(device, NULL, &stack, &children);
+        if (r < 0)
+                return r;
+
+        for (;;) {
+                _cleanup_free_ char *subdir = NULL;
+
+                subdir = set_steal_first(stack);
+                if (!subdir)
+                        break;
+
+                r = device_enumerate_children_internal(device, subdir, &stack, &children);
+                if (r < 0)
+                        return r;
+        }
+
+        device->children_enumerated = true;
+        device->children = TAKE_PTR(children);
+        return 1; /* Enumerated. */
+}
+
+_public_ sd_device *sd_device_get_child_first(sd_device *device, const char **ret_suffix) {
+        int r;
+
+        assert(device);
+
+        r = device_enumerate_children(device);
+        if (r < 0) {
+                log_device_debug_errno(device, r, "sd-device: failed to enumerate child devices: %m");
+                if (ret_suffix)
+                        *ret_suffix = NULL;
+                return NULL;
+        }
+
+        device->children_iterator = ITERATOR_FIRST;
+
+        return sd_device_get_child_next(device, ret_suffix);
+}
+
+_public_ sd_device *sd_device_get_child_next(sd_device *device, const char **ret_suffix) {
+        sd_device *child;
+
+        assert(device);
+
+        hashmap_iterate(device->children, &device->children_iterator, (void**) &child, (const void**) ret_suffix);
+        return child;
+}
+
 _public_ int sd_device_new_child(sd_device **ret, sd_device *device, const char *suffix) {
         _cleanup_free_ char *path = NULL;
+        sd_device *child;
         const char *s;
         int r;
 
@@ -879,9 +1001,16 @@ _public_ int sd_device_new_child(sd_device **ret, sd_device *device, const char
         assert_return(device, -EINVAL);
         assert_return(suffix, -EINVAL);
 
-        if (!path_is_normalized(suffix))
+        if (!path_is_safe(suffix))
                 return -EINVAL;
 
+        /* If we have already enumerated children, try to find the child from the cache. */
+        child = hashmap_get(device->children, suffix);
+        if (child) {
+                *ret = sd_device_ref(child);
+                return 0;
+        }
+
         r = sd_device_get_syspath(device, &s);
         if (r < 0)
                 return r;

src/libsystemd/sd-device/test-sd-device.c

@@ -472,6 +472,57 @@ TEST(sd_device_enumerator_add_match_parent) {
         }
 }
 
+TEST(sd_device_get_child) {
+        _cleanup_(sd_device_enumerator_unrefp) sd_device_enumerator *e = NULL;
+        sd_device *dev;
+        int r;
+
+        assert_se(sd_device_enumerator_new(&e) >= 0);
+        assert_se(sd_device_enumerator_allow_uninitialized(e) >= 0);
+        /* See comments in TEST(sd_device_enumerator_devices). */
+        assert_se(sd_device_enumerator_add_match_subsystem(e, "bdi", false) >= 0);
+        assert_se(sd_device_enumerator_add_nomatch_sysname(e, "loop*") >= 0);
+        assert_se(sd_device_enumerator_add_match_subsystem(e, "net", false) >= 0);
+
+        if (!slow_tests_enabled())
+                assert_se(sd_device_enumerator_add_match_subsystem(e, "block", true) >= 0);
+
+        FOREACH_DEVICE(e, dev) {
+                const char *syspath, *parent_syspath, *expected_suffix, *suffix;
+                sd_device *parent, *child;
+                bool found = false;
+
+                assert_se(sd_device_get_syspath(dev, &syspath) >= 0);
+
+                r = sd_device_get_parent(dev, &parent);
+                if (r < 0) {
+                        assert_se(ERRNO_IS_DEVICE_ABSENT(r));
+                        continue;
+                }
+
+                assert_se(sd_device_get_syspath(parent, &parent_syspath) >= 0);
+                assert_se(expected_suffix = path_startswith(syspath, parent_syspath));
+
+                log_debug("> %s", syspath);
+
+                FOREACH_DEVICE_CHILD_WITH_SUFFIX(parent, child, suffix) {
+                        const char *s;
+
+                        assert_se(child);
+                        assert_se(suffix);
+
+                        if (!streq(suffix, expected_suffix))
+                                continue;
+
+                        assert_se(sd_device_get_syspath(child, &s) >= 0);
+                        assert_se(streq(s, syspath));
+                        found = true;
+                        break;
+                }
+                assert_se(found);
+        }
+}
+
 TEST(sd_device_new_from_nulstr) {
         const char *devlinks =
                 "/dev/disk/by-partuuid/1290d63a-42cc-4c71-b87c-xxxxxxxxxxxx\0"

src/network/networkd-sriov.c

@@ -2,7 +2,7 @@
  * Copyright © 2020 VMware, Inc. */
 
 #include "device-enumerator-private.h"
-#include "dirent-util.h"
+#include "device-util.h"
 #include "fd-util.h"
 #include "networkd-link.h"
 #include "networkd-manager.h"
@@ -229,9 +229,8 @@ static int link_set_sr_iov_phys_port(Link *link) {
 }
 
 static int link_set_sr_iov_virt_ports(Link *link) {
-        const char *dev_port, *pci_syspath;
-        _cleanup_closedir_ DIR *dir = NULL;
-        sd_device *pci_dev;
+        const char *dev_port, *name;
+        sd_device *pci_dev, *child;
         int r;
 
         assert(link);
@@ -250,28 +249,16 @@ static int link_set_sr_iov_virt_ports(Link *link) {
         if (r < 0)
                 return r;
 
-        r = sd_device_get_syspath(pci_dev, &pci_syspath);
-        if (r < 0)
-                return r;
-
-        dir = opendir(pci_syspath);
-        if (!dir)
-                return -errno;
-
-        FOREACH_DIRENT_ALL(de, dir, break) {
-                _cleanup_(sd_device_unrefp) sd_device *pci_virtfn_dev = NULL;
-
-                if (de->d_type != DT_LNK)
-                        continue;
+        FOREACH_DEVICE_CHILD_WITH_SUFFIX(pci_dev, child, name) {
+                const char *n;
 
                 /* Accept name prefixed with "virtfn", but refuse "virtfn" itself. */
-                if (isempty(startswith(de->d_name, "virtfn")))
+                n = startswith(name, "virtfn");
+                if (isempty(n) || !in_charset(n, DIGITS))
                         continue;
 
-                if (sd_device_new_child(&pci_virtfn_dev, pci_dev, de->d_name) < 0)
-                        continue;
-
-                if (find_ifindex_from_pci_dev_port(pci_virtfn_dev, dev_port) < 0)
+                r = find_ifindex_from_pci_dev_port(child, dev_port);
+                if (r < 0)
                         continue;
 
                 if (manager_update_sr_iov_ifindices(link->manager, link->ifindex, r) < 0)

src/systemd/sd-device.h

@@ -100,6 +100,8 @@ const char *sd_device_get_property_first(sd_device *device, const char **value);
 const char *sd_device_get_property_next(sd_device *device, const char **value);
 const char *sd_device_get_sysattr_first(sd_device *device);
 const char *sd_device_get_sysattr_next(sd_device *device);
+sd_device *sd_device_get_child_first(sd_device *device, const char **ret_suffix);
+sd_device *sd_device_get_child_next(sd_device *device, const char **ret_suffix);
 
 int sd_device_has_tag(sd_device *device, const char *tag);
 int sd_device_has_current_tag(sd_device *device, const char *tag);

src/udev/udev-builtin-net_id.c

@@ -99,8 +99,8 @@ static sd_device *skip_virtio(sd_device *dev) {
 
 static int get_virtfn_info(sd_device *pcidev, sd_device **ret_physfn_pcidev, char **ret_suffix) {
         _cleanup_(sd_device_unrefp) sd_device *physfn_pcidev = NULL;
-        const char *syspath;
-        _cleanup_closedir_ DIR *dir = NULL;
+        const char *syspath, *name;
+        sd_device *child;
         int r;
 
         assert(pcidev);
@@ -117,22 +117,15 @@ static int get_virtfn_info(sd_device *pcidev, sd_device **ret_physfn_pcidev, cha
                 return r;
 
         /* Find the virtual function number by finding the right virtfn link. */
-        r = device_opendir(physfn_pcidev, NULL, &dir);
-        if (r < 0)
-                return r;
-
-        FOREACH_DIRENT_ALL(de, dir, break) {
-                _cleanup_(sd_device_unrefp) sd_device *virtfn_pcidev = NULL;
+        FOREACH_DEVICE_CHILD_WITH_SUFFIX(physfn_pcidev, child, name) {
                 const char *n, *s;
 
-                n = startswith(de->d_name, "virtfn");
-                if (isempty(n))
+                /* Only accepts e.g. virtfn0, virtfn1, and so on. */
+                n = startswith(name, "virtfn");
+                if (isempty(n) || !in_charset(n, DIGITS))
                         continue;
 
-                if (sd_device_new_child(&virtfn_pcidev, physfn_pcidev, de->d_name) < 0)
-                        continue;
-
-                if (sd_device_get_syspath(virtfn_pcidev, &s) < 0)
+                if (sd_device_get_syspath(child, &s) < 0)
                         continue;
 
                 if (streq(s, syspath)) {
@@ -142,7 +135,7 @@ static int get_virtfn_info(sd_device *pcidev, sd_device **ret_physfn_pcidev, cha
                         if (!suffix)
                                 return -ENOMEM;
 
-                        *ret_physfn_pcidev = TAKE_PTR(physfn_pcidev);
+                        *ret_physfn_pcidev = sd_device_ref(child);
                         *ret_suffix = suffix;
                         return 0;
                 }