From 1043ae3bc2429677262cc124f4d29475e6249bf0 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 8 Apr 2025 04:58:01 +0900 Subject: [PATCH 1/5] udev-varlink: introduce io.systemd.Udev.Revert method It reverts dynamically set configurations. --- src/shared/varlink-io.systemd.Udev.c | 4 +++ src/udev/udev-config.c | 45 +++++++++++++++++++++------- src/udev/udev-config.h | 1 + src/udev/udev-manager.c | 11 +++++++ src/udev/udev-manager.h | 1 + src/udev/udev-varlink.c | 16 ++++++++++ 6 files changed, 68 insertions(+), 10 deletions(-) diff --git a/src/shared/varlink-io.systemd.Udev.c b/src/shared/varlink-io.systemd.Udev.c index 09c1ca70ce..b6cfbd3bf3 100644 --- a/src/shared/varlink-io.systemd.Udev.c +++ b/src/shared/varlink-io.systemd.Udev.c @@ -17,6 +17,8 @@ static SD_VARLINK_DEFINE_METHOD( SD_VARLINK_FIELD_COMMENT("An array of global udev property assignments. Each string must be in KEY=VALUE style."), SD_VARLINK_DEFINE_INPUT(assignments, SD_VARLINK_STRING, SD_VARLINK_ARRAY)); +static SD_VARLINK_DEFINE_METHOD(Revert); + static SD_VARLINK_DEFINE_METHOD(StartExecQueue); static SD_VARLINK_DEFINE_METHOD(StopExecQueue); @@ -34,6 +36,8 @@ SD_VARLINK_DEFINE_INTERFACE( SD_VARLINK_SYMBOL_COMMENT("Sets the global udev properties."), &vl_method_SetEnvironment, SD_VARLINK_SYMBOL_COMMENT("Starts processing of queued events."), + &vl_method_Revert, + SD_VARLINK_SYMBOL_COMMENT("Revert previously set configurations."), &vl_method_StartExecQueue, SD_VARLINK_SYMBOL_COMMENT("Stops processing of queued events."), &vl_method_StopExecQueue, diff --git a/src/udev/udev-config.c b/src/udev/udev-config.c index 142f19e1b1..d88c18ba15 100644 --- a/src/udev/udev-config.c +++ b/src/udev/udev-config.c @@ -463,6 +463,24 @@ int manager_load(Manager *manager, int argc, char *argv[]) { return 1; } +static UdevReloadFlags manager_needs_reload(Manager *manager, const UdevConfig *old) { + assert(manager); + assert(old); + + if (manager->config.resolve_name_timing != old->resolve_name_timing) + return UDEV_RELOAD_RULES | UDEV_RELOAD_KILL_WORKERS; + + if (manager->config.log_level != old->log_level || + manager->config.exec_delay_usec != old->exec_delay_usec || + manager->config.timeout_usec != old->timeout_usec || + manager->config.timeout_signal != old->timeout_signal || + manager->config.blockdev_read_only != old->blockdev_read_only || + manager->config.trace != old->trace) + return UDEV_RELOAD_KILL_WORKERS; + + return 0; +} + UdevReloadFlags manager_reload_config(Manager *manager) { assert(manager); @@ -473,18 +491,25 @@ UdevReloadFlags manager_reload_config(Manager *manager) { manager_merge_config(manager); manager_adjust_config(&manager->config); - if (manager->config.resolve_name_timing != old.resolve_name_timing) - return UDEV_RELOAD_RULES | UDEV_RELOAD_KILL_WORKERS; + return manager_needs_reload(manager, &old); +} - if (manager->config.log_level != old.log_level || - manager->config.exec_delay_usec != old.exec_delay_usec || - manager->config.timeout_usec != old.timeout_usec || - manager->config.timeout_signal != old.timeout_signal || - manager->config.blockdev_read_only != old.blockdev_read_only || - manager->config.trace != old.trace) - return UDEV_RELOAD_KILL_WORKERS; +UdevReloadFlags manager_revert_config(Manager *manager) { + assert(manager); - return 0; + UdevReloadFlags flags = 0; + if (!hashmap_isempty(manager->properties)) { + flags |= UDEV_RELOAD_KILL_WORKERS; + manager->properties = hashmap_free(manager->properties); + } + + UdevConfig old = manager->config; + + manager->config_by_control = UDEV_CONFIG_INIT; + manager_merge_config(manager); + manager_adjust_config(&manager->config); + + return flags | manager_needs_reload(manager, &old); } static usec_t extra_timeout_usec(void) { diff --git a/src/udev/udev-config.h b/src/udev/udev-config.h index 339d426a0f..72ac3920ab 100644 --- a/src/udev/udev-config.h +++ b/src/udev/udev-config.h @@ -34,5 +34,6 @@ void manager_set_environment(Manager *manager, char * const *v); int manager_load(Manager *manager, int argc, char *argv[]); UdevReloadFlags manager_reload_config(Manager *manager); +UdevReloadFlags manager_revert_config(Manager *manager); usec_t manager_kill_worker_timeout(Manager *manager); diff --git a/src/udev/udev-manager.c b/src/udev/udev-manager.c index 8d27373b45..919314b546 100644 --- a/src/udev/udev-manager.c +++ b/src/udev/udev-manager.c @@ -311,6 +311,17 @@ void manager_reload(Manager *manager, bool force) { notify_ready(manager); } +void manager_revert(Manager *manager) { + assert(manager); + + UdevReloadFlags flags = manager_revert_config(manager); + if (flags == 0) + return; + + assert(flags == UDEV_RELOAD_KILL_WORKERS); + manager_kill_workers(manager, SIGTERM); +} + static int on_sigchld(sd_event_source *s, const siginfo_t *si, void *userdata) { _cleanup_(worker_freep) Worker *worker = ASSERT_PTR(userdata); sd_device *dev = worker->event ? ASSERT_PTR(worker->event->dev) : NULL; diff --git a/src/udev/udev-manager.h b/src/udev/udev-manager.h index fc2df313e2..806adb3ef8 100644 --- a/src/udev/udev-manager.h +++ b/src/udev/udev-manager.h @@ -70,6 +70,7 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(Manager*, manager_free); int manager_main(Manager *manager); void manager_reload(Manager *manager, bool force); +void manager_revert(Manager *manager); void manager_exit(Manager *manager); void notify_ready(Manager *manager); diff --git a/src/udev/udev-varlink.c b/src/udev/udev-varlink.c index 41f7fe08f9..d0e85c2633 100644 --- a/src/udev/udev-varlink.c +++ b/src/udev/udev-varlink.c @@ -103,6 +103,21 @@ static int vl_method_set_environment(sd_varlink *link, sd_json_variant *paramete return sd_varlink_reply(link, NULL); } +static int vl_method_revert(sd_varlink *link, sd_json_variant *parameters, sd_varlink_method_flags_t flags, void *userdata) { + Manager *manager = ASSERT_PTR(userdata); + int r; + + assert(link); + + r = sd_varlink_dispatch(link, parameters, /* dispatch_table = */ NULL, /* userdata = */ NULL); + if (r != 0) + return r; + + log_debug("Received io.systemd.Udev.Revert()"); + manager_revert(manager); + return sd_varlink_reply(link, NULL); +} + static int vl_method_start_stop_exec_queue(sd_varlink *link, sd_json_variant *parameters, sd_varlink_method_flags_t flags, void *userdata) { Manager *manager = ASSERT_PTR(userdata); const char *method; @@ -183,6 +198,7 @@ int manager_start_varlink_server(Manager *manager) { "io.systemd.Udev.SetTrace", vl_method_set_trace, "io.systemd.Udev.SetChildrenMax", vl_method_set_children_max, "io.systemd.Udev.SetEnvironment", vl_method_set_environment, + "io.systemd.Udev.Revert", vl_method_revert, "io.systemd.Udev.StartExecQueue", vl_method_start_stop_exec_queue, "io.systemd.Udev.StopExecQueue", vl_method_start_stop_exec_queue, "io.systemd.Udev.Exit", vl_method_exit); From c53d155d8d7557aee3ec6e371fd9c64442389bef Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 8 Apr 2025 05:06:22 +0900 Subject: [PATCH 2/5] udevadm: introduce --revert option to call io.systemd.service.Revert --- man/udevadm.xml | 10 ++++++++++ shell-completion/bash/udevadm | 2 +- shell-completion/zsh/_udevadm | 1 + src/udev/udevadm-control.c | 17 ++++++++++++++++- 4 files changed, 28 insertions(+), 2 deletions(-) diff --git a/man/udevadm.xml b/man/udevadm.xml index 4e1255bf4d..5636d102b7 100644 --- a/man/udevadm.xml +++ b/man/udevadm.xml @@ -769,6 +769,16 @@ + + + + Revert settings previously set with udevadm control command. When + specified, settings set with , , + , and will be cleared. + + + + diff --git a/shell-completion/bash/udevadm b/shell-completion/bash/udevadm index d3c07e81dd..ee463902b5 100644 --- a/shell-completion/bash/udevadm +++ b/shell-completion/bash/udevadm @@ -93,7 +93,7 @@ _udevadm() { -g --tag-match -y --sysname-match --name-match -b --parent-match --prioritized-subsystem' [SETTLE]='-t --timeout -E --exit-if-exists' - [CONTROL_STANDALONE]='-e --exit -s --stop-exec-queue -S --start-exec-queue -R --reload --ping + [CONTROL_STANDALONE]='-e --exit -s --stop-exec-queue -S --start-exec-queue -R --reload --ping --revert --load-credentials' [CONTROL_ARG]='-l --log-priority -p --property -m --children-max -t --timeout --trace' [MONITOR_STANDALONE]='-k --kernel -u --udev -p --property' diff --git a/shell-completion/zsh/_udevadm b/shell-completion/zsh/_udevadm index d5cda3977e..0f4b8aff2c 100644 --- a/shell-completion/zsh/_udevadm +++ b/shell-completion/zsh/_udevadm @@ -67,6 +67,7 @@ _udevadm_control(){ '(-p --property)'{-p,--property=}'[Set a global property for all events.]:KEY=VALUE' \ '(-m --children-max=)'{-m,--children-max=}'[Set the maximum number of events.]:N' \ '--trace=[Enable/disable trace logging.]:BOOL' \ + '--revert[Revert previously set configurations.]' \ '(-t --timeout=)'{-t,--timeout=}'[The maximum number of seconds to wait for a reply from systemd-udevd.]:SECONDS' } diff --git a/src/udev/udevadm-control.c b/src/udev/udevadm-control.c index 1fa5a42018..f9cc4c34cf 100644 --- a/src/udev/udevadm-control.c +++ b/src/udev/udevadm-control.c @@ -32,6 +32,7 @@ static int arg_max_children = -1; static int arg_log_level = -1; static int arg_start_exec_queue = -1; static int arg_trace = -1; +static bool arg_revert = false; static bool arg_load_credentials = false; STATIC_DESTRUCTOR_REGISTER(arg_env, strv_freep); @@ -45,7 +46,8 @@ static bool arg_has_control_commands(void) { !strv_isempty(arg_env) || arg_max_children >= 0 || arg_ping || - arg_trace >= 0; + arg_trace >= 0 || + arg_revert; } static int help(void) { @@ -62,6 +64,7 @@ static int help(void) { " -m --children-max=N Maximum number of children\n" " --ping Wait for udev to respond to a ping message\n" " --trace=BOOL Enable/disable trace logging\n" + " --revert Revert previously set configurations\n" " -t --timeout=SECONDS Maximum time to block for a reply\n" " --load-credentials Load udev rules from credentials\n", program_invocation_short_name); @@ -73,6 +76,7 @@ static int parse_argv(int argc, char *argv[]) { enum { ARG_PING = 0x100, ARG_TRACE, + ARG_REVERT, ARG_LOAD_CREDENTIALS, }; @@ -89,6 +93,7 @@ static int parse_argv(int argc, char *argv[]) { { "children-max", required_argument, NULL, 'm' }, { "ping", no_argument, NULL, ARG_PING }, { "trace", required_argument, NULL, ARG_TRACE }, + { "revert", no_argument, NULL, ARG_REVERT }, { "timeout", required_argument, NULL, 't' }, { "load-credentials", no_argument, NULL, ARG_LOAD_CREDENTIALS }, { "version", no_argument, NULL, 'V' }, @@ -157,6 +162,10 @@ static int parse_argv(int argc, char *argv[]) { arg_trace = r; break; + case ARG_REVERT: + arg_revert = true; + break; + case 't': r = parse_sec(optarg, &arg_timeout); if (r < 0) @@ -270,6 +279,12 @@ static int send_control_commands(void) { if (arg_exit) return varlink_call_and_log(link, "io.systemd.Udev.Exit", /* parameters = */ NULL, /* reply = */ NULL); + if (arg_revert) { + r = varlink_call_and_log(link, "io.systemd.Udev.Revert", /* parameters = */ NULL, /* reply = */ NULL); + if (r < 0) + return r; + } + if (arg_log_level >= 0) { r = varlink_callbo_and_log(link, "io.systemd.service.SetLogLevel", /* reply = */ NULL, SD_JSON_BUILD_PAIR_INTEGER("level", arg_log_level)); From 8b078c5b86e13e9af88ead08ac1570cb4c61f7ef Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 5 Apr 2025 22:41:28 +0900 Subject: [PATCH 3/5] daemon-util: expose notify_remove_fd_warn() --- src/shared/daemon-util.c | 2 +- src/shared/daemon-util.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/shared/daemon-util.c b/src/shared/daemon-util.c index d8aa259ee9..e83ca8f663 100644 --- a/src/shared/daemon-util.c +++ b/src/shared/daemon-util.c @@ -6,7 +6,7 @@ #include "string-util.h" #include "time-util.h" -static int notify_remove_fd_warn(const char *name) { +int notify_remove_fd_warn(const char *name) { int r; assert(name); diff --git a/src/shared/daemon-util.h b/src/shared/daemon-util.h index cbb175cbe1..ff5e60e9be 100644 --- a/src/shared/daemon-util.h +++ b/src/shared/daemon-util.h @@ -23,6 +23,7 @@ static inline void notify_on_cleanup(const char **p) { (void) sd_notify(false, *p); } +int notify_remove_fd_warn(const char *name); int notify_remove_fd_warnf(const char *format, ...) _printf_(1, 2); int close_and_notify_warn(int fd, const char *name); int notify_push_fd(int fd, const char *name); From 49120a7a8fdf772483f907c0bc4111b141185ac2 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 8 Apr 2025 04:33:14 +0900 Subject: [PATCH 4/5] udev-config: serialize/deserialize dynamical configurations Previously, configurations set by 'udevadm control' such as log level, maximum number of childrens, global properties, and so on were discarded on restart. This makes udevd serialize those configurations on stop, and deserialize them in the next invocation. --- src/udev/udev-config.c | 113 ++++++++++++++++++++++++++++++++++++++++ src/udev/udev-config.h | 3 ++ src/udev/udev-manager.c | 3 ++ 3 files changed, 119 insertions(+) diff --git a/src/udev/udev-config.c b/src/udev/udev-config.c index d88c18ba15..52855145da 100644 --- a/src/udev/udev-config.c +++ b/src/udev/udev-config.c @@ -5,10 +5,14 @@ #include "conf-parser.h" #include "cpu-set-util.h" +#include "daemon-util.h" +#include "fd-util.h" +#include "fileio.h" #include "limits-util.h" #include "parse-util.h" #include "pretty-print.h" #include "proc-cmdline.h" +#include "serialize.h" #include "signal-util.h" #include "syslog-util.h" #include "udev-config.h" @@ -512,6 +516,115 @@ UdevReloadFlags manager_revert_config(Manager *manager) { return flags | manager_needs_reload(manager, &old); } +int manager_serialize_config(Manager *manager) { + int r; + + assert(manager); + + _cleanup_fclose_ FILE *f = NULL; + r = open_serialization_file("systemd-udevd", &f); + if (r < 0) + return log_warning_errno(r, "Failed to open new serialization file: %m"); + + if (manager->config_by_control.log_level >= 0) { + r = serialize_item_format(f, "log-level", "%i", manager->config_by_control.log_level); + if (r < 0) + return r; + } + + if (manager->config_by_control.children_max > 0) { + r = serialize_item_format(f, "children-max", "%u", manager->config_by_control.children_max); + if (r < 0) + return r; + } + + r = serialize_bool_elide(f, "trace", manager->config_by_control.trace); + if (r < 0) + return r; + + const char *k, *v; + HASHMAP_FOREACH_KEY(v, k, manager->properties) { + r = serialize_item_format(f, "property", "%s=%s", k, v); + if (r < 0) + return r; + } + + r = finish_serialization_file(f); + if (r < 0) + return log_warning_errno(r, "Failed to finalize serialization file: %m"); + + /* Remove the previous serialization to make it replaced with the new one. */ + (void) notify_remove_fd_warn("config-serialization"); + + r = notify_push_fd(fileno(f), "config-serialization"); + if (r < 0) + return log_warning_errno(r, "Failed to push serialization fd to service manager: %m"); + + log_debug("Serialized configurations."); + return 0; +} + +int manager_deserialize_config(Manager *manager, int *fd) { + int r; + + assert(manager); + assert(fd); + assert(*fd >= 0); + + /* This may invalidate passed file descriptor even on failure. */ + + _cleanup_fclose_ FILE *f = take_fdopen(fd, "r"); + if (!f) + return log_warning_errno(errno, "Failed to open serialization fd: %m"); + + for (;;) { + _cleanup_free_ char *l = NULL; + const char *val; + + r = deserialize_read_line(f, &l); + if (r < 0) + return r; + if (r == 0) /* eof or end marker */ + break; + + if ((val = startswith(l, "log-level="))) { + r = log_level_from_string(val); + if (r < 0) + log_debug_errno(r, "Failed to parse serialized log level (%s), ignoring: %m", val); + else + manager->config_by_control.log_level = r; + + } else if ((val = startswith(l, "children-max="))) { + r = safe_atou(val, &manager->config_by_control.children_max); + if (r < 0) + log_debug_errno(r, "Failed to parse serialized children max (%s), ignoring: %m", val); + + } else if ((val = startswith(l, "trace="))) { + r = parse_boolean(val); + if (r < 0) + log_debug_errno(r, "Failed to parse serialized trace (%s), ignoring: %m", val); + else + manager->config_by_control.trace = r; + + } else if ((val = startswith(l, "property="))) { + if (!udev_property_assignment_is_valid(val)) + r = -EINVAL; + else + r = manager_set_environment_one(manager, val); + if (r < 0) + log_debug_errno(r, "Failed to deserialize property (%s), ignoring: %m", val); + + } else + log_debug("Unknown serialization item, ignoring: %s", l); + } + + manager_merge_config(manager); + manager_adjust_config(&manager->config); + + log_debug("Deserialized configurations."); + return 0; +} + static usec_t extra_timeout_usec(void) { static usec_t saved = 10 * USEC_PER_SEC; static bool parsed = false; diff --git a/src/udev/udev-config.h b/src/udev/udev-config.h index 72ac3920ab..127b3b9a1b 100644 --- a/src/udev/udev-config.h +++ b/src/udev/udev-config.h @@ -36,4 +36,7 @@ int manager_load(Manager *manager, int argc, char *argv[]); UdevReloadFlags manager_reload_config(Manager *manager); UdevReloadFlags manager_revert_config(Manager *manager); +int manager_serialize_config(Manager *manager); +int manager_deserialize_config(Manager *manager, int *fd); + usec_t manager_kill_worker_timeout(Manager *manager); diff --git a/src/udev/udev-manager.c b/src/udev/udev-manager.c index 919314b546..d35100198b 100644 --- a/src/udev/udev-manager.c +++ b/src/udev/udev-manager.c @@ -237,6 +237,7 @@ void manager_exit(Manager *manager) { /* close sources of new events and discard buffered events */ manager->ctrl = udev_ctrl_unref(manager->ctrl); manager->varlink_server = sd_varlink_server_unref(manager->varlink_server); + (void) manager_serialize_config(manager); /* Disable the event source, but does not close the inotify fd here, as we may still receive * notification messages about requests to add or remove inotify watches. */ @@ -1178,6 +1179,8 @@ static int manager_listen_fds(Manager *manager) { r = manager_init_device_monitor(manager, fd); else if (streq(names[i], "inotify")) r = manager_init_inotify(manager, fd); + else if (streq(names[i], "config-serialization")) + r = manager_deserialize_config(manager, &fd); else r = log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "Received unexpected fd (%s), ignoring.", names[i]); From c50606aa56609ab91a35f647635e77db3a625497 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 8 Apr 2025 05:26:09 +0900 Subject: [PATCH 5/5] TEST-17-UDEV: add test cases for reverting/serializing/deserializing config --- test/units/TEST-17-UDEV.global-property.sh | 29 ++++++++++++++++++++++ test/units/TEST-17-UDEV.sanity-check.sh | 5 +++- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/test/units/TEST-17-UDEV.global-property.sh b/test/units/TEST-17-UDEV.global-property.sh index 152922b364..c9b070e203 100755 --- a/test/units/TEST-17-UDEV.global-property.sh +++ b/test/units/TEST-17-UDEV.global-property.sh @@ -86,4 +86,33 @@ udevadm trigger --action change --settle /dev/null test_property /dev/null PROP_FOO foo test_not_property /dev/null PROP_BAR +: revert + +udevadm control --revert +udevadm trigger --action change --settle /dev/null +test_not_property /dev/null PROP_FOO +test_not_property /dev/null PROP_BAR + +: set again, and restart + +udevadm control -p FOO=foo -p BAR=bar +udevadm trigger --action change --settle /dev/null +test_property /dev/null PROP_FOO foo +test_property /dev/null PROP_BAR bar +systemctl restart systemd-udevd.service +udevadm trigger --action change --settle /dev/null +test_property /dev/null PROP_FOO foo +test_property /dev/null PROP_BAR bar + +: revert again, and restart + +udevadm control --revert +udevadm trigger --action change --settle /dev/null +test_not_property /dev/null PROP_FOO +test_not_property /dev/null PROP_BAR +systemctl restart systemd-udevd.service +udevadm trigger --action change --settle /dev/null +test_not_property /dev/null PROP_FOO +test_not_property /dev/null PROP_BAR + exit 0 diff --git a/test/units/TEST-17-UDEV.sanity-check.sh b/test/units/TEST-17-UDEV.sanity-check.sh index 5bbf42a5cf..125a5946f7 100755 --- a/test/units/TEST-17-UDEV.sanity-check.sh +++ b/test/units/TEST-17-UDEV.sanity-check.sh @@ -61,10 +61,13 @@ udevadm control -R udevadm control -p HELLO=world udevadm control -m 42 udevadm control --ping -t 5 -udevadm control --trace yes udevadm control --trace no +udevadm control --trace yes udevadm control --load-credentials udevadm control -h +# Sanity check for serialization and deserialization +systemctl restart systemd-udevd.service +udevadm control --revert udevadm info /dev/null udevadm info /sys/class/net/$netdev