From 623461c13074542b9a4dd2e7f605b6b7f8be5286 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 21 Feb 2022 17:11:52 +0100 Subject: [PATCH] systemctl: rework daemon_reload() functions Let's split out the inner parts of verb_daemon_reload() as a function daemon_reload() and then stop using the former outside of the verbs logic, and instead call the latter whenever we need to reload the daemon as auxiliary opeation. This should make our logic more systematic as we don't have to provide fake or misleading argc/argv to verb_daemon_reload() anymore. --- src/systemctl/systemctl-add-dependency.c | 4 ++- src/systemctl/systemctl-compat-telinit.c | 11 +++--- src/systemctl/systemctl-daemon-reload.c | 45 ++++++++++++++++-------- src/systemctl/systemctl-daemon-reload.h | 4 +++ src/systemctl/systemctl-edit.c | 7 ++-- src/systemctl/systemctl-enable.c | 12 ++++--- src/systemctl/systemctl-preset-all.c | 4 ++- src/systemctl/systemctl-set-default.c | 8 +++-- src/systemctl/systemctl.c | 4 +-- 9 files changed, 67 insertions(+), 32 deletions(-) diff --git a/src/systemctl/systemctl-add-dependency.c b/src/systemctl/systemctl-add-dependency.c index 9276ed3e2bc..120798b33f3 100644 --- a/src/systemctl/systemctl-add-dependency.c +++ b/src/systemctl/systemctl-add-dependency.c @@ -78,7 +78,9 @@ int verb_add_dependency(int argc, char *argv[], void *userdata) { goto finish; } - r = verb_daemon_reload(argc, argv, userdata); + r = daemon_reload(ACTION_RELOAD, /* graceful= */ false); + if (r > 0) + r = 0; } finish: diff --git a/src/systemctl/systemctl-compat-telinit.c b/src/systemctl/systemctl-compat-telinit.c index 1e771ef4a66..20325e5e1c6 100644 --- a/src/systemctl/systemctl-compat-telinit.c +++ b/src/systemctl/systemctl-compat-telinit.c @@ -142,13 +142,14 @@ int start_with_fallback(void) { } int reload_with_fallback(void) { - /* First, try systemd via D-Bus. */ - if (verb_daemon_reload(0, NULL, NULL) >= 0) - return 0; - /* Nothing else worked, so let's try signals */ assert(IN_SET(arg_action, ACTION_RELOAD, ACTION_REEXEC)); + /* First, try systemd via D-Bus */ + if (daemon_reload(arg_action, /* graceful= */ true) > 0) + return 0; + + /* That didn't work, so let's try signals */ if (kill(1, arg_action == ACTION_RELOAD ? SIGHUP : SIGTERM) < 0) return log_error_errno(errno, "kill() failed: %m"); @@ -157,7 +158,7 @@ int reload_with_fallback(void) { int exec_telinit(char *argv[]) { (void) rlimit_nofile_safe(); - execv(TELINIT, argv); + (void) execv(TELINIT, argv); return log_error_errno(SYNTHETIC_ERRNO(EIO), "Couldn't find an alternative telinit implementation to spawn."); diff --git a/src/systemctl/systemctl-daemon-reload.c b/src/systemctl/systemctl-daemon-reload.c index 35a10e84cc0..33de7d161e2 100644 --- a/src/systemctl/systemctl-daemon-reload.c +++ b/src/systemctl/systemctl-daemon-reload.c @@ -6,7 +6,7 @@ #include "systemctl-util.h" #include "systemctl.h" -int verb_daemon_reload(int argc, char *argv[], void *userdata) { +int daemon_reload(enum action action, bool graceful) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL; const char *method; @@ -19,7 +19,7 @@ int verb_daemon_reload(int argc, char *argv[], void *userdata) { polkit_agent_open_maybe(); - switch (arg_action) { + switch (action) { case ACTION_RELOAD: method = "Reload"; @@ -29,13 +29,8 @@ int verb_daemon_reload(int argc, char *argv[], void *userdata) { method = "Reexecute"; break; - case ACTION_SYSTEMCTL: - method = streq(argv[0], "daemon-reexec") ? "Reexecute" : - /* "daemon-reload" */ "Reload"; - break; - default: - assert_not_reached(); + return -EINVAL; } r = bus_message_new_method_call(bus, &m, bus_systemd_mgr, method); @@ -50,14 +45,36 @@ int verb_daemon_reload(int argc, char *argv[], void *userdata) { r = sd_bus_call(bus, m, DEFAULT_TIMEOUT_USEC * 2, &error, NULL); /* On reexecution, we expect a disconnect, not a reply */ - if (IN_SET(r, -ETIMEDOUT, -ECONNRESET) && streq(method, "Reexecute")) - r = 0; + if (IN_SET(r, -ETIMEDOUT, -ECONNRESET) && action == ACTION_REEXEC) + return 1; + if (r < 0) { + if (graceful) { /* If graceful mode is selected, debug log, but don't fail */ + log_debug_errno(r, "Failed to reload daemon via the bus, ignoring: %s", bus_error_message(&error, r)); + return 0; + } - if (r < 0 && arg_action == ACTION_SYSTEMCTL) return log_error_errno(r, "Failed to reload daemon: %s", bus_error_message(&error, r)); + } + + return 1; +} - /* Note that for the legacy commands (i.e. those with action != ACTION_SYSTEMCTL) we support - * fallbacks to the old ways of doing things, hence don't log any error in that case here. */ +int verb_daemon_reload(int argc, char *argv[], void *userdata) { + enum action a; + int r; + + assert(argc >= 1); + + if (streq(argv[0], "daemon-reexec")) + a = ACTION_REEXEC; + else if (streq(argv[0], "daemon-reload")) + a = ACTION_RELOAD; + else + assert_not_reached(); + + r = daemon_reload(a, /* graceful= */ false); + if (r < 0) + return r; - return r < 0 ? r : 0; + return 0; } diff --git a/src/systemctl/systemctl-daemon-reload.h b/src/systemctl/systemctl-daemon-reload.h index d4215cc37c0..ced34ce44e3 100644 --- a/src/systemctl/systemctl-daemon-reload.h +++ b/src/systemctl/systemctl-daemon-reload.h @@ -1,4 +1,8 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ #pragma once +#include "systemctl.h" + +int daemon_reload(enum action, bool graceful); + int verb_daemon_reload(int argc, char *argv[], void *userdata); diff --git a/src/systemctl/systemctl-edit.c b/src/systemctl/systemctl-edit.c index 4b19bf7015e..f5123e80c12 100644 --- a/src/systemctl/systemctl-edit.c +++ b/src/systemctl/systemctl-edit.c @@ -569,8 +569,11 @@ int verb_edit(int argc, char *argv[], void *userdata) { r = 0; - if (!arg_no_reload && !install_client_side()) - r = verb_daemon_reload(argc, argv, userdata); + if (!arg_no_reload && !install_client_side()) { + r = daemon_reload(ACTION_RELOAD, /* graceful= */ false); + if (r > 0) + r = 0; + } end: STRV_FOREACH_PAIR(original, tmp, paths) { diff --git a/src/systemctl/systemctl-enable.c b/src/systemctl/systemctl-enable.c index f258d9643ba..3da2cd928ac 100644 --- a/src/systemctl/systemctl-enable.c +++ b/src/systemctl/systemctl-enable.c @@ -86,7 +86,9 @@ int verb_enable(int argc, char *argv[], void *userdata) { if (strv_isempty(names)) { if (arg_no_reload || install_client_side()) return 0; - return verb_daemon_reload(argc, argv, userdata); + + r = daemon_reload(ACTION_RELOAD, /* graceful= */ false); + return r > 0 ? 0 : r; } if (streq(verb, "disable")) { @@ -234,9 +236,11 @@ int verb_enable(int argc, char *argv[], void *userdata) { goto finish; /* Try to reload if enabled */ - if (!arg_no_reload) - r = verb_daemon_reload(argc, argv, userdata); - else + if (!arg_no_reload) { + r = daemon_reload(ACTION_RELOAD, /* graceful= */ false); + if (r > 0) + r = 0; + } else r = 0; } diff --git a/src/systemctl/systemctl-preset-all.c b/src/systemctl/systemctl-preset-all.c index fa98a9bf802..8e36ddc0c05 100644 --- a/src/systemctl/systemctl-preset-all.c +++ b/src/systemctl/systemctl-preset-all.c @@ -51,7 +51,9 @@ int verb_preset_all(int argc, char *argv[], void *userdata) { goto finish; } - r = verb_daemon_reload(argc, argv, userdata); + r = daemon_reload(ACTION_RELOAD, /* graceful= */ false); + if (r > 0) + r = 0; } finish: diff --git a/src/systemctl/systemctl-set-default.c b/src/systemctl/systemctl-set-default.c index db149e78cff..5f9186aa387 100644 --- a/src/systemctl/systemctl-set-default.c +++ b/src/systemctl/systemctl-set-default.c @@ -132,9 +132,11 @@ int verb_set_default(int argc, char *argv[], void *userdata) { goto finish; /* Try to reload if enabled */ - if (!arg_no_reload) - r = verb_daemon_reload(argc, argv, userdata); - else + if (!arg_no_reload) { + r = daemon_reload(ACTION_RELOAD, /* graceful= */ false); + if (r > 0) + r = 0; + } else r = 0; } diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index ec29e12e394..e81703ebe8e 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -1051,8 +1051,8 @@ static int systemctl_main(int argc, char *argv[]) { { "cat", 2, VERB_ANY, VERB_ONLINE_ONLY, verb_cat }, { "status", VERB_ANY, VERB_ANY, VERB_ONLINE_ONLY, verb_show }, { "help", VERB_ANY, VERB_ANY, VERB_ONLINE_ONLY, verb_show }, - { "daemon-reload", VERB_ANY, 1, VERB_ONLINE_ONLY, verb_daemon_reload }, - { "daemon-reexec", VERB_ANY, 1, VERB_ONLINE_ONLY, verb_daemon_reload }, + { "daemon-reload", 1, 1, VERB_ONLINE_ONLY, verb_daemon_reload }, + { "daemon-reexec", 1, 1, VERB_ONLINE_ONLY, verb_daemon_reload }, { "log-level", VERB_ANY, 2, VERB_ONLINE_ONLY, verb_log_setting }, { "log-target", VERB_ANY, 2, VERB_ONLINE_ONLY, verb_log_setting }, { "service-log-level", 2, 3, VERB_ONLINE_ONLY, verb_service_log_setting }, -- 2.47.3