diff --git a/src/libsystemd/sd-id128/id128-util.c b/src/libsystemd/sd-id128/id128-util.c index a27f4bf7427..a009a110a96 100644 --- a/src/libsystemd/sd-id128/id128-util.c +++ b/src/libsystemd/sd-id128/id128-util.c @@ -173,6 +173,7 @@ sd_id128_t id128_make_v4_uuid(sd_id128_t id) { } DEFINE_HASH_OPS(id128_hash_ops, sd_id128_t, id128_hash_func, id128_compare_func); +DEFINE_HASH_OPS_WITH_KEY_DESTRUCTOR(id128_hash_ops_free, sd_id128_t, id128_hash_func, id128_compare_func, free); int id128_get_product(sd_id128_t *ret) { sd_id128_t uuid; diff --git a/src/libsystemd/sd-id128/id128-util.h b/src/libsystemd/sd-id128/id128-util.h index 4b91a16bd16..887f443d693 100644 --- a/src/libsystemd/sd-id128/id128-util.h +++ b/src/libsystemd/sd-id128/id128-util.h @@ -27,6 +27,7 @@ int id128_write(const char *p, Id128FormatFlag f, sd_id128_t id); void id128_hash_func(const sd_id128_t *p, struct siphash *state); int id128_compare_func(const sd_id128_t *a, const sd_id128_t *b) _pure_; extern const struct hash_ops id128_hash_ops; +extern const struct hash_ops id128_hash_ops_free; sd_id128_t id128_make_v4_uuid(sd_id128_t id); diff --git a/src/udev/udevadm-trigger.c b/src/udev/udevadm-trigger.c index 1d421064d78..40ee5085a0b 100644 --- a/src/udev/udevadm-trigger.c +++ b/src/udev/udevadm-trigger.c @@ -11,10 +11,12 @@ #include "device-util.h" #include "fd-util.h" #include "fileio.h" +#include "id128-util.h" #include "parse-util.h" #include "path-util.h" #include "process-util.h" #include "set.h" +#include "static-destruct.h" #include "string-util.h" #include "strv.h" #include "udevadm.h" @@ -26,17 +28,21 @@ static bool arg_verbose = false; static bool arg_dry_run = false; static bool arg_quiet = false; static bool arg_uuid = false; +static bool arg_settle = false; static int exec_list( sd_device_enumerator *e, sd_device_action_t action, - Hashmap *settle_hashmap) { + Set **ret_settle_path_or_ids) { - bool skip_uuid_logic = false; + _cleanup_set_free_ Set *settle_path_or_ids = NULL; + int uuid_supported = -1; const char *action_str; sd_device *d; int r, ret = 0; + assert(e); + action_str = device_action_to_string(action); FOREACH_DEVICE_AND_SUBSYSTEM(e, d) { @@ -57,14 +63,14 @@ static int exec_list( /* Use the UUID mode if the user explicitly asked for it, or if --settle has been specified, * so that we can recognize our own uevent. */ - r = sd_device_trigger_with_uuid(d, action, (arg_uuid || settle_hashmap) && !skip_uuid_logic ? &id : NULL); - if (r == -EINVAL && !arg_uuid && settle_hashmap && !skip_uuid_logic) { + r = sd_device_trigger_with_uuid(d, action, (arg_uuid || arg_settle) && uuid_supported != 0 ? &id : NULL); + if (r == -EINVAL && !arg_uuid && arg_settle && uuid_supported < 0) { /* If we specified a UUID because of the settling logic, and we got EINVAL this might * be caused by an old kernel which doesn't know the UUID logic (pre-4.13). Let's try * if it works without the UUID logic then. */ r = sd_device_trigger(d, action); if (r != -EINVAL) - skip_uuid_logic = true; /* dropping the uuid stuff changed the return code, + uuid_supported = false; /* dropping the uuid stuff changed the return code, * hence don't bother next time */ } if (r < 0) { @@ -108,65 +114,96 @@ static int exec_list( continue; } + if (uuid_supported < 0) + uuid_supported = true; + /* If the user asked for it, write event UUID to stdout */ if (arg_uuid) printf(SD_ID128_UUID_FORMAT_STR "\n", SD_ID128_FORMAT_VAL(id)); - if (settle_hashmap) { - _cleanup_free_ sd_id128_t *mid = NULL; - _cleanup_free_ char *sp = NULL; + if (arg_settle) { + if (uuid_supported) { + sd_id128_t *dup; - sp = strdup(syspath); - if (!sp) - return log_oom(); + dup = newdup(sd_id128_t, &id, 1); + if (!dup) + return log_oom(); - mid = newdup(sd_id128_t, &id, 1); - if (!d) - return log_oom(); + r = set_ensure_consume(&settle_path_or_ids, &id128_hash_ops_free, dup); + } else { + char *dup; - r = hashmap_put(settle_hashmap, sp, mid); + dup = strdup(syspath); + if (!dup) + return log_oom(); + + r = set_ensure_consume(&settle_path_or_ids, &path_hash_ops_free, dup); + } if (r < 0) return log_oom(); - - TAKE_PTR(sp); - TAKE_PTR(mid); } } + if (ret_settle_path_or_ids) + *ret_settle_path_or_ids = TAKE_PTR(settle_path_or_ids); + return ret; } static int device_monitor_handler(sd_device_monitor *m, sd_device *dev, void *userdata) { - Hashmap *settle_hashmap = ASSERT_PTR(userdata); - sd_id128_t *settle_id; + Set *settle_path_or_ids = * (Set**) ASSERT_PTR(userdata); const char *syspath; - char *k; + sd_id128_t id; int r; assert(dev); r = sd_device_get_syspath(dev, &syspath); if (r < 0) { - log_debug_errno(r, "Failed to get syspath of device event, ignoring: %m"); + log_device_debug_errno(dev, r, "Failed to get syspath of device event, ignoring: %m"); return 0; } - settle_id = hashmap_get2(settle_hashmap, syspath, (void**) &k); - if (!settle_id) { - log_debug("Got uevent for unexpected device '%s', ignoring.", syspath); - return 0; - } - if (!sd_id128_is_null(*settle_id)) { /* If this is SD_ID128_NULL then we are on pre-4.13 and have no UUID to check, hence don't */ - sd_id128_t event_id; + if (sd_device_get_trigger_uuid(dev, &id) >= 0) { + _cleanup_free_ sd_id128_t *saved = NULL; - r = sd_device_get_trigger_uuid(dev, &event_id); - if (r < 0) { - log_debug_errno(r, "Got uevent without synthetic UUID for device '%s', ignoring: %m", syspath); + saved = set_remove(settle_path_or_ids, &id); + if (!saved) { + log_device_debug(dev, "Got uevent not matching expected UUID, ignoring."); return 0; } + } else { + _cleanup_free_ char *saved = NULL; - if (!sd_id128_equal(event_id, *settle_id)) { - log_debug("Got uevent not matching expected UUID for device '%s', ignoring.", syspath); + saved = set_remove(settle_path_or_ids, syspath); + if (!saved) { + const char *old_sysname; + + /* When the device is renamed, the new name is broadcast, and the old name is saved + * in INTERFACE_OLD. */ + + if (sd_device_get_property_value(dev, "INTERFACE_OLD", &old_sysname) >= 0) { + _cleanup_free_ char *dir = NULL, *old_syspath = NULL; + + r = path_extract_directory(syspath, &dir); + if (r < 0) { + log_device_debug_errno(dev, r, + "Failed to extract directory from '%s', ignoring: %m", + syspath); + return 0; + } + + old_syspath = path_join(dir, old_sysname); + if (!old_syspath) { + log_oom_debug(); + return 0; + } + + saved = set_remove(settle_path_or_ids, old_syspath); + } + } + if (!saved) { + log_device_debug(dev, "Got uevent for unexpected device, ignoring."); return 0; } } @@ -175,12 +212,9 @@ static int device_monitor_handler(sd_device_monitor *m, sd_device *dev, void *us printf("settle %s\n", syspath); if (arg_uuid) - printf("settle " SD_ID128_UUID_FORMAT_STR "\n", SD_ID128_FORMAT_VAL(*settle_id)); + printf("settle " SD_ID128_UUID_FORMAT_STR "\n", SD_ID128_FORMAT_VAL(id)); - free(hashmap_remove(settle_hashmap, syspath)); - free(k); - - if (hashmap_isempty(settle_hashmap)) + if (set_isempty(settle_path_or_ids)) return sd_event_exit(sd_device_monitor_get_event(m), 0); return 0; @@ -283,9 +317,9 @@ int trigger_main(int argc, char *argv[], void *userdata) { _cleanup_(sd_device_enumerator_unrefp) sd_device_enumerator *e = NULL; _cleanup_(sd_device_monitor_unrefp) sd_device_monitor *m = NULL; _cleanup_(sd_event_unrefp) sd_event *event = NULL; - _cleanup_hashmap_free_ Hashmap *settle_hashmap = NULL; + _cleanup_set_free_ Set *settle_path_or_ids = NULL; usec_t ping_timeout_usec = 5 * USEC_PER_SEC; - bool settle = false, ping = false; + bool ping = false; int c, r; if (running_in_chroot() > 0) { @@ -389,7 +423,7 @@ int trigger_main(int argc, char *argv[], void *userdata) { break; } case 'w': - settle = true; + arg_settle = true; break; case ARG_NAME: { @@ -477,11 +511,7 @@ int trigger_main(int argc, char *argv[], void *userdata) { return log_error_errno(r, "Failed to add parent match '%s': %m", argv[optind]); } - if (settle) { - settle_hashmap = hashmap_new(&path_hash_ops_free_free); - if (!settle_hashmap) - return log_oom(); - + if (arg_settle) { r = sd_event_default(&event); if (r < 0) return log_error_errno(r, "Failed to get default event: %m"); @@ -494,7 +524,7 @@ int trigger_main(int argc, char *argv[], void *userdata) { if (r < 0) return log_error_errno(r, "Failed to attach event to device monitor: %m"); - r = sd_device_monitor_start(m, device_monitor_handler, settle_hashmap); + r = sd_device_monitor_start(m, device_monitor_handler, &settle_path_or_ids); if (r < 0) return log_error_errno(r, "Failed to start device monitor: %m"); } @@ -519,11 +549,11 @@ int trigger_main(int argc, char *argv[], void *userdata) { assert_not_reached(); } - r = exec_list(e, action, settle_hashmap); + r = exec_list(e, action, arg_settle ? &settle_path_or_ids : NULL); if (r < 0) return r; - if (event && !hashmap_isempty(settle_hashmap)) { + if (!set_isempty(settle_path_or_ids)) { r = sd_event_loop(event); if (r < 0) return log_error_errno(r, "Event loop failed: %m"); diff --git a/test/units/testsuite-17.02.sh b/test/units/testsuite-17.02.sh index ed3b39d074a..82f9fd1f629 100755 --- a/test/units/testsuite-17.02.sh +++ b/test/units/testsuite-17.02.sh @@ -61,9 +61,7 @@ EOF udevadm control --log-priority=debug --reload --timeout=30 -# FIXME(?): the 'add' uevent is broadcast as for 'foobar', instead of 'hoge'. Hence, we cannot use --settle here. -# See issue #25115. -udevadm trigger --action=add /sys/devices/virtual/net/hoge +udevadm trigger --action=add --settle /sys/devices/virtual/net/hoge udevadm wait --timeout=30 --settle /sys/devices/virtual/net/foobar assert_not_in "ID_RENAMING=" "$(udevadm info /sys/devices/virtual/net/foobar)" timeout 30 bash -c 'while [[ "$(systemctl show --property=ActiveState --value /sys/devices/virtual/net/hoge)" != "inactive" ]]; do sleep .5; done' @@ -71,7 +69,7 @@ timeout 30 bash -c 'while [[ "$(systemctl show --property=ActiveState --value /s timeout 30 bash -c 'while [[ "$(systemctl show --property=ActiveState --value /sys/devices/virtual/net/foobar)" != "active" ]]; do sleep .5; done' timeout 30 bash -c 'while [[ "$(systemctl show --property=ActiveState --value /sys/subsystem/net/devices/foobar)" != "active" ]]; do sleep .5; done' -udevadm trigger --action=add /sys/devices/virtual/net/foobar +udevadm trigger --action=add --settle /sys/devices/virtual/net/foobar udevadm wait --timeout=30 --settle /sys/devices/virtual/net/hoge assert_not_in "ID_RENAMING=" "$(udevadm info /sys/devices/virtual/net/hoge)" timeout 30 bash -c 'while [[ "$(systemctl show --property=ActiveState --value /sys/devices/virtual/net/hoge)" != "active" ]]; do sleep .5; done'