From cd57038a30aa9447bde3af7111ac8dc517b38bbf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 13 Mar 2020 15:46:30 +0100 Subject: [PATCH 1/3] core: consistently emit oom warning when parsing args In practice we are very unlikely to fail at this point, but for consistency, we should always warn when allocation fails, and we have free_and_strdup_warn() for this. --- src/core/main.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/core/main.c b/src/core/main.c index 1a1e09035b..f7cda72fdd 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -318,7 +318,6 @@ static int set_machine_id(const char *m) { } static int parse_proc_cmdline_item(const char *key, const char *value, void *data) { - int r; assert(key); @@ -330,10 +329,8 @@ static int parse_proc_cmdline_item(const char *key, const char *value, void *dat if (!unit_name_is_valid(value, UNIT_NAME_PLAIN|UNIT_NAME_INSTANCE)) log_warning("Unit name specified on %s= is not valid, ignoring: %s", key, value); - else if (in_initrd() == !!startswith(key, "rd.")) { - if (free_and_strdup(&arg_default_unit, value) < 0) - return log_oom(); - } + else if (in_initrd() == !!startswith(key, "rd.")) + return free_and_strdup_warn(&arg_default_unit, value); } else if (proc_cmdline_key_streq(key, "systemd.dump_core")) { @@ -510,7 +507,7 @@ static int parse_proc_cmdline_item(const char *key, const char *value, void *dat /* SysV compatibility */ target = runlevel_to_target(key); if (target) - return free_and_strdup(&arg_default_unit, target); + return free_and_strdup_warn(&arg_default_unit, target); } return 0; @@ -987,11 +984,9 @@ static int parse_argv(int argc, char *argv[]) { case 'b': case 's': case 'z': - /* Just to eat away the sysvinit kernel - * cmdline args without getopt() error - * messages that we'll parse in - * parse_proc_cmdline_word() or ignore. */ - + /* Just to eat away the sysvinit kernel cmdline args that we'll parse in + * parse_proc_cmdline_item() or ignore, without any getopt() error messages. + */ case '?': if (getpid_cached() != 1) return -EINVAL; From 5e59431c53645a6f506fa4270fdc0063ee27c927 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 13 Mar 2020 15:50:37 +0100 Subject: [PATCH 2/3] systemctl: emit notice about some kernel commandline options Things can be pretty confusing when there's a commandline option overrding the configured default... Let's be nice to the user and emit a warning. --- src/systemctl/systemctl.c | 40 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 07f060e953..2cefcac693 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -2065,6 +2065,41 @@ static int list_machines(int argc, char *argv[], void *userdata) { return rc; } +static int parse_proc_cmdline_item(const char *key, const char *value, void *data) { + char **ret = data; + + if (streq(key, "systemd.unit")) { + if (proc_cmdline_value_missing(key, value)) + return 0; + if (!unit_name_is_valid(value, UNIT_NAME_PLAIN|UNIT_NAME_INSTANCE)) + return log_warning("Unit name specified on %s= is not valid, ignoring: %s", key, value); + + return free_and_strdup_warn(ret, key); + + } else if (!value) { + if (runlevel_to_target(key)) + return free_and_strdup_warn(ret, key); + } + + return 0; +} + +static void emit_cmdline_warning(void) { + if (arg_quiet || arg_root) + /* don't bother checking the commandline if we're operating on a container */ + return; + + _cleanup_free_ char *override = NULL; + int r; + + r = proc_cmdline_parse(parse_proc_cmdline_item, &override, 0); + if (r < 0) + log_debug_errno(r, "Failed to parse kernel command line, ignoring: %m"); + if (override) + log_notice("Note: found \"%s\" on the kernel commandline, which overrides the default unit.", + override); +} + static int get_default(int argc, char *argv[], void *userdata) { _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; _cleanup_free_ char *_path = NULL; @@ -2077,7 +2112,6 @@ static int get_default(int argc, char *argv[], void *userdata) { return log_error_errno(r, "Failed to get default target: %m"); path = _path; - r = 0; } else { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; sd_bus *bus; @@ -2106,6 +2140,8 @@ static int get_default(int argc, char *argv[], void *userdata) { if (path) printf("%s\n", path); + emit_cmdline_warning(); + return 0; } @@ -2164,6 +2200,8 @@ static int set_default(int argc, char *argv[], void *userdata) { r = 0; } + emit_cmdline_warning(); + finish: unit_file_changes_free(changes, n_changes); From 12877da21508c6e55e3421930ad1781b982e1155 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 13 Mar 2020 17:57:06 +0100 Subject: [PATCH 3/3] systemctl: print a notice when set-default is not effective MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit $ sudo ln -svf multi-user.target /run/systemd/generator.early/default.target '/run/systemd/generator.early/default.target' -> 'multi-user.target' $ sudo build/systemctl set-default --root=/ sysinit.target Removed /etc/systemd/system/default.target. Created symlink /etc/systemd/system/default.target → /usr/lib/systemd/system/sysinit.target. Note: "multi-user.target" is the default unit (possibly a runtime override). The output is not super informative, but it should be enough to point the user in the right direction. Fixes #3645. --- src/systemctl/systemctl.c | 42 +++++++++++++++++++++++++++++---------- 1 file changed, 31 insertions(+), 11 deletions(-) diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 2cefcac693..ba3fe9ec2f 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -2100,21 +2100,20 @@ static void emit_cmdline_warning(void) { override); } -static int get_default(int argc, char *argv[], void *userdata) { - _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; - _cleanup_free_ char *_path = NULL; - const char *path; +static int determine_default(char **ret_name) { int r; if (install_client_side()) { - r = unit_file_get_default(arg_scope, arg_root, &_path); + r = unit_file_get_default(arg_scope, arg_root, ret_name); if (r < 0) return log_error_errno(r, "Failed to get default target: %m"); - path = _path; + return 0; } else { - _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; sd_bus *bus; + _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; + _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; + const char *name; r = acquire_bus(BUS_MANAGER, &bus); if (r < 0) @@ -2132,13 +2131,23 @@ static int get_default(int argc, char *argv[], void *userdata) { if (r < 0) return log_error_errno(r, "Failed to get default target: %s", bus_error_message(&error, r)); - r = sd_bus_message_read(reply, "s", &path); + r = sd_bus_message_read(reply, "s", &name); if (r < 0) return bus_log_parse_error(r); - } - if (path) - printf("%s\n", path); + return free_and_strdup_warn(ret_name, name); + } +} + +static int get_default(int argc, char *argv[], void *userdata) { + _cleanup_free_ char *name = NULL; + int r; + + r = determine_default(&name); + if (r < 0) + return r; + + printf("%s\n", name); emit_cmdline_warning(); @@ -2202,6 +2211,17 @@ static int set_default(int argc, char *argv[], void *userdata) { emit_cmdline_warning(); + if (!arg_quiet) { + _cleanup_free_ char *final = NULL; + + r = determine_default(&final); + if (r < 0) + return r; + + if (!streq(final, unit)) + log_notice("Note: \"%s\" is the default unit (possibly a runtime override).", final); + } + finish: unit_file_changes_free(changes, n_changes);