Merge pull request #28255 from yuwata/sd-device-fix-clone

sd-device: fix device_clone_with_db()

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

@@ -620,51 +620,56 @@ int device_get_devlink_priority(sd_device *device, int *ret) {
         return 0;
 }
 
-static int device_shallow_clone(sd_device *device, sd_device **ret) {
+int device_clone_with_db(sd_device *device, sd_device **ret) {
         _cleanup_(sd_device_unrefp) sd_device *dest = NULL;
-        const char *val = NULL;
+        const char *key, *val;
         int r;
 
         assert(device);
         assert(ret);
 
+        /* The device may be already removed. Let's copy minimal set of information that was obtained through
+         * netlink socket. */
+
         r = device_new_aux(&dest);
         if (r < 0)
                 return r;
 
+        /* Seal device to prevent reading the uevent file, as the device may have been already removed. */
+        dest->sealed = true;
+
+        /* Copy syspath, then also devname, sysname or sysnum can be obtained. */
         r = device_set_syspath(dest, device->syspath, false);
         if (r < 0)
                 return r;
 
-        (void) sd_device_get_subsystem(device, &val);
-        r = device_set_subsystem(dest, val);
-        if (r < 0)
-                return r;
-        if (streq_ptr(val, "drivers")) {
-                r = free_and_strdup(&dest->driver_subsystem, device->driver_subsystem);
+        /* Copy other information stored in database. Here, do not use FOREACH_DEVICE_PROPERTY() and
+         * sd_device_get_property_value(), as they calls device_properties_prepare() ->
+         * device_read_uevent_file(), but as commented in the above, the device may be already removed and
+         * reading uevent file may fail. */
+        ORDERED_HASHMAP_FOREACH_KEY(val, key, device->properties) {
+                if (streq(key, "MINOR"))
+                        continue;
+
+                if (streq(key, "MAJOR")) {
+                        const char *minor = NULL;
+
+                        minor = ordered_hashmap_get(device->properties, "MINOR");
+                        r = device_set_devnum(dest, val, minor);
+                } else
+                        r = device_amend(dest, key, val);
                 if (r < 0)
                         return r;
+
+                if (streq(key, "SUBSYSTEM") && streq(val, "drivers")) {
+                        r = free_and_strdup(&dest->driver_subsystem, device->driver_subsystem);
+                        if (r < 0)
+                                return r;
+                }
         }
 
-        /* The device may be already removed. Let's copy minimal set of information to make
-         * device_get_device_id() work without uevent file. */
-
-        if (sd_device_get_property_value(device, "IFINDEX", &val) >= 0) {
-                r = device_set_ifindex(dest, val);
-                if (r < 0)
-                        return r;
-        }
-
-        if (sd_device_get_property_value(device, "MAJOR", &val) >= 0) {
-                const char *minor = NULL;
-
-                (void) sd_device_get_property_value(device, "MINOR", &minor);
-                r = device_set_devnum(dest, val, minor);
-                if (r < 0)
-                        return r;
-        }
-
-        r = device_read_uevent_file(dest);
+        /* Finally, read the udev database. */
+        r = device_read_db_internal(dest, /* force = */ true);
         if (r < 0)
                 return r;
 
@@ -672,27 +677,6 @@ static int device_shallow_clone(sd_device *device, sd_device **ret) {
         return 0;
 }
 
-int device_clone_with_db(sd_device *device, sd_device **ret) {
-        _cleanup_(sd_device_unrefp) sd_device *dest = NULL;
-        int r;
-
-        assert(device);
-        assert(ret);
-
-        r = device_shallow_clone(device, &dest);
-        if (r < 0)
-                return r;
-
-        r = device_read_db(dest);
-        if (r < 0)
-                return r;
-
-        dest->sealed = true;
-
-        *ret = TAKE_PTR(dest);
-        return 0;
-}
-
 void device_cleanup_tags(sd_device *device) {
         assert(device);
 

test/units/testsuite-64.sh

@@ -444,10 +444,14 @@ EOF
 }
 
 testcase_simultaneous_events_2() {
-    local disk expected i iterations key link num_part part partscript target timeout
+    local disk expected i iterations key link num_part part script_dir target timeout
     local -a devices symlinks
     local -A running
 
+    script_dir="$(mktemp --directory "/tmp/test-udev-storage.script.XXXXXXXXXX")"
+    # shellcheck disable=SC2064
+    trap "rm -rf '$script_dir'" RETURN
+
     if [[ -v ASAN_OPTIONS || "$(systemd-detect-virt -v)" == "qemu" ]]; then
         num_part=20
         iterations=1
@@ -469,13 +473,11 @@ testcase_simultaneous_events_2() {
         devices+=("$target")
     done
 
-    symlinks=("/dev/disk/by-partlabel/testlabel")
-
-    partscript="$(mktemp)"
-
-    cat >"$partscript" <<EOF
-$(for ((part = 1; part <= num_part; part++)); do printf 'name="testlabel", size=1M\n'; done)
+    for ((i = 1; i <= iterations; i++)); do
+        cat >"$script_dir/partscript-$i" <<EOF
+$(for ((part = 1; part <= num_part; part++)); do printf 'name="testlabel-%d", size=1M\n' "$i"; done)
 EOF
+    done
 
     echo "## $iterations iterations start: $(date '+%H:%M:%S.%N')"
     for ((i = 1; i <= iterations; i++)); do
@@ -491,7 +493,7 @@ EOF
         done
 
         for disk in {0..9}; do
-            udevadm lock --device="${devices[$disk]}" sfdisk -q -X gpt "${devices[$disk]}" <"$partscript" &
+            udevadm lock --device="${devices[$disk]}" sfdisk -q -X gpt "${devices[$disk]}" <"$script_dir/partscript-$i" &
             running[$disk]=$!
         done
 
@@ -500,7 +502,7 @@ EOF
             unset "running[$key]"
         done
 
-        udevadm wait --settle --timeout="$timeout" "${devices[@]}" "${symlinks[@]}"
+        udevadm wait --settle --timeout="$timeout" "${devices[@]}" "/dev/disk/by-partlabel/testlabel-$i"
     done
     echo "## $iterations iterations end: $(date '+%H:%M:%S.%N')"
 }