From 26b8190841fa6e00a66a4a46b343bde117b855fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 29 Apr 2020 18:22:46 +0200 Subject: [PATCH 01/14] man: mention that ProtectSystem= also takes care of /efi --- man/systemd.exec.xml | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/man/systemd.exec.xml b/man/systemd.exec.xml index 3e2ae93bf0..4818f3423c 100644 --- a/man/systemd.exec.xml +++ b/man/systemd.exec.xml @@ -891,10 +891,11 @@ CapabilityBoundingSet=~CAP_B CAP_C ProtectSystem= Takes a boolean argument or the special values full or - strict. If true, mounts the /usr and /boot - directories read-only for processes invoked by this unit. If set to full, the - /etc directory is mounted read-only, too. If set to strict the entire - file system hierarchy is mounted read-only, except for the API file system subtrees /dev, + strict. If true, mounts the /usr and the boot loader + directories (/boot and /efi) read-only for processes + invoked by this unit. If set to full, the /etc directory is + mounted read-only, too. If set to strict the entire file system hierarchy is + mounted read-only, except for the API file system subtrees /dev, /proc and /sys (protect these directories using PrivateDevices=, ProtectKernelTunables=, ProtectControlGroups=). This setting ensures that any modification of the vendor-supplied From b8239b9c501f6d08e918e74f0af301b2cc472b89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 30 Apr 2020 12:02:44 +0200 Subject: [PATCH 02/14] shared/unit-file: fix resolution of absoulute symlinks with --root $ systemctl --no-pager --root /tmp/root2/ cat ctrl-alt-del.target Failed to resolve symlink /tmp/root2/etc/systemd/system/ctrl-alt-del.target pointing to /usr/lib/systemd/system/reboot.target, ignoring: Channel number out of range ... --- src/shared/unit-file.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/shared/unit-file.c b/src/shared/unit-file.c index 7ad0ecc8fd..7b64bbf7f1 100644 --- a/src/shared/unit-file.c +++ b/src/shared/unit-file.c @@ -320,7 +320,7 @@ int unit_file_build_name_map( /* We don't explicitly check for alias loops here. unit_ids_map_get() which * limits the number of hops should be used to access the map. */ - _cleanup_free_ char *target = NULL, *target_abs = NULL; + _cleanup_free_ char *target = NULL; r = readlinkat_malloc(dirfd(d), de->d_name, &target); if (r < 0) { @@ -329,8 +329,9 @@ int unit_file_build_name_map( continue; } - if (!path_is_absolute(target)) { - target_abs = path_join(*dir, target); + const bool is_abs = path_is_absolute(target); + if (lp->root_dir || !is_abs) { + char *target_abs = path_join(is_abs ? lp->root_dir : *dir, target); if (!target_abs) return log_oom(); From 77102db288e533050397128da182b081a98211f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 30 Apr 2020 12:38:36 +0200 Subject: [PATCH 03/14] systemctl: fix hint when 'systemctl help' is given Not all verbs require unit names, but that is beside the point. We need a verb here, and help is not a valid verb. --- src/shared/verbs.c | 4 ++-- src/systemctl/systemctl.c | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/shared/verbs.c b/src/shared/verbs.c index 1d23ed48a2..06d89b4736 100644 --- a/src/shared/verbs.c +++ b/src/shared/verbs.c @@ -68,9 +68,9 @@ int dispatch_verb(int argc, char *argv[], const Verb verbs[], void *userdata) { /* At the end of the list? */ if (!verbs[i].dispatch) { if (name) - log_error("Unknown operation %s.", name); + log_error("Unknown command verb %s.", name); else - log_error("Requires operation parameter."); + log_error("Command verb required."); return -EINVAL; } diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index d319d5d375..d5dc52853a 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -5860,7 +5860,8 @@ static int show(int argc, char *argv[], void *userdata) { if (show_mode == SYSTEMCTL_SHOW_HELP && argc <= 1) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), - "This command expects one or more unit names. Did you mean --help?"); + "'help' command expects one or more unit names.\n" + "(Alternatively, help for systemctl itself may be shown with --help)"); r = acquire_bus(BUS_MANAGER, &bus); if (r < 0) From ae98f726ffc58ed8b1e80c8054fca10d6c9a05e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 30 Apr 2020 13:31:24 +0200 Subject: [PATCH 04/14] systemctl: mark log-level,log-target,service-watchdogs as online-only They were added in 6ab863190dee5ab631795ef547fcc7314ddccd40 and 38fcb7f766c84736425e86854b8a4468c126dafa. I don't see a good reason to exclude them from the online-only check. --- src/systemctl/systemctl.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index d5dc52853a..0a3d975d88 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -9171,9 +9171,9 @@ static int systemctl_main(int argc, char *argv[]) { { "help", VERB_ANY, VERB_ANY, VERB_ONLINE_ONLY, show }, { "daemon-reload", VERB_ANY, 1, VERB_ONLINE_ONLY, daemon_reload }, { "daemon-reexec", VERB_ANY, 1, VERB_ONLINE_ONLY, daemon_reload }, - { "log-level", VERB_ANY, 2, 0, log_level }, - { "log-target", VERB_ANY, 2, 0, log_target }, - { "service-watchdogs", VERB_ANY, 2, 0, service_watchdogs }, + { "log-level", VERB_ANY, 2, VERB_ONLINE_ONLY, log_level }, + { "log-target", VERB_ANY, 2, VERB_ONLINE_ONLY, log_target }, + { "service-watchdogs", VERB_ANY, 2, VERB_ONLINE_ONLY, service_watchdogs }, { "show-environment", VERB_ANY, 1, VERB_ONLINE_ONLY, show_environment }, { "set-environment", 2, VERB_ANY, VERB_ONLINE_ONLY, set_environment }, { "unset-environment", 2, VERB_ANY, VERB_ONLINE_ONLY, set_environment }, From d8b065e0561e0180d34ddae580ec1b17929d78e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 30 Apr 2020 13:59:33 +0200 Subject: [PATCH 05/14] shared/verbs: add a modicum of documentation --- src/shared/verbs.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/shared/verbs.h b/src/shared/verbs.h index c5fe6cc7c5..fd35a2e0dd 100644 --- a/src/shared/verbs.h +++ b/src/shared/verbs.h @@ -6,8 +6,8 @@ #define VERB_ANY ((unsigned) -1) typedef enum VerbFlags { - VERB_DEFAULT = 1 << 0, - VERB_ONLINE_ONLY = 1 << 1, + VERB_DEFAULT = 1 << 0, /* The verb to run if no verb is specified */ + VERB_ONLINE_ONLY = 1 << 1, /* Just do nothing when running in chroot or offline */ } VerbFlags; typedef struct { From 9321e23c40f3dc6bd785105ce882cbad6447a1c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 30 Apr 2020 14:20:31 +0200 Subject: [PATCH 06/14] shared/verbs: split out helper to find verbs It will be used later, but I think it makes the code clearer anyway. Also change the message about ignoring to include the name for default verbs. --- src/shared/verbs.c | 43 +++++++++++++++++-------------------------- src/shared/verbs.h | 1 + 2 files changed, 18 insertions(+), 26 deletions(-) diff --git a/src/shared/verbs.c b/src/shared/verbs.c index 06d89b4736..d2744b6918 100644 --- a/src/shared/verbs.c +++ b/src/shared/verbs.c @@ -45,10 +45,19 @@ bool running_in_chroot_or_offline(void) { return r > 0; } +const Verb* verbs_find_verb(const char *name, const Verb verbs[]) { + for (size_t i = 0; verbs[i].dispatch; i++) + if (streq_ptr(name, verbs[i].verb) || + (!name && FLAGS_SET(verbs[i].flags, VERB_DEFAULT))) + return &verbs[i]; + + /* At the end of the list? */ + return NULL; +} + int dispatch_verb(int argc, char *argv[], const Verb verbs[], void *userdata) { const Verb *verb; const char *name; - unsigned i; int left; assert(verbs); @@ -62,31 +71,16 @@ int dispatch_verb(int argc, char *argv[], const Verb verbs[], void *userdata) { optind = 0; name = argv[0]; - for (i = 0;; i++) { - bool found; - - /* At the end of the list? */ - if (!verbs[i].dispatch) { - if (name) - log_error("Unknown command verb %s.", name); - else - log_error("Command verb required."); - return -EINVAL; - } - + verb = verbs_find_verb(name, verbs); + if (!verb) { if (name) - found = streq(name, verbs[i].verb); + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "Unknown command verb %s.", name); else - found = verbs[i].flags & VERB_DEFAULT; - - if (found) { - verb = &verbs[i]; - break; - } + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "Command verb required."); } - assert(verb); - if (!name) left = 1; @@ -101,10 +95,7 @@ int dispatch_verb(int argc, char *argv[], const Verb verbs[], void *userdata) { "Too many arguments."); if ((verb->flags & VERB_ONLINE_ONLY) && running_in_chroot_or_offline()) { - if (name) - log_info("Running in chroot, ignoring request: %s", name); - else - log_info("Running in chroot, ignoring request."); + log_info("Running in chroot, ignoring command '%s'", name ?: verb->verb); return 0; } diff --git a/src/shared/verbs.h b/src/shared/verbs.h index fd35a2e0dd..b6a1afcdee 100644 --- a/src/shared/verbs.h +++ b/src/shared/verbs.h @@ -19,4 +19,5 @@ typedef struct { bool running_in_chroot_or_offline(void); +const Verb* verbs_find_verb(const char *name, const Verb verbs[]); int dispatch_verb(int argc, char *argv[], const Verb verbs[], void *userdata); From 4cb1015e6b815f72ae7ba4b04d8e7f856e9c51fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 30 Apr 2020 14:26:36 +0200 Subject: [PATCH 07/14] systemctl: refuse online-only ops with --root It is super confusing when a command does not support --root, and is called with it specified, and returns some bogus results. Let's just catch this early and refuse. --- src/systemctl/systemctl.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 0a3d975d88..077609f4de 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -9213,6 +9213,12 @@ static int systemctl_main(int argc, char *argv[]) { {} }; + const Verb *verb = verbs_find_verb(argv[optind], verbs); + if (verb && (verb->flags & VERB_ONLINE_ONLY) && arg_root) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "Verb '%s' cannot be used with --root=.", + argv[optind] ?: verb->verb); + return dispatch_verb(argc, argv, verbs, NULL); } From c39b7821f6d24d0b096ee347c6e1e2708391438f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 30 Apr 2020 18:45:08 +0200 Subject: [PATCH 08/14] systemctl: fix --root support in querying presets We would always look on the host, ignoring --root. --- src/systemctl/systemctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 077609f4de..3653e5c81b 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -1478,7 +1478,7 @@ static int output_unit_file_list(const UnitFileList *units, unsigned c) { id = basename(u->path); - r = unit_file_query_preset(arg_scope, NULL, id); + r = unit_file_query_preset(arg_scope, arg_root, id); if (r < 0) { unit_preset_str = "n/a"; on_preset_color = underline ? on_underline : ansi_normal(); From 8f7b256665a3e6d154e8f1987e2b79c723c761ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 30 Apr 2020 22:37:34 +0200 Subject: [PATCH 09/14] shared/install: optionally cache the preset list When doing list-unit-files with --root, we would re-read the preset list for every unit. This uses a cache to only do it once. The time for list-unit-files goes down by about ~30%. unit_file_query_preset() is also called from src/core/. This patch does not touch that path, since the saving there are smaller, since preset status is only read on demand over dbus, and caching would be more complicated. --- src/core/unit.c | 3 +- src/shared/install.c | 62 +++++++++++++++++++-------------------- src/shared/install.h | 11 ++++++- src/systemctl/systemctl.c | 6 ++-- 4 files changed, 46 insertions(+), 36 deletions(-) diff --git a/src/core/unit.c b/src/core/unit.c index 6a33657b88..ca96afd778 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -4267,7 +4267,8 @@ int unit_get_unit_file_preset(Unit *u) { u->unit_file_preset = unit_file_query_preset( u->manager->unit_file_scope, NULL, - basename(u->fragment_path)); + basename(u->fragment_path), + NULL); return u->unit_file_preset; } diff --git a/src/shared/install.c b/src/shared/install.c index 236f279ffb..43ec315c2c 100644 --- a/src/shared/install.c +++ b/src/shared/install.c @@ -55,16 +55,11 @@ typedef enum { PRESET_DISABLE, } PresetAction; -typedef struct { +struct UnitFilePresetRule { char *pattern; PresetAction action; char **instances; -} PresetRule; - -typedef struct { - PresetRule *rules; - size_t n_rules; -} Presets; +}; static bool unit_file_install_info_has_rules(const UnitFileInstallInfo *i) { assert(i); @@ -80,7 +75,7 @@ static bool unit_file_install_info_has_also(const UnitFileInstallInfo *i) { return !strv_isempty(i->also); } -static void presets_freep(Presets *p) { +void unit_file_presets_freep(UnitFilePresets *p) { size_t i; if (!p) @@ -2913,8 +2908,8 @@ static int presets_find_config(UnitFileScope scope, const char *root_dir, char * return conf_files_list_strv(files, ".preset", root_dir, 0, dirs); } -static int read_presets(UnitFileScope scope, const char *root_dir, Presets *presets) { - _cleanup_(presets_freep) Presets ps = {}; +static int read_presets(UnitFileScope scope, const char *root_dir, UnitFilePresets *presets) { + _cleanup_(unit_file_presets_freep) UnitFilePresets ps = {}; size_t n_allocated = 0; _cleanup_strv_free_ char **files = NULL; char **p; @@ -2942,7 +2937,7 @@ static int read_presets(UnitFileScope scope, const char *root_dir, Presets *pres for (;;) { _cleanup_free_ char *line = NULL; - PresetRule rule = {}; + UnitFilePresetRule rule = {}; const char *parameter; char *l; @@ -2972,7 +2967,7 @@ static int read_presets(UnitFileScope scope, const char *root_dir, Presets *pres continue; } - rule = (PresetRule) { + rule = (UnitFilePresetRule) { .pattern = unit_name, .action = PRESET_ENABLE, .instances = instances, @@ -2987,7 +2982,7 @@ static int read_presets(UnitFileScope scope, const char *root_dir, Presets *pres if (!pattern) return -ENOMEM; - rule = (PresetRule) { + rule = (UnitFilePresetRule) { .pattern = pattern, .action = PRESET_DISABLE, }; @@ -3005,14 +3000,15 @@ static int read_presets(UnitFileScope scope, const char *root_dir, Presets *pres } } + ps.initialized = true; *presets = ps; - ps = (Presets){}; + ps = (UnitFilePresets){}; return 0; } static int pattern_match_multiple_instances( - const PresetRule rule, + const UnitFilePresetRule rule, const char *unit_name, char ***ret) { @@ -3066,17 +3062,17 @@ static int pattern_match_multiple_instances( return 0; } -static int query_presets(const char *name, const Presets presets, char ***instance_name_list) { +static int query_presets(const char *name, const UnitFilePresets *presets, char ***instance_name_list) { PresetAction action = PRESET_UNKNOWN; size_t i; char **s; if (!unit_name_is_valid(name, UNIT_NAME_ANY)) return -EINVAL; - for (i = 0; i < presets.n_rules; i++) - if (pattern_match_multiple_instances(presets.rules[i], name, instance_name_list) > 0 || - fnmatch(presets.rules[i].pattern, name, FNM_NOESCAPE) == 0) { - action = presets.rules[i].action; + for (i = 0; i < presets->n_rules; i++) + if (pattern_match_multiple_instances(presets->rules[i], name, instance_name_list) > 0 || + fnmatch(presets->rules[i].pattern, name, FNM_NOESCAPE) == 0) { + action = presets->rules[i].action; break; } @@ -3099,15 +3095,19 @@ static int query_presets(const char *name, const Presets presets, char ***instan } } -int unit_file_query_preset(UnitFileScope scope, const char *root_dir, const char *name) { - _cleanup_(presets_freep) Presets presets = {}; +int unit_file_query_preset(UnitFileScope scope, const char *root_dir, const char *name, UnitFilePresets *cached) { + _cleanup_(unit_file_presets_freep) UnitFilePresets tmp = {}; int r; - r = read_presets(scope, root_dir, &presets); - if (r < 0) - return r; + if (!cached) + cached = &tmp; + if (!cached->initialized) { + r = read_presets(scope, root_dir, cached); + if (r < 0) + return r; + } - return query_presets(name, presets, NULL); + return query_presets(name, cached, NULL); } static int execute_preset( @@ -3162,7 +3162,7 @@ static int preset_prepare_one( InstallContext *minus, LookupPaths *paths, const char *name, - Presets presets, + const UnitFilePresets *presets, UnitFileChange **changes, size_t *n_changes) { @@ -3221,7 +3221,7 @@ int unit_file_preset( _cleanup_(install_context_done) InstallContext plus = {}, minus = {}; _cleanup_(lookup_paths_free) LookupPaths paths = {}; - _cleanup_(presets_freep) Presets presets = {}; + _cleanup_(unit_file_presets_freep) UnitFilePresets presets = {}; const char *config_path; char **i; int r; @@ -3243,7 +3243,7 @@ int unit_file_preset( return r; STRV_FOREACH(i, files) { - r = preset_prepare_one(scope, &plus, &minus, &paths, *i, presets, changes, n_changes); + r = preset_prepare_one(scope, &plus, &minus, &paths, *i, &presets, changes, n_changes); if (r < 0) return r; } @@ -3261,7 +3261,7 @@ int unit_file_preset_all( _cleanup_(install_context_done) InstallContext plus = {}, minus = {}; _cleanup_(lookup_paths_free) LookupPaths paths = {}; - _cleanup_(presets_freep) Presets presets = {}; + _cleanup_(unit_file_presets_freep) UnitFilePresets presets = {}; const char *config_path = NULL; char **i; int r; @@ -3305,7 +3305,7 @@ int unit_file_preset_all( continue; /* we don't pass changes[] in, because we want to handle errors on our own */ - r = preset_prepare_one(scope, &plus, &minus, &paths, de->d_name, presets, NULL, 0); + r = preset_prepare_one(scope, &plus, &minus, &paths, de->d_name, &presets, NULL, 0); if (r == -ERFKILL) r = unit_file_changes_add(changes, n_changes, UNIT_FILE_IS_MASKED, de->d_name, NULL); diff --git a/src/shared/install.h b/src/shared/install.h index 54d22a45d3..5ff8bec165 100644 --- a/src/shared/install.h +++ b/src/shared/install.h @@ -189,7 +189,16 @@ void unit_file_dump_changes(int r, const char *verb, const UnitFileChange *chang int unit_file_verify_alias(const UnitFileInstallInfo *i, const char *dst, char **ret_dst); -int unit_file_query_preset(UnitFileScope scope, const char *root_dir, const char *name); +typedef struct UnitFilePresetRule UnitFilePresetRule; + +typedef struct { + UnitFilePresetRule *rules; + size_t n_rules; + bool initialized; +} UnitFilePresets; + +void unit_file_presets_freep(UnitFilePresets *p); +int unit_file_query_preset(UnitFileScope scope, const char *root_dir, const char *name, UnitFilePresets *cached); const char *unit_file_state_to_string(UnitFileState s) _const_; UnitFileState unit_file_state_from_string(const char *s) _pure_; diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 3653e5c81b..ac46ac88e0 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -1443,7 +1443,7 @@ static bool output_show_unit_file(const UnitFileList *u, char **states, char **p static int output_unit_file_list(const UnitFileList *units, unsigned c) { _cleanup_(table_unrefp) Table *table = NULL; - const UnitFileList *u; + _cleanup_(unit_file_presets_freep) UnitFilePresets presets = {}; int r; table = table_new("unit file", "state", "vendor preset"); @@ -1454,7 +1454,7 @@ static int output_unit_file_list(const UnitFileList *units, unsigned c) { if (arg_full) table_set_width(table, 0); - for (u = units; u < units + c; u++) { + for (const UnitFileList *u = units; u < units + c; u++) { const char *on_underline = NULL, *on_unit_color = NULL, *id; const char *on_preset_color = NULL, *unit_preset_str; bool underline; @@ -1478,7 +1478,7 @@ static int output_unit_file_list(const UnitFileList *units, unsigned c) { id = basename(u->path); - r = unit_file_query_preset(arg_scope, arg_root, id); + r = unit_file_query_preset(arg_scope, arg_root, id, &presets); if (r < 0) { unit_preset_str = "n/a"; on_preset_color = underline ? on_underline : ansi_normal(); From babccf14ced5d68f37882f443bcd04edaba3f4cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 4 May 2020 14:25:58 +0200 Subject: [PATCH 10/14] shared/install: indent tables --- src/shared/install.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/shared/install.c b/src/shared/install.c index 43ec315c2c..a692aed96e 100644 --- a/src/shared/install.c +++ b/src/shared/install.c @@ -3416,34 +3416,34 @@ int unit_file_get_list( } static const char* const unit_file_state_table[_UNIT_FILE_STATE_MAX] = { - [UNIT_FILE_ENABLED] = "enabled", + [UNIT_FILE_ENABLED] = "enabled", [UNIT_FILE_ENABLED_RUNTIME] = "enabled-runtime", - [UNIT_FILE_LINKED] = "linked", - [UNIT_FILE_LINKED_RUNTIME] = "linked-runtime", - [UNIT_FILE_MASKED] = "masked", - [UNIT_FILE_MASKED_RUNTIME] = "masked-runtime", - [UNIT_FILE_STATIC] = "static", - [UNIT_FILE_DISABLED] = "disabled", - [UNIT_FILE_INDIRECT] = "indirect", - [UNIT_FILE_GENERATED] = "generated", - [UNIT_FILE_TRANSIENT] = "transient", - [UNIT_FILE_BAD] = "bad", + [UNIT_FILE_LINKED] = "linked", + [UNIT_FILE_LINKED_RUNTIME] = "linked-runtime", + [UNIT_FILE_MASKED] = "masked", + [UNIT_FILE_MASKED_RUNTIME] = "masked-runtime", + [UNIT_FILE_STATIC] = "static", + [UNIT_FILE_DISABLED] = "disabled", + [UNIT_FILE_INDIRECT] = "indirect", + [UNIT_FILE_GENERATED] = "generated", + [UNIT_FILE_TRANSIENT] = "transient", + [UNIT_FILE_BAD] = "bad", }; DEFINE_STRING_TABLE_LOOKUP(unit_file_state, UnitFileState); static const char* const unit_file_change_type_table[_UNIT_FILE_CHANGE_TYPE_MAX] = { - [UNIT_FILE_SYMLINK] = "symlink", - [UNIT_FILE_UNLINK] = "unlink", - [UNIT_FILE_IS_MASKED] = "masked", + [UNIT_FILE_SYMLINK] = "symlink", + [UNIT_FILE_UNLINK] = "unlink", + [UNIT_FILE_IS_MASKED] = "masked", [UNIT_FILE_IS_DANGLING] = "dangling", }; DEFINE_STRING_TABLE_LOOKUP(unit_file_change_type, UnitFileChangeType); static const char* const unit_file_preset_mode_table[_UNIT_FILE_PRESET_MAX] = { - [UNIT_FILE_PRESET_FULL] = "full", - [UNIT_FILE_PRESET_ENABLE_ONLY] = "enable-only", + [UNIT_FILE_PRESET_FULL] = "full", + [UNIT_FILE_PRESET_ENABLE_ONLY] = "enable-only", [UNIT_FILE_PRESET_DISABLE_ONLY] = "disable-only", }; From bce84e10f8e38982306b07596584287c1d27d79d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 4 May 2020 19:45:19 +0200 Subject: [PATCH 11/14] shared/install: print name of offending file in error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "Unit type slice cannot be templated." ↓ "/etc/systemd/system.control/user@.slice: unit type slice cannot be templated." --- src/shared/install.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shared/install.c b/src/shared/install.c index a692aed96e..1d9d5bf6e9 100644 --- a/src/shared/install.c +++ b/src/shared/install.c @@ -1226,7 +1226,7 @@ static int unit_file_load( return -EINVAL; if (unit_name_is_valid(info->name, UNIT_NAME_TEMPLATE|UNIT_NAME_INSTANCE) && !unit_type_may_template(type)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), - "Unit type %s cannot be templated.", unit_type_to_string(type)); + "%s: unit type %s cannot be templated, ignoring.", path, unit_type_to_string(type)); if (!(flags & SEARCH_LOAD)) { r = lstat(path, &st); From 9c894b8579d05cf027ef6ebe4ea6648cc4535e65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 4 May 2020 19:48:59 +0200 Subject: [PATCH 12/14] shared/install: rename variable for clarity --- src/shared/install.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/shared/install.c b/src/shared/install.c index 1d9d5bf6e9..e64157cc0e 100644 --- a/src/shared/install.c +++ b/src/shared/install.c @@ -3344,7 +3344,7 @@ int unit_file_get_list( char **patterns) { _cleanup_(lookup_paths_free) LookupPaths paths = {}; - char **i; + char **dirname; int r; assert(scope >= 0); @@ -3355,16 +3355,16 @@ int unit_file_get_list( if (r < 0) return r; - STRV_FOREACH(i, paths.search_path) { + STRV_FOREACH(dirname, paths.search_path) { _cleanup_closedir_ DIR *d = NULL; struct dirent *de; - d = opendir(*i); + d = opendir(*dirname); if (!d) { if (errno == ENOENT) continue; if (IN_SET(errno, ENOTDIR, EACCES)) { - log_debug_errno(errno, "Failed to open \"%s\": %m", *i); + log_debug_errno(errno, "Failed to open \"%s\": %m", *dirname); continue; } @@ -3392,7 +3392,7 @@ int unit_file_get_list( if (!f) return -ENOMEM; - f->path = path_make_absolute(de->d_name, *i); + f->path = path_make_absolute(de->d_name, *dirname); if (!f->path) return -ENOMEM; From 15d7ab87c4e5917f5788f1f8dce327a1e272bea3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 4 May 2020 19:53:33 +0200 Subject: [PATCH 13/14] systemctl: add new enablement state "alias" For units which are aliases of other units, reporting preset status as "enabled" is rather misleading. For example, dbus.service is an alias of dbus-broker.service. In list-unit-files we'd show both as "enabled". In particular, systemctl preset ignores aliases, so showing any preset status at all is always going to be misleading. Let's introduce a new state "alias" and use that for all aliases. I was trying to avoid adding a new state, to keep compatibility with previous behaviour, but for alias unit files it simply doesn't seem very useful to show any of the existing states. It seems that the clearly showing that those are aliases for other units will be easiest to understand for users. --- man/systemctl.xml | 5 ++++ src/shared/install.c | 7 ++++++ src/shared/unit-file.h | 1 + src/systemctl/systemctl.c | 5 +++- src/test/test-install-root.c | 46 ++++++++++++++++++------------------ 5 files changed, 40 insertions(+), 24 deletions(-) diff --git a/man/systemctl.xml b/man/systemctl.xml index 30880b4110..70b4ab4212 100644 --- a/man/systemctl.xml +++ b/man/systemctl.xml @@ -729,6 +729,11 @@ Jan 12 10:46:45 example.com bluetoothd[8900]: gatt-time-server: Input/output err linked-runtime + + alias + The name is an alias (symlink to another unit file). + 0 + masked Completely disabled, so that any start operation on it fails (permanently in /etc/systemd/system/ or transiently in /run/systemd/systemd/). diff --git a/src/shared/install.c b/src/shared/install.c index e64157cc0e..ec2b99ab98 100644 --- a/src/shared/install.c +++ b/src/shared/install.c @@ -2767,6 +2767,12 @@ int unit_file_lookup_state( break; case UNIT_FILE_TYPE_REGULAR: + /* Check if the name we were querying is actually an alias */ + if (!streq(name, basename(i->path)) && !unit_name_is_valid(i->name, UNIT_NAME_INSTANCE)) { + state = UNIT_FILE_ALIAS; + break; + } + r = path_is_generator(paths, i->path); if (r < 0) return r; @@ -3420,6 +3426,7 @@ static const char* const unit_file_state_table[_UNIT_FILE_STATE_MAX] = { [UNIT_FILE_ENABLED_RUNTIME] = "enabled-runtime", [UNIT_FILE_LINKED] = "linked", [UNIT_FILE_LINKED_RUNTIME] = "linked-runtime", + [UNIT_FILE_ALIAS] = "alias", [UNIT_FILE_MASKED] = "masked", [UNIT_FILE_MASKED_RUNTIME] = "masked-runtime", [UNIT_FILE_STATIC] = "static", diff --git a/src/shared/unit-file.h b/src/shared/unit-file.h index a44ba5b051..9d402e792a 100644 --- a/src/shared/unit-file.h +++ b/src/shared/unit-file.h @@ -16,6 +16,7 @@ enum UnitFileState { UNIT_FILE_ENABLED_RUNTIME, UNIT_FILE_LINKED, UNIT_FILE_LINKED_RUNTIME, + UNIT_FILE_ALIAS, UNIT_FILE_MASKED, UNIT_FILE_MASKED_RUNTIME, UNIT_FILE_STATIC, diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index ac46ac88e0..a1bbe85046 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -1471,7 +1471,9 @@ static int output_unit_file_list(const UnitFileList *units, unsigned c) { UNIT_FILE_DISABLED, UNIT_FILE_BAD)) on_unit_color = underline ? ansi_highlight_red_underline() : ansi_highlight_red(); - else if (u->state == UNIT_FILE_ENABLED) + else if (IN_SET(u->state, + UNIT_FILE_ENABLED, + UNIT_FILE_ALIAS)) on_unit_color = underline ? ansi_highlight_green_underline() : ansi_highlight_green(); else on_unit_color = on_underline; @@ -7336,6 +7338,7 @@ static int unit_is_enabled(int argc, char *argv[], void *userdata) { UNIT_FILE_ENABLED, UNIT_FILE_ENABLED_RUNTIME, UNIT_FILE_STATIC, + UNIT_FILE_ALIAS, UNIT_FILE_INDIRECT, UNIT_FILE_GENERATED)) enabled = true; diff --git a/src/test/test-install-root.c b/src/test/test-install-root.c index 79a105c5c3..515f14b8ca 100644 --- a/src/test/test-install-root.c +++ b/src/test/test-install-root.c @@ -36,20 +36,20 @@ static void test_basic_mask_and_enable(const char *root) { assert_se(symlink("a.service", p) >= 0); assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "b.service", NULL) >= 0); - assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "b.service", &state) >= 0 && state == UNIT_FILE_DISABLED); + assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "b.service", &state) >= 0 && state == UNIT_FILE_ALIAS); p = strjoina(root, "/usr/lib/systemd/system/c.service"); assert_se(symlink("/usr/lib/systemd/system/a.service", p) >= 0); assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "c.service", NULL) >= 0); - assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "c.service", &state) >= 0 && state == UNIT_FILE_DISABLED); + assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "c.service", &state) >= 0 && state == UNIT_FILE_ALIAS); p = strjoina(root, "/usr/lib/systemd/system/d.service"); assert_se(symlink("c.service", p) >= 0); /* This one is interesting, as d follows a relative, then an absolute symlink */ assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "d.service", NULL) >= 0); - assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "d.service", &state) >= 0 && state == UNIT_FILE_DISABLED); + assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "d.service", &state) >= 0 && state == UNIT_FILE_ALIAS); assert_se(unit_file_mask(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("a.service"), &changes, &n_changes) >= 0); assert_se(n_changes == 1); @@ -89,9 +89,9 @@ static void test_basic_mask_and_enable(const char *root) { changes = NULL; n_changes = 0; assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "a.service", &state) >= 0 && state == UNIT_FILE_ENABLED); - assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "b.service", &state) >= 0 && state == UNIT_FILE_ENABLED); - assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "c.service", &state) >= 0 && state == UNIT_FILE_ENABLED); - assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "d.service", &state) >= 0 && state == UNIT_FILE_ENABLED); + assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "b.service", &state) >= 0 && state == UNIT_FILE_ALIAS); + assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "c.service", &state) >= 0 && state == UNIT_FILE_ALIAS); + assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "d.service", &state) >= 0 && state == UNIT_FILE_ALIAS); /* Enabling it again should succeed but be a NOP */ assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("a.service"), &changes, &n_changes) >= 0); @@ -108,9 +108,9 @@ static void test_basic_mask_and_enable(const char *root) { changes = NULL; n_changes = 0; assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "a.service", &state) >= 0 && state == UNIT_FILE_DISABLED); - assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "b.service", &state) >= 0 && state == UNIT_FILE_DISABLED); - assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "c.service", &state) >= 0 && state == UNIT_FILE_DISABLED); - assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "d.service", &state) >= 0 && state == UNIT_FILE_DISABLED); + assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "b.service", &state) >= 0 && state == UNIT_FILE_ALIAS); + assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "c.service", &state) >= 0 && state == UNIT_FILE_ALIAS); + assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "d.service", &state) >= 0 && state == UNIT_FILE_ALIAS); /* Disabling a disabled unit must succeed but be a NOP */ assert_se(unit_file_disable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("a.service"), &changes, &n_changes) >= 0); @@ -129,9 +129,9 @@ static void test_basic_mask_and_enable(const char *root) { changes = NULL; n_changes = 0; assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "a.service", &state) >= 0 && state == UNIT_FILE_ENABLED); - assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "b.service", &state) >= 0 && state == UNIT_FILE_ENABLED); - assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "c.service", &state) >= 0 && state == UNIT_FILE_ENABLED); - assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "d.service", &state) >= 0 && state == UNIT_FILE_ENABLED); + assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "b.service", &state) >= 0 && state == UNIT_FILE_ALIAS); + assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "c.service", &state) >= 0 && state == UNIT_FILE_ALIAS); + assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "d.service", &state) >= 0 && state == UNIT_FILE_ALIAS); /* Let's try to reenable */ @@ -147,9 +147,9 @@ static void test_basic_mask_and_enable(const char *root) { changes = NULL; n_changes = 0; assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "a.service", &state) >= 0 && state == UNIT_FILE_ENABLED); - assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "b.service", &state) >= 0 && state == UNIT_FILE_ENABLED); - assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "c.service", &state) >= 0 && state == UNIT_FILE_ENABLED); - assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "d.service", &state) >= 0 && state == UNIT_FILE_ENABLED); + assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "b.service", &state) >= 0 && state == UNIT_FILE_ALIAS); + assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "c.service", &state) >= 0 && state == UNIT_FILE_ALIAS); + assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "d.service", &state) >= 0 && state == UNIT_FILE_ALIAS); } static void test_linked_units(const char *root) { @@ -386,7 +386,7 @@ static void test_template_enable(const char *root) { assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template@.service", &state) >= 0 && state == UNIT_FILE_DISABLED); assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template@def.service", &state) >= 0 && state == UNIT_FILE_DISABLED); assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template@foo.service", &state) >= 0 && state == UNIT_FILE_DISABLED); - assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template-symlink@.service", &state) >= 0 && state == UNIT_FILE_DISABLED); + assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template-symlink@.service", &state) >= 0 && state == UNIT_FILE_ALIAS); assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template-symlink@def.service", &state) >= 0 && state == UNIT_FILE_DISABLED); assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template-symlink@foo.service", &state) >= 0 && state == UNIT_FILE_DISABLED); @@ -404,7 +404,7 @@ static void test_template_enable(const char *root) { assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template@.service", &state) >= 0 && state == UNIT_FILE_ENABLED); assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template@def.service", &state) >= 0 && state == UNIT_FILE_ENABLED); assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template@foo.service", &state) >= 0 && state == UNIT_FILE_DISABLED); - assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template-symlink@.service", &state) >= 0 && state == UNIT_FILE_ENABLED); + assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template-symlink@.service", &state) >= 0 && state == UNIT_FILE_ALIAS); assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template-symlink@def.service", &state) >= 0 && state == UNIT_FILE_ENABLED); assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template-symlink@foo.service", &state) >= 0 && state == UNIT_FILE_DISABLED); @@ -418,7 +418,7 @@ static void test_template_enable(const char *root) { assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template@.service", &state) >= 0 && state == UNIT_FILE_DISABLED); assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template@def.service", &state) >= 0 && state == UNIT_FILE_DISABLED); assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template@foo.service", &state) >= 0 && state == UNIT_FILE_DISABLED); - assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template-symlink@.service", &state) >= 0 && state == UNIT_FILE_DISABLED); + assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template-symlink@.service", &state) >= 0 && state == UNIT_FILE_ALIAS); assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template-symlink@def.service", &state) >= 0 && state == UNIT_FILE_DISABLED); assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template-symlink@foo.service", &state) >= 0 && state == UNIT_FILE_DISABLED); @@ -450,7 +450,7 @@ static void test_template_enable(const char *root) { assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template@def.service", &state) >= 0 && state == UNIT_FILE_DISABLED); assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template@foo.service", &state) >= 0 && state == UNIT_FILE_DISABLED); assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template@quux.service", &state) >= 0 && state == UNIT_FILE_DISABLED); - assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template-symlink@.service", &state) >= 0 && state == UNIT_FILE_DISABLED); + assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template-symlink@.service", &state) >= 0 && state == UNIT_FILE_ALIAS); assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template-symlink@def.service", &state) >= 0 && state == UNIT_FILE_DISABLED); assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template-symlink@foo.service", &state) >= 0 && state == UNIT_FILE_DISABLED); assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template-symlink@quux.service", &state) >= 0 && state == UNIT_FILE_DISABLED); @@ -469,7 +469,7 @@ static void test_template_enable(const char *root) { assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template@def.service", &state) >= 0 && state == UNIT_FILE_DISABLED); assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template@foo.service", &state) >= 0 && state == UNIT_FILE_DISABLED); assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template@quux.service", &state) >= 0 && state == UNIT_FILE_ENABLED); - assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template-symlink@.service", &state) >= 0 && state == UNIT_FILE_INDIRECT); + assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template-symlink@.service", &state) >= 0 && state == UNIT_FILE_ALIAS); assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template-symlink@def.service", &state) >= 0 && state == UNIT_FILE_DISABLED); assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template-symlink@foo.service", &state) >= 0 && state == UNIT_FILE_DISABLED); assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "template-symlink@quux.service", &state) >= 0 && state == UNIT_FILE_ENABLED); @@ -500,7 +500,7 @@ static void test_indirect(const char *root) { assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "indirecta.service", &state) >= 0 && state == UNIT_FILE_INDIRECT); assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "indirectb.service", &state) >= 0 && state == UNIT_FILE_DISABLED); - assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "indirectc.service", &state) >= 0 && state == UNIT_FILE_INDIRECT); + assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "indirectc.service", &state) >= 0 && state == UNIT_FILE_ALIAS); assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("indirectc.service"), &changes, &n_changes) >= 0); assert_se(n_changes == 1); @@ -513,7 +513,7 @@ static void test_indirect(const char *root) { assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "indirecta.service", &state) >= 0 && state == UNIT_FILE_INDIRECT); assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "indirectb.service", &state) >= 0 && state == UNIT_FILE_ENABLED); - assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "indirectc.service", &state) >= 0 && state == UNIT_FILE_INDIRECT); + assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "indirectc.service", &state) >= 0 && state == UNIT_FILE_ALIAS); assert_se(unit_file_disable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("indirectc.service"), &changes, &n_changes) >= 0); assert_se(n_changes == 1); @@ -624,7 +624,7 @@ static void test_preset_and_list(const char *root) { got_no = true; assert_se(fl->state == UNIT_FILE_DISABLED); } else - assert_se(IN_SET(fl->state, UNIT_FILE_DISABLED, UNIT_FILE_STATIC, UNIT_FILE_INDIRECT)); + assert_se(IN_SET(fl->state, UNIT_FILE_DISABLED, UNIT_FILE_STATIC, UNIT_FILE_INDIRECT, UNIT_FILE_ALIAS)); } unit_file_list_free(h); From 31b8895af71f97bb59e93f30d35095ed55cf426d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 4 May 2020 20:01:52 +0200 Subject: [PATCH 14/14] systemctl: do not show preset state for "static"/"alias"/"generated"/"transient" units This modifies list-unit-files and status. The output of list-unit-files would contain various lines with "static disabled", which is just misleading, because systemctl preset will not touch those files, so the preset configuration is not relevant. Similarly, for "generated" and "transient" units, preset state is irrelevant. For "alias" lines, the preset for the alias target would be used. In all cases let's just skip preset status in those cases. Diff for a a Fedora rawhide container: --- $ systemctl --root=/tmp/root2 list-unit-files +++ $ build/systemctl --root=/tmp/root2 list-unit-files @@ -1,125 +1,125 @@ UNIT FILE STATE VENDOR PRESET -proc-sys-fs-binfmt_misc.automount static disabled -dev-hugepages.mount static disabled -dev-mqueue.mount static disabled -proc-fs-nfsd.mount static disabled +proc-sys-fs-binfmt_misc.automount static +dev-hugepages.mount static +dev-mqueue.mount static +proc-fs-nfsd.mount static proc-sys-fs-binfmt_misc.mount disabled disabled -sys-fs-fuse-connections.mount static disabled -sys-kernel-config.mount static disabled -sys-kernel-debug.mount static disabled -sys-kernel-tracing.mount static disabled -tmp.mount static disabled -var-lib-nfs-rpc_pipefs.mount static disabled +sys-fs-fuse-connections.mount static +sys-kernel-config.mount static +sys-kernel-debug.mount static +sys-kernel-tracing.mount static +tmp.mount static +var-lib-nfs-rpc_pipefs.mount static ostree-finalize-staged.path disabled enabled -systemd-ask-password-console.path static disabled -systemd-ask-password-plymouth.path static disabled -systemd-ask-password-wall.path static disabled +systemd-ask-password-console.path static +systemd-ask-password-plymouth.path static +systemd-ask-password-wall.path static abrt-journal-core.service enabled enabled abrt-oops.service enabled enabled abrt-pstoreoops.service disabled disabled abrt-vmcore.service enabled enabled abrt-xorg.service enabled enabled abrtd.service enabled enabled -anaconda-direct.service static disabled -anaconda-nm-config.service static disabled -anaconda-noshell.service static disabled -anaconda-pre.service static disabled -anaconda-shell@.service static disabled -anaconda-sshd.service static disabled -anaconda-tmux@.service static disabled -anaconda.service static disabled +anaconda-direct.service static +anaconda-nm-config.service static +anaconda-noshell.service static +anaconda-pre.service static +anaconda-shell@.service static +anaconda-sshd.service static +anaconda-tmux@.service static +anaconda.service static arp-ethers.service disabled disabled atd.service enabled enabled auditd.service enabled enabled -auth-rpcgss-module.service static disabled +auth-rpcgss-module.service static autofs.service disabled disabled -autovt@.service enabled disabled -blivet.service static disabled +autovt@.service alias +blivet.service static blk-availability.service disabled disabled bluetooth.service enabled enabled -btattach-bcm@.service static disabled +btattach-bcm@.service static certmonger.service disabled disabled -chrony-dnssrv@.service static disabled +chrony-dnssrv@.service static chrony-wait.service disabled disabled chronyd.service enabled enabled -clean-mount-point@.service static disabled -cockpit-motd.service static disabled -cockpit-wsinstance-http-redirect.service static disabled -cockpit-wsinstance-http.service static disabled -cockpit-wsinstance-https-factory@.service static disabled -cockpit-wsinstance-https@.service static disabled -cockpit.service static disabled +clean-mount-point@.service static +cockpit-motd.service static +cockpit-wsinstance-http-redirect.service static +cockpit-wsinstance-http.service static +cockpit-wsinstance-https-factory@.service static +cockpit-wsinstance-https@.service static +cockpit.service static console-getty.service disabled disabled -container-getty@.service static disabled +container-getty@.service static dbus-broker.service enabled enabled dbus-daemon.service disabled disabled -dbus-org.bluez.service enabled disabled -dbus-org.fedoraproject.FirewallD1.service enabled disabled -dbus-org.freedesktop.home1.service static disabled -dbus-org.freedesktop.hostname1.service static disabled -dbus-org.freedesktop.locale1.service static disabled -dbus-org.freedesktop.login1.service static disabled -dbus-org.freedesktop.ModemManager1.service enabled disabled -dbus-org.freedesktop.nm-dispatcher.service enabled disabled -dbus-org.freedesktop.portable1.service static disabled -dbus-org.freedesktop.timedate1.service static disabled -dbus.service enabled disabled +dbus-org.bluez.service alias +dbus-org.fedoraproject.FirewallD1.service alias +dbus-org.freedesktop.home1.service alias +dbus-org.freedesktop.hostname1.service alias +dbus-org.freedesktop.locale1.service alias +dbus-org.freedesktop.login1.service alias +dbus-org.freedesktop.ModemManager1.service alias +dbus-org.freedesktop.nm-dispatcher.service alias +dbus-org.freedesktop.portable1.service alias +dbus-org.freedesktop.timedate1.service alias +dbus.service alias dbxtool.service enabled enabled debug-shell.service disabled disabled -dm-event.service static enabled +dm-event.service static dmraid-activation.service enabled enabled -dnf-makecache.service static disabled +dnf-makecache.service static dnsmasq.service disabled disabled -dracut-cmdline.service static disabled -dracut-initqueue.service static disabled -dracut-mount.service static disabled -dracut-pre-mount.service static disabled -dracut-pre-pivot.service static disabled -dracut-pre-trigger.service static disabled -dracut-pre-udev.service static disabled -dracut-shutdown.service static disabled +dracut-cmdline.service static +dracut-initqueue.service static +dracut-mount.service static +dracut-pre-mount.service static +dracut-pre-pivot.service static +dracut-pre-trigger.service static +dracut-pre-udev.service static +dracut-shutdown.service static ead.service disabled disabled -emergency.service static disabled +emergency.service static fancontrol.service disabled disabled firewalld.service enabled enabled -fprintd.service static disabled -fstrim.service static disabled +fprintd.service static +fstrim.service static getty@.service enabled enabled -grub-boot-indeterminate.service static disabled +grub-boot-indeterminate.service static gssproxy.service disabled disabled import-state.service enabled enabled initial-setup-reconfiguration.service disabled disabled initial-setup.service enabled disabled -initrd-cleanup.service static disabled -initrd-parse-etc.service static disabled -initrd-switch-root.service static disabled -initrd-udevadm-cleanup-db.service static disabled -instperf.service static disabled +initrd-cleanup.service static +initrd-parse-etc.service static +initrd-switch-root.service static +initrd-udevadm-cleanup-db.service static +instperf.service static io.podman.service disabled disabled irqbalance.service enabled enabled -iscsi-shutdown.service static disabled +iscsi-shutdown.service static iscsi.service enabled disabled iscsid.service disabled disabled iscsiuio.service disabled disabled iwd.service disabled disabled kdump.service disabled disabled -kmod-static-nodes.service static disabled -ldconfig.service static disabled +kmod-static-nodes.service static +ldconfig.service static lm_sensors.service enabled enabled loadmodules.service disabled disabled -logrotate.service static disabled -lvm2-lvmpolld.service static enabled +logrotate.service static +lvm2-lvmpolld.service static lvm2-monitor.service enabled enabled -lvm2-pvscan@.service static disabled -man-db-cache-update.service static disabled +lvm2-pvscan@.service static +man-db-cache-update.service static man-db-restart-cache-update.service disabled disabled -mdadm-grow-continue@.service static disabled -mdadm-last-resort@.service static disabled -mdmon@.service static disabled +mdadm-grow-continue@.service static +mdadm-last-resort@.service static +mdmon@.service static mdmonitor.service enabled enabled -mlocate-updatedb.service static disabled +mlocate-updatedb.service static ModemManager.service enabled enabled -modprobe@.service static disabled +modprobe@.service static multipathd.service enabled enabled ndctl-monitor.service disabled disabled NetworkManager-dispatcher.service enabled enabled @@ -127,53 +127,53 @@ NetworkManager.service enabled enabled nfs-blkmap.service disabled disabled nfs-convert.service enabled disabled -nfs-idmapd.service static disabled -nfs-mountd.service static disabled +nfs-idmapd.service static +nfs-mountd.service static nfs-server.service disabled disabled -nfs-utils.service static disabled -nfsdcld.service static disabled +nfs-utils.service static +nfsdcld.service static nftables.service disabled disabled nis-domainname.service disabled disabled oddjobd.service disabled disabled openhpid.service disabled disabled -ostree-finalize-staged.service static disabled -ostree-prepare-root.service static disabled +ostree-finalize-staged.service static +ostree-prepare-root.service static ostree-remount.service enabled enabled -packagekit-offline-update.service static disabled -packagekit.service static disabled +packagekit-offline-update.service static +packagekit.service static pcscd.service indirect disabled -plymouth-halt.service static disabled -plymouth-kexec.service static disabled -plymouth-poweroff.service static disabled -plymouth-quit-wait.service static disabled -plymouth-quit.service static disabled -plymouth-read-write.service static disabled -plymouth-reboot.service static disabled -plymouth-start.service static disabled -plymouth-switch-root.service static disabled +plymouth-halt.service static +plymouth-kexec.service static +plymouth-poweroff.service static +plymouth-quit-wait.service static +plymouth-quit.service static +plymouth-read-write.service static +plymouth-reboot.service static +plymouth-start.service static +plymouth-switch-root.service static podman.service disabled disabled -polkit.service static enabled +polkit.service static psacct.service disabled disabled qemu-guest-agent.service enabled enabled -quotaon.service static disabled +quotaon.service static raid-check.service disabled disabled -rc-local.service static disabled +rc-local.service static rdisc.service disabled disabled -realmd.service static disabled -rescue.service static disabled +realmd.service static +rescue.service static rngd.service enabled enabled -rpc-gssd.service static disabled -rpc-statd-notify.service static disabled -rpc-statd.service static disabled +rpc-gssd.service static +rpc-statd-notify.service static +rpc-statd.service static rpcbind.service disabled disabled rsyslog.service enabled enabled selinux-autorelabel-mark.service enabled enabled -selinux-autorelabel.service static disabled +selinux-autorelabel.service static serial-getty@.service disabled disabled smartd.service enabled enabled sshd-keygen@.service disabled disabled sshd.service enabled enabled -sshd@.service static disabled +sshd@.service static sssd-autofs.service indirect disabled sssd-kcm.service indirect disabled sssd-nss.service indirect disabled @@ -182,84 +182,84 @@ sssd-ssh.service indirect disabled sssd-sudo.service indirect disabled sssd.service enabled enabled -system-update-cleanup.service static disabled -systemd-ask-password-console.service static disabled -systemd-ask-password-plymouth.service static disabled -systemd-ask-password-wall.service static disabled -systemd-backlight@.service static disabled -systemd-binfmt.service static disabled -systemd-bless-boot.service static disabled +system-update-cleanup.service static +systemd-ask-password-console.service static +systemd-ask-password-plymouth.service static +systemd-ask-password-wall.service static +systemd-backlight@.service static +systemd-binfmt.service static +systemd-bless-boot.service static systemd-boot-check-no-failures.service disabled disabled -systemd-boot-system-token.service static disabled -systemd-coredump@.service static disabled -systemd-exit.service static disabled -systemd-firstboot.service static disabled -systemd-fsck-root.service static disabled -systemd-fsck@.service static disabled -systemd-halt.service static disabled -systemd-hibernate-resume@.service static disabled -systemd-hibernate.service static disabled -systemd-homed.service static disabled -systemd-hostnamed.service static disabled -systemd-hwdb-update.service static disabled -systemd-hybrid-sleep.service static disabled -systemd-initctl.service static disabled -systemd-journal-catalog-update.service static disabled -systemd-journal-flush.service static disabled -systemd-journald.service static disabled -systemd-journald@.service static disabled -systemd-kexec.service static disabled -systemd-localed.service static disabled -systemd-logind.service static disabled -systemd-machine-id-commit.service static disabled -systemd-modules-load.service static disabled +systemd-boot-system-token.service static +systemd-coredump@.service static +systemd-exit.service static +systemd-firstboot.service static +systemd-fsck-root.service static +systemd-fsck@.service static +systemd-halt.service static +systemd-hibernate-resume@.service static +systemd-hibernate.service static +systemd-homed.service static +systemd-hostnamed.service static +systemd-hwdb-update.service static +systemd-hybrid-sleep.service static +systemd-initctl.service static +systemd-journal-catalog-update.service static +systemd-journal-flush.service static +systemd-journald.service static +systemd-journald@.service static +systemd-kexec.service static +systemd-localed.service static +systemd-logind.service static +systemd-machine-id-commit.service static +systemd-modules-load.service static systemd-network-generator.service disabled disabled systemd-networkd-wait-online.service disabled disabled systemd-networkd.service disabled disabled -systemd-portabled.service static disabled -systemd-poweroff.service static disabled +systemd-portabled.service static +systemd-poweroff.service static systemd-pstore.service disabled enabled -systemd-quotacheck.service static disabled -systemd-random-seed.service static disabled -systemd-reboot.service static disabled -systemd-remount-fs.service static disabled -systemd-repart.service static disabled +systemd-quotacheck.service static +systemd-random-seed.service static +systemd-reboot.service static +systemd-remount-fs.service static +systemd-repart.service static systemd-resolved.service disabled disabled -systemd-rfkill.service static disabled -systemd-suspend-then-hibernate.service static disabled -systemd-suspend.service static disabled -systemd-sysctl.service static disabled -systemd-sysusers.service static disabled +systemd-rfkill.service static +systemd-suspend-then-hibernate.service static +systemd-suspend.service static +systemd-sysctl.service static +systemd-sysusers.service static systemd-time-wait-sync.service disabled disabled -systemd-timedated.service static disabled +systemd-timedated.service static systemd-timesyncd.service disabled disabled -systemd-tmpfiles-clean.service static disabled -systemd-tmpfiles-setup-dev.service static disabled -systemd-tmpfiles-setup.service static disabled -systemd-udev-settle.service static disabled -systemd-udev-trigger.service static disabled -systemd-udevd.service static disabled -systemd-update-done.service static disabled -systemd-update-utmp-runlevel.service static disabled -systemd-update-utmp.service static disabled -systemd-user-sessions.service static disabled -systemd-userdbd.service static disabled -systemd-vconsole-setup.service static disabled -systemd-volatile-root.service static disabled -teamd@.service static disabled +systemd-tmpfiles-clean.service static +systemd-tmpfiles-setup-dev.service static +systemd-tmpfiles-setup.service static +systemd-udev-settle.service static +systemd-udev-trigger.service static +systemd-udevd.service static +systemd-update-done.service static +systemd-update-utmp-runlevel.service static +systemd-update-utmp.service static +systemd-user-sessions.service static +systemd-userdbd.service static +systemd-vconsole-setup.service static +systemd-volatile-root.service static +teamd@.service static udisks2.service enabled enabled -user-runtime-dir@.service static disabled -user@.service static disabled +user-runtime-dir@.service static +user@.service static winbind.service disabled disabled zram-swap.service enabled disabled -zram.service static disabled -system-cockpithttps.slice static disabled -system-systemd\x2dcryptsetup.slice static disabled -user.slice static disabled -cockpit-wsinstance-http-redirect.socket static disabled -cockpit-wsinstance-http.socket static disabled -cockpit-wsinstance-https-factory.socket static disabled -cockpit-wsinstance-https@.socket static disabled +zram.service static +system-cockpithttps.slice static +system-systemd\x2dcryptsetup.slice static +user.slice static +cockpit-wsinstance-http-redirect.socket static +cockpit-wsinstance-http.socket static +cockpit-wsinstance-https-factory.socket static +cockpit-wsinstance-https@.socket static cockpit.socket enabled enabled dbus.socket enabled enabled dm-event.socket enabled enabled @@ -280,95 +280,95 @@ sssd-pam.socket disabled disabled sssd-ssh.socket disabled disabled sssd-sudo.socket disabled disabled -syslog.socket static disabled -systemd-coredump.socket static disabled -systemd-initctl.socket static disabled -systemd-journald-audit.socket static disabled -systemd-journald-dev-log.socket static disabled -systemd-journald-varlink@.socket static disabled -systemd-journald.socket static disabled -systemd-journald@.socket static disabled +syslog.socket static +systemd-coredump.socket static +systemd-initctl.socket static +systemd-journald-audit.socket static +systemd-journald-dev-log.socket static +systemd-journald-varlink@.socket static +systemd-journald.socket static +systemd-journald@.socket static systemd-networkd.socket disabled disabled -systemd-rfkill.socket static disabled -systemd-udevd-control.socket static disabled -systemd-udevd-kernel.socket static disabled -systemd-userdbd.socket static disabled -anaconda.target static disabled -basic.target static disabled -blockdev@.target static disabled -bluetooth.target static enabled -boot-complete.target static disabled -cryptsetup-pre.target static disabled -cryptsetup.target static disabled -ctrl-alt-del.target enabled disabled -default.target indirect disabled -emergency.target static disabled +systemd-rfkill.socket static +systemd-udevd-control.socket static +systemd-udevd-kernel.socket static +systemd-userdbd.socket static +anaconda.target static +basic.target static +blockdev@.target static +bluetooth.target static +boot-complete.target static +cryptsetup-pre.target static +cryptsetup.target static +ctrl-alt-del.target alias +default.target alias +emergency.target static exit.target disabled disabled -final.target static disabled -getty-pre.target static disabled -getty.target static disabled -graphical.target static disabled +final.target static +getty-pre.target static +getty.target static +graphical.target static halt.target disabled disabled -hibernate.target static disabled -hybrid-sleep.target static disabled -initrd-fs.target static disabled -initrd-root-device.target static disabled -initrd-root-fs.target static disabled -initrd-switch-root.target static disabled -initrd.target static disabled +hibernate.target static +hybrid-sleep.target static +initrd-fs.target static +initrd-root-device.target static +initrd-root-fs.target static +initrd-switch-root.target static +initrd.target static kexec.target disabled disabled -local-fs-pre.target static disabled -local-fs.target static disabled +local-fs-pre.target static +local-fs.target static multi-user.target indirect disabled -network-online.target static disabled -network-pre.target static disabled -network.target static disabled +network-online.target static +network-pre.target static +network.target static nfs-client.target enabled disabled -nss-lookup.target static disabled -nss-user-lookup.target static disabled -paths.target static disabled +nss-lookup.target static +nss-user-lookup.target static +paths.target static poweroff.target disabled disabled -printer.target static disabled +printer.target static reboot.target enabled enabled remote-cryptsetup.target disabled enabled -remote-fs-pre.target static disabled +remote-fs-pre.target static remote-fs.target enabled enabled -rescue.target static disabled -rpc_pipefs.target static disabled -rpcbind.target static disabled -runlevel0.target disabled disabled -runlevel1.target static disabled -runlevel2.target indirect disabled -runlevel3.target indirect disabled -runlevel4.target indirect disabled -runlevel5.target static disabled -runlevel6.target enabled disabled -selinux-autorelabel.target static disabled -shutdown.target static disabled -sigpwr.target static disabled -sleep.target static disabled -slices.target static disabled -smartcard.target static disabled -sockets.target static disabled -sound.target static disabled -sshd-keygen.target static disabled -suspend-then-hibernate.target static disabled -suspend.target static disabled -swap.target static disabled -sysinit.target static disabled -system-update-pre.target static disabled -system-update.target static disabled -time-set.target static disabled -time-sync.target static disabled -timers.target static disabled -umount.target static disabled +rescue.target static +rpc_pipefs.target static +rpcbind.target static +runlevel0.target alias +runlevel1.target alias +runlevel2.target alias +runlevel3.target alias +runlevel4.target alias +runlevel5.target alias +runlevel6.target alias +selinux-autorelabel.target static +shutdown.target static +sigpwr.target static +sleep.target static +slices.target static +smartcard.target static +sockets.target static +sound.target static +sshd-keygen.target static +suspend-then-hibernate.target static +suspend.target static +swap.target static +sysinit.target static +system-update-pre.target static +system-update.target static +time-set.target static +time-sync.target static +timers.target static +umount.target static chrony-dnssrv@.timer disabled disabled dnf-makecache.timer enabled enabled fstrim.timer enabled enabled logrotate.timer enabled enabled -mdadm-last-resort@.timer static disabled +mdadm-last-resort@.timer static mlocate-updatedb.timer enabled enabled raid-check.timer disabled disabled -systemd-tmpfiles-clean.timer static disabled +systemd-tmpfiles-clean.timer static 371 unit files listed. In particular, this allows a meaningful comparison to be made: $ ~/src/systemd/build/systemctl --root=/tmp/root2 list-unit-files|rg 'enabled.*disabled|disabled.*enabled' ostree-finalize-staged.path disabled enabled initial-setup.service enabled disabled iscsi.service enabled disabled nfs-convert.service enabled disabled systemd-pstore.service disabled enabled zram-swap.service enabled disabled iscsid.socket enabled disabled iscsiuio.socket enabled disabled multipathd.socket enabled disabled nfs-client.target enabled disabled remote-cryptsetup.target disabled enabled (It turns out that most of the services on this list that are enabled without a corresponding preset are against rpm packaging and do direct 'systemctl enable' calls in %post, instead of going through presets as they are supposed to.) systemctl: also skip preset status in status verb --- src/systemctl/systemctl.c | 70 +++++++++++++++++++++++++-------------- 1 file changed, 46 insertions(+), 24 deletions(-) diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index a1bbe85046..6b643597b1 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -1441,6 +1441,15 @@ static bool output_show_unit_file(const UnitFileList *u, char **states, char **p return true; } +static bool show_preset_for_state(UnitFileState state) { + /* Don't show preset state in those unit file states, it'll only confuse users. */ + return !IN_SET(state, + UNIT_FILE_ALIAS, + UNIT_FILE_STATIC, + UNIT_FILE_GENERATED, + UNIT_FILE_TRANSIENT); +} + static int output_unit_file_list(const UnitFileList *units, unsigned c) { _cleanup_(table_unrefp) Table *table = NULL; _cleanup_(unit_file_presets_freep) UnitFilePresets presets = {}; @@ -1456,7 +1465,6 @@ static int output_unit_file_list(const UnitFileList *units, unsigned c) { for (const UnitFileList *u = units; u < units + c; u++) { const char *on_underline = NULL, *on_unit_color = NULL, *id; - const char *on_preset_color = NULL, *unit_preset_str; bool underline; underline = u + 1 < units + c && @@ -1480,25 +1488,35 @@ static int output_unit_file_list(const UnitFileList *units, unsigned c) { id = basename(u->path); - r = unit_file_query_preset(arg_scope, arg_root, id, &presets); - if (r < 0) { - unit_preset_str = "n/a"; - on_preset_color = underline ? on_underline : ansi_normal(); - } else if (r == 0) { - unit_preset_str = "disabled"; - on_preset_color = underline ? ansi_highlight_red_underline() : ansi_highlight_red(); - } else { - unit_preset_str = "enabled"; - on_preset_color = underline ? ansi_highlight_green_underline() : ansi_highlight_green(); - } - r = table_add_many(table, TABLE_STRING, id, TABLE_SET_COLOR, strempty(on_underline), TABLE_STRING, unit_file_state_to_string(u->state), - TABLE_SET_COLOR, strempty(on_unit_color), - TABLE_STRING, unit_preset_str, - TABLE_SET_COLOR, strempty(on_preset_color)); + TABLE_SET_COLOR, strempty(on_unit_color)); + if (r < 0) + return table_log_add_error(r); + + if (show_preset_for_state(u->state)) { + const char *unit_preset_str, *on_preset_color; + + r = unit_file_query_preset(arg_scope, arg_root, id, &presets); + if (r < 0) { + unit_preset_str = "n/a"; + on_preset_color = underline ? on_underline : ansi_normal(); + } else if (r == 0) { + unit_preset_str = "disabled"; + on_preset_color = underline ? ansi_highlight_red_underline() : ansi_highlight_red(); + } else { + unit_preset_str = "enabled"; + on_preset_color = underline ? ansi_highlight_green_underline() : ansi_highlight_green(); + } + + r = table_add_many(table, + TABLE_STRING, unit_preset_str, + TABLE_SET_COLOR, strempty(on_preset_color)); + } else + r = table_add_many(table, TABLE_EMPTY); + if (r < 0) return table_log_add_error(r); } @@ -4222,14 +4240,18 @@ static void print_status_info( if (!isempty(i->load_error)) printf(" Loaded: %s%s%s (Reason: %s)\n", on, strna(i->load_state), off, i->load_error); - else if (path && !isempty(i->unit_file_state) && !isempty(i->unit_file_preset) && - !STR_IN_SET(i->unit_file_state, "generated", "transient")) - printf(" Loaded: %s%s%s (%s; %s; vendor preset: %s)\n", - on, strna(i->load_state), off, path, i->unit_file_state, i->unit_file_preset); - else if (path && !isempty(i->unit_file_state)) - printf(" Loaded: %s%s%s (%s; %s)\n", - on, strna(i->load_state), off, path, i->unit_file_state); - else if (path) + else if (path && !isempty(i->unit_file_state)) { + bool show_preset = !isempty(i->unit_file_preset) && + show_preset_for_state(unit_file_state_from_string(i->unit_file_state)); + + printf(" Loaded: %s%s%s (%s; %s%s%s)\n", + on, strna(i->load_state), off, + path, + i->unit_file_state, + show_preset ? "; vendor preset: " : "", + show_preset ? i->unit_file_preset : ""); + + } else if (path) printf(" Loaded: %s%s%s (%s)\n", on, strna(i->load_state), off, path); else