From 0ff5985176acaccf4d2f220f92e14cd0f6ee82bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 26 Oct 2025 11:34:22 +0100 Subject: [PATCH 1/2] systemctl: convert return value of install_client_side() to enum The checks are reordered to do checks that don't require interacting with the system first. --- src/systemctl/systemctl-add-dependency.c | 2 +- src/systemctl/systemctl-edit.c | 3 +- src/systemctl/systemctl-enable.c | 74 +++++++++++------------ src/systemctl/systemctl-is-enabled.c | 2 +- src/systemctl/systemctl-list-unit-files.c | 2 +- src/systemctl/systemctl-preset-all.c | 2 +- src/systemctl/systemctl-set-default.c | 4 +- src/systemctl/systemctl-util.c | 35 +++++------ src/systemctl/systemctl-util.h | 11 +++- 9 files changed, 73 insertions(+), 62 deletions(-) diff --git a/src/systemctl/systemctl-add-dependency.c b/src/systemctl/systemctl-add-dependency.c index 9393bc4d6a..b1fd924209 100644 --- a/src/systemctl/systemctl-add-dependency.c +++ b/src/systemctl/systemctl-add-dependency.c @@ -44,7 +44,7 @@ int verb_add_dependency(int argc, char *argv[], void *userdata) { else assert_not_reached(); - if (install_client_side()) { + if (install_client_side() != INSTALL_CLIENT_SIDE_NO) { InstallChange *changes = NULL; size_t n_changes = 0; diff --git a/src/systemctl/systemctl-edit.c b/src/systemctl/systemctl-edit.c index 53bc57186a..ef2735ddca 100644 --- a/src/systemctl/systemctl-edit.c +++ b/src/systemctl/systemctl-edit.c @@ -370,7 +370,8 @@ int verb_edit(int argc, char *argv[], void *userdata) { if (r < 0) return r; - if (!arg_no_reload && !install_client_side()) { + if (!arg_no_reload && + install_client_side() == INSTALL_CLIENT_SIDE_NO) { r = daemon_reload(ACTION_RELOAD, /* graceful= */ false); if (r < 0) return r; diff --git a/src/systemctl/systemctl-enable.c b/src/systemctl/systemctl-enable.c index 59b1438042..177e4aaa84 100644 --- a/src/systemctl/systemctl-enable.c +++ b/src/systemctl/systemctl-enable.c @@ -103,7 +103,7 @@ int verb_enable(int argc, char *argv[], void *userdata) { /* If the operation was fully executed by the SysV compat, let's finish early */ if (strv_isempty(names)) { - if (arg_no_reload || install_client_side()) + if (arg_no_reload || install_client_side() != INSTALL_CLIENT_SIDE_NO) return 0; r = daemon_reload(ACTION_RELOAD, /* graceful= */ false); @@ -119,41 +119,7 @@ int verb_enable(int argc, char *argv[], void *userdata) { if (r < 0) return r; - if (install_client_side()) { - UnitFileFlags flags; - InstallChange *changes = NULL; - size_t n_changes = 0; - - CLEANUP_ARRAY(changes, n_changes, install_changes_free); - - flags = unit_file_flags_from_args(); - - if (streq(verb, "enable")) { - r = unit_file_enable(arg_runtime_scope, flags, arg_root, names, &changes, &n_changes); - carries_install_info = r; - } else if (streq(verb, "disable")) { - r = unit_file_disable(arg_runtime_scope, flags, arg_root, names, &changes, &n_changes); - carries_install_info = r; - } else if (streq(verb, "reenable")) { - r = unit_file_reenable(arg_runtime_scope, flags, arg_root, names, &changes, &n_changes); - carries_install_info = r; - } else if (streq(verb, "link")) - r = unit_file_link(arg_runtime_scope, flags, arg_root, names, &changes, &n_changes); - else if (streq(verb, "preset")) - r = unit_file_preset(arg_runtime_scope, flags, arg_root, names, arg_preset_mode, &changes, &n_changes); - else if (streq(verb, "mask")) - r = unit_file_mask(arg_runtime_scope, flags, arg_root, names, &changes, &n_changes); - else if (streq(verb, "unmask")) - r = unit_file_unmask(arg_runtime_scope, flags, arg_root, names, &changes, &n_changes); - else if (streq(verb, "revert")) - r = unit_file_revert(arg_runtime_scope, arg_root, names, &changes, &n_changes); - else - assert_not_reached(); - - install_changes_dump(r, verb, changes, n_changes, arg_quiet); - if (r < 0) - return r; - } else { + if (install_client_side() == INSTALL_CLIENT_SIDE_NO) { _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL, *m = NULL; _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; bool expect_carries_install_info = false; @@ -275,6 +241,40 @@ int verb_enable(int argc, char *argv[], void *userdata) { if (warn_trigger_operation && !arg_quiet && !arg_no_warn) STRV_FOREACH(unit, names) warn_triggering_units(bus, *unit, warn_trigger_operation, warn_trigger_ignore_masked); + } else { + UnitFileFlags flags; + InstallChange *changes = NULL; + size_t n_changes = 0; + + CLEANUP_ARRAY(changes, n_changes, install_changes_free); + + flags = unit_file_flags_from_args(); + + if (streq(verb, "enable")) { + r = unit_file_enable(arg_runtime_scope, flags, arg_root, names, &changes, &n_changes); + carries_install_info = r; + } else if (streq(verb, "disable")) { + r = unit_file_disable(arg_runtime_scope, flags, arg_root, names, &changes, &n_changes); + carries_install_info = r; + } else if (streq(verb, "reenable")) { + r = unit_file_reenable(arg_runtime_scope, flags, arg_root, names, &changes, &n_changes); + carries_install_info = r; + } else if (streq(verb, "link")) + r = unit_file_link(arg_runtime_scope, flags, arg_root, names, &changes, &n_changes); + else if (streq(verb, "preset")) + r = unit_file_preset(arg_runtime_scope, flags, arg_root, names, arg_preset_mode, &changes, &n_changes); + else if (streq(verb, "mask")) + r = unit_file_mask(arg_runtime_scope, flags, arg_root, names, &changes, &n_changes); + else if (streq(verb, "unmask")) + r = unit_file_unmask(arg_runtime_scope, flags, arg_root, names, &changes, &n_changes); + else if (streq(verb, "revert")) + r = unit_file_revert(arg_runtime_scope, arg_root, names, &changes, &n_changes); + else + assert_not_reached(); + + install_changes_dump(r, verb, changes, n_changes, arg_quiet); + if (r < 0) + return r; } if (carries_install_info == 0 && !ignore_carries_install_info) @@ -354,7 +354,7 @@ int verb_enable(int argc, char *argv[], void *userdata) { return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "--now can only be used with verb enable, disable, reenable, or mask."); - if (install_client_side()) + if (install_client_side() != INSTALL_CLIENT_SIDE_NO) return log_error_errno(SYNTHETIC_ERRNO(EREMOTE), "--now cannot be used when systemd is not running or in conjunction with --root=/--global, refusing."); diff --git a/src/systemctl/systemctl-is-enabled.c b/src/systemctl/systemctl-is-enabled.c index 32966e24a2..da028b1385 100644 --- a/src/systemctl/systemctl-is-enabled.c +++ b/src/systemctl/systemctl-is-enabled.c @@ -82,7 +82,7 @@ int verb_is_enabled(int argc, char *argv[], void *userdata) { not_found = r == 0; /* Doesn't have SysV support or SYSV_UNIT_NOT_FOUND */ enabled = r == SYSV_UNIT_ENABLED; - if (install_client_side()) + if (install_client_side() != INSTALL_CLIENT_SIDE_NO) STRV_FOREACH(name, names) { UnitFileState state; diff --git a/src/systemctl/systemctl-list-unit-files.c b/src/systemctl/systemctl-list-unit-files.c index 5759cf6367..548b2573fc 100644 --- a/src/systemctl/systemctl-list-unit-files.c +++ b/src/systemctl/systemctl-list-unit-files.c @@ -180,7 +180,7 @@ int verb_list_unit_files(int argc, char *argv[], void *userdata) { unsigned c = 0; int r; - if (install_client_side()) { + if (install_client_side() != INSTALL_CLIENT_SIDE_NO) { unsigned n_units; r = unit_file_get_list(arg_runtime_scope, arg_root, arg_states, strv_skip(argv, 1), &h); diff --git a/src/systemctl/systemctl-preset-all.c b/src/systemctl/systemctl-preset-all.c index 20079d1004..b9c4a6d5f3 100644 --- a/src/systemctl/systemctl-preset-all.c +++ b/src/systemctl/systemctl-preset-all.c @@ -19,7 +19,7 @@ int verb_preset_all(int argc, char *argv[], void *userdata) { if (should_bypass("SYSTEMD_PRESET")) return 0; - if (install_client_side()) { + if (install_client_side() != INSTALL_CLIENT_SIDE_NO) { InstallChange *changes = NULL; size_t n_changes = 0; diff --git a/src/systemctl/systemctl-set-default.c b/src/systemctl/systemctl-set-default.c index ca0f2575cd..3a8e3969e2 100644 --- a/src/systemctl/systemctl-set-default.c +++ b/src/systemctl/systemctl-set-default.c @@ -58,7 +58,7 @@ static void emit_cmdline_warning(void) { static int determine_default(char **ret_name) { int r; - if (install_client_side()) { + if (install_client_side() != INSTALL_CLIENT_SIDE_NO) { r = unit_file_get_default(arg_runtime_scope, arg_root, ret_name); if (r == -ERFKILL) return log_error_errno(r, "Failed to get default target: Unit file is masked."); @@ -116,7 +116,7 @@ int verb_set_default(int argc, char *argv[], void *userdata) { if (r < 0) return log_error_errno(r, "Failed to mangle unit name: %m"); - if (install_client_side()) { + if (install_client_side() != INSTALL_CLIENT_SIDE_NO) { InstallChange *changes = NULL; size_t n_changes = 0; diff --git a/src/systemctl/systemctl-util.c b/src/systemctl/systemctl-util.c index faefc55619..71f14b8abf 100644 --- a/src/systemctl/systemctl-util.c +++ b/src/systemctl/systemctl-util.c @@ -523,7 +523,7 @@ int unit_find_paths( /* Go via the bus to acquire the path, unless we are explicitly told not to, or when the unit name is a template */ if (!force_client_side && - !install_client_side() && + install_client_side() == INSTALL_CLIENT_SIDE_NO && !unit_name_is_valid(unit_name, UNIT_NAME_TEMPLATE)) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_free_ char *load_state = NULL, *dbus_path = NULL; @@ -880,26 +880,27 @@ bool output_show_unit(const UnitInfo *u, char **patterns) { return true; } -bool install_client_side(void) { - /* Decides when to execute enable/disable/... operations client-side rather than server-side. */ - - if (running_in_chroot_or_offline()) - return true; - - if (sd_booted() <= 0) - return true; - - if (!isempty(arg_root)) - return true; - - if (arg_runtime_scope == RUNTIME_SCOPE_GLOBAL) - return true; +InstallClientSide install_client_side(void) { + /* Decides whether to execute enable/disable/… client-side offline operation rather than + * server-side. */ /* Unsupported environment variable, mostly for debugging purposes */ if (getenv_bool("SYSTEMCTL_INSTALL_CLIENT_SIDE") > 0) - return true; + return INSTALL_CLIENT_SIDE_OVERRIDE; - return false; + if (!isempty(arg_root)) + return INSTALL_CLIENT_SIDE_ARG_ROOT; + + if (running_in_chroot_or_offline()) + return INSTALL_CLIENT_SIDE_OFFLINE; + + if (sd_booted() <= 0) + return INSTALL_CLIENT_SIDE_NOT_BOOTED; + + if (arg_runtime_scope == RUNTIME_SCOPE_GLOBAL) + return INSTALL_CLIENT_SIDE_GLOBAL_SCOPE; + + return INSTALL_CLIENT_SIDE_NO; } int output_table(Table *table) { diff --git a/src/systemctl/systemctl-util.h b/src/systemctl/systemctl-util.h index f2560b6268..fbeb25aa7a 100644 --- a/src/systemctl/systemctl-util.h +++ b/src/systemctl/systemctl-util.h @@ -69,7 +69,16 @@ int unit_get_dependencies(sd_bus *bus, const char *name, char ***ret); const char* unit_type_suffix(const char *unit); bool output_show_unit(const UnitInfo *u, char **patterns); -bool install_client_side(void); +typedef enum InstallClientSide { + INSTALL_CLIENT_SIDE_NO = 0, + INSTALL_CLIENT_SIDE_OVERRIDE, + INSTALL_CLIENT_SIDE_ARG_ROOT, + INSTALL_CLIENT_SIDE_OFFLINE, + INSTALL_CLIENT_SIDE_NOT_BOOTED, + INSTALL_CLIENT_SIDE_GLOBAL_SCOPE, +} InstallClientSide; + +InstallClientSide install_client_side(void); int output_table(Table *table); From 77a1cc8fa09c264991d147ec71d70a4b5d2a553e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 26 Oct 2025 11:57:56 +0100 Subject: [PATCH 2/2] systemctl: downgrade or silence warnings for --now When calling systemctl enable/disable/reenable --now, we'd always fail with error when operating offline. This seemly overly restricitive. In particular, if systemd is not running at all, the service is not running either, so complaining that we can't stop it is completely unnecessary. But even when operating in a chroot where systemd is not running, let's just emit a warning and return success. It's fairly common to have installation or package scripts which do such calls and not starting/restarting the service in those scenarios is the desired and expected operation. (If --now is called in combination with --global or --root=, keep returning an error.) Also make the messages nicer. I was adding some docs to tell the user to run 'systemctl enable --now', and checked how the command can fail, and the error message that the user might see in some common scenarios was too complicated. Split it up to be nicer. --- src/systemctl/systemctl-enable.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/systemctl/systemctl-enable.c b/src/systemctl/systemctl-enable.c index 177e4aaa84..c760b917f6 100644 --- a/src/systemctl/systemctl-enable.c +++ b/src/systemctl/systemctl-enable.c @@ -334,7 +334,7 @@ int verb_enable(int argc, char *argv[], void *userdata) { if (arg_now) { _cleanup_strv_free_ char **new_args = NULL; const char *start_verb; - bool accept_path, prohibit_templates; + bool accept_path, prohibit_templates, dead_ok = false; if (streq(verb, "enable")) { start_verb = "start"; @@ -344,6 +344,7 @@ int verb_enable(int argc, char *argv[], void *userdata) { start_verb = "stop"; accept_path = false; prohibit_templates = false; + dead_ok = true; /* If the service is not running anyway, no need to stop it. */ } else if (streq(verb, "reenable")) { /* Note that we use try-restart here. This matches the semantics of reenable better, * and allows us to glob template units. */ @@ -354,9 +355,20 @@ int verb_enable(int argc, char *argv[], void *userdata) { return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "--now can only be used with verb enable, disable, reenable, or mask."); - if (install_client_side() != INSTALL_CLIENT_SIDE_NO) - return log_error_errno(SYNTHETIC_ERRNO(EREMOTE), - "--now cannot be used when systemd is not running or in conjunction with --root=/--global, refusing."); + switch (install_client_side()) { + case INSTALL_CLIENT_SIDE_NO: + break; + case INSTALL_CLIENT_SIDE_OVERRIDE: + case INSTALL_CLIENT_SIDE_OFFLINE: + case INSTALL_CLIENT_SIDE_NOT_BOOTED: + if (!dead_ok) + log_warning("Cannot %s unit with --now when systemd is not running, ignoring.", start_verb); + return 0; + case INSTALL_CLIENT_SIDE_ARG_ROOT: + return log_error_errno(SYNTHETIC_ERRNO(EREMOTE), "--now cannot be used with --root=."); + case INSTALL_CLIENT_SIDE_GLOBAL_SCOPE: + return log_error_errno(SYNTHETIC_ERRNO(EREMOTE), "--now cannot be used with --global."); + } assert(bus);