From 2a7cf953e1f5372ccbabed02208de989d396ae51 Mon Sep 17 00:00:00 2001 From: Maanya Goenka Date: Wed, 30 Jun 2021 09:57:54 -0700 Subject: [PATCH] systemd-analyze: add --root option for 'verify' verb and allow path parsing ------------------------------------------------------------------------------- Example Run: foobar.service created below is a service unit file that has a non-existing key-value pairing (foo = bar) and is thus, syntactically invalid. maanya-goenka@debian:~/systemd (img-support)$ cat <img/usr/lib/systemd/system/foobar.service > [Unit] > foo = bar > > [Service] > ExecStart = /opt/script0.sh > EOF The failure to create foobar.service because of the recursive dependency searching and verification has been addressed in a different PR: systemd-analyze: add option to return an error value when unit verification fails #20233 maanya-goenka@debian:~/systemd (img-support)$ sudo build/systemd-analyze verify --root=img/ foobar.service /home/maanya-goenka/systemd/img/usr/lib/systemd/system/foobar.service:2: Unknown key name 'foo' in section 'Unit', ignoring. foobar.service: Failed to create foobar.service/start: Unit sysinit.target not found. --- man/systemd-analyze.xml | 4 ++-- shell-completion/bash/systemd-analyze | 2 +- shell-completion/zsh/_systemd-analyze | 1 + src/analyze/analyze-condition.c | 2 +- src/analyze/analyze-verify.c | 4 ++-- src/analyze/analyze-verify.h | 2 +- src/analyze/analyze.c | 6 +++--- src/core/main.c | 2 +- src/core/manager.c | 4 ++-- src/core/manager.h | 2 +- src/test/test-bpf-firewall.c | 2 +- src/test/test-bpf-foreign-programs.c | 2 +- src/test/test-cgroup-mask.c | 2 +- src/test/test-cgroup-unit-default.c | 2 +- src/test/test-engine.c | 2 +- src/test/test-execute.c | 2 +- src/test/test-load-fragment.c | 4 ++-- src/test/test-path.c | 2 +- src/test/test-sched-prio.c | 2 +- src/test/test-socket-bind.c | 2 +- src/test/test-watch-pid.c | 2 +- 21 files changed, 27 insertions(+), 26 deletions(-) diff --git a/man/systemd-analyze.xml b/man/systemd-analyze.xml index 4da066e05c..21e2e928cf 100644 --- a/man/systemd-analyze.xml +++ b/man/systemd-analyze.xml @@ -747,8 +747,8 @@ Service b@0.service not loaded, b.socket cannot be started. - With cat-files, show config files underneath - the specified root path PATH. + With cat-files and verify, + operate on files underneath the specified root path PATH. diff --git a/shell-completion/bash/systemd-analyze b/shell-completion/bash/systemd-analyze index 36fcf432ff..e0a9ef5d15 100644 --- a/shell-completion/bash/systemd-analyze +++ b/shell-completion/bash/systemd-analyze @@ -125,7 +125,7 @@ _systemd_analyze() { elif __contains_word "$verb" ${VERBS[VERIFY]}; then if [[ $cur = -* ]]; then - comps='--help --version --system --user --global --man=no --generators=yes' + comps='--help --version --system --user --global --man=no --generators=yes --root' else comps=$( compgen -A file -- "$cur" ) compopt -o filenames diff --git a/shell-completion/zsh/_systemd-analyze b/shell-completion/zsh/_systemd-analyze index ce8e6162e8..921b6cb27d 100644 --- a/shell-completion/zsh/_systemd-analyze +++ b/shell-completion/zsh/_systemd-analyze @@ -87,6 +87,7 @@ _arguments \ '--system[Operate on system systemd instance]' \ '--user[Operate on user systemd instance]' \ '--global[Show global user instance config]' \ + '--root=[Add support for root argument]:PATH' \ '--no-pager[Do not pipe output into a pager]' \ '--man=[Do (not) check for existence of man pages]:boolean:(1 0)' \ '--order[When generating graph for dot, show only order]' \ diff --git a/src/analyze/analyze-condition.c b/src/analyze/analyze-condition.c index 241c188ed6..09870b95ec 100644 --- a/src/analyze/analyze-condition.c +++ b/src/analyze/analyze-condition.c @@ -83,7 +83,7 @@ int verify_conditions(char **lines, UnitFileScope scope) { return log_error_errno(r, "Failed to initialize manager: %m"); log_debug("Starting manager..."); - r = manager_startup(m, NULL, NULL); + r = manager_startup(m, /* serialization= */ NULL, /* fds= */ NULL, /* root= */ NULL); if (r < 0) return r; diff --git a/src/analyze/analyze-verify.c b/src/analyze/analyze-verify.c index bb5bdf998a..4fcec2fcdc 100644 --- a/src/analyze/analyze-verify.c +++ b/src/analyze/analyze-verify.c @@ -218,7 +218,7 @@ static int verify_unit(Unit *u, bool check_man) { return r; } -int verify_units(char **filenames, UnitFileScope scope, bool check_man, bool run_generators) { +int verify_units(char **filenames, UnitFileScope scope, bool check_man, bool run_generators, const char *root) { const ManagerTestRunFlags flags = MANAGER_TEST_RUN_MINIMAL | MANAGER_TEST_RUN_ENV_GENERATORS | @@ -246,7 +246,7 @@ int verify_units(char **filenames, UnitFileScope scope, bool check_man, bool run log_debug("Starting manager..."); - r = manager_startup(m, NULL, NULL); + r = manager_startup(m, /* serialization= */ NULL, /* fds= */ NULL, root); if (r < 0) return r; diff --git a/src/analyze/analyze-verify.h b/src/analyze/analyze-verify.h index 43bfbcbc8c..b547ca6b8d 100644 --- a/src/analyze/analyze-verify.h +++ b/src/analyze/analyze-verify.h @@ -7,4 +7,4 @@ #include "path-lookup.h" int verify_executable(Unit *u, const ExecCommand *exec); -int verify_units(char **filenames, UnitFileScope scope, bool check_man, bool run_generators); +int verify_units(char **filenames, UnitFileScope scope, bool check_man, bool run_generators, const char *root); diff --git a/src/analyze/analyze.c b/src/analyze/analyze.c index 970ed34f06..f5ce2c3ad3 100644 --- a/src/analyze/analyze.c +++ b/src/analyze/analyze.c @@ -2142,7 +2142,7 @@ static int do_condition(int argc, char *argv[], void *userdata) { } static int do_verify(int argc, char *argv[], void *userdata) { - return verify_units(strv_skip(argv, 1), arg_scope, arg_man, arg_generators); + return verify_units(strv_skip(argv, 1), arg_scope, arg_man, arg_generators, arg_root); } static int do_security(int argc, char *argv[], void *userdata) { @@ -2381,9 +2381,9 @@ static int parse_argv(int argc, char *argv[]) { return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Option --user is not supported for cat-config right now."); - if (arg_root && !streq_ptr(argv[optind], "cat-config")) + if (arg_root && !STRPTR_IN_SET(argv[optind], "cat-config", "verify")) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), - "Option --root is only supported for cat-config right now."); + "Option --root is only supported for cat-config and verify right now."); return 1; /* work to do */ } diff --git a/src/core/main.c b/src/core/main.c index eb24245fb3..0914f92a2e 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -2908,7 +2908,7 @@ int main(int argc, char *argv[]) { before_startup = now(CLOCK_MONOTONIC); - r = manager_startup(m, arg_serialization, fds); + r = manager_startup(m, arg_serialization, fds, /* root= */ NULL); if (r < 0) { error_message = "Failed to start up manager"; goto finish; diff --git a/src/core/manager.c b/src/core/manager.c index 24dfe9fc06..aab230e114 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -1727,7 +1727,7 @@ void manager_reloading_stopp(Manager **m) { } } -int manager_startup(Manager *m, FILE *serialization, FDSet *fds) { +int manager_startup(Manager *m, FILE *serialization, FDSet *fds, const char *root) { int r; assert(m); @@ -1736,7 +1736,7 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds) { * but we should not touch the real generator directories. */ r = lookup_paths_init(&m->lookup_paths, m->unit_file_scope, MANAGER_IS_TEST_RUN(m) ? LOOKUP_PATHS_TEMPORARY_GENERATED : 0, - NULL); + root); if (r < 0) return log_error_errno(r, "Failed to initialize path lookup table: %m"); diff --git a/src/core/manager.h b/src/core/manager.h index 284ea42a9d..4ce4368474 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -470,7 +470,7 @@ int manager_new(UnitFileScope scope, ManagerTestRunFlags test_run_flags, Manager Manager* manager_free(Manager *m); DEFINE_TRIVIAL_CLEANUP_FUNC(Manager*, manager_free); -int manager_startup(Manager *m, FILE *serialization, FDSet *fds); +int manager_startup(Manager *m, FILE *serialization, FDSet *fds, const char *root); Job *manager_get_job(Manager *m, uint32_t id); Unit *manager_get_unit(Manager *m, const char *name); diff --git a/src/test/test-bpf-firewall.c b/src/test/test-bpf-firewall.c index b29c0d7844..8b7d46bee3 100644 --- a/src/test/test-bpf-firewall.c +++ b/src/test/test-bpf-firewall.c @@ -97,7 +97,7 @@ int main(int argc, char *argv[]) { /* The simple tests succeeded. Now let's try full unit-based use-case. */ assert_se(manager_new(UNIT_FILE_USER, MANAGER_TEST_RUN_BASIC, &m) >= 0); - assert_se(manager_startup(m, NULL, NULL) >= 0); + assert_se(manager_startup(m, NULL, NULL, NULL) >= 0); assert_se(u = unit_new(m, sizeof(Service))); assert_se(unit_add_name(u, "foo.service") == 0); diff --git a/src/test/test-bpf-foreign-programs.c b/src/test/test-bpf-foreign-programs.c index a6f8eb6f4a..bbf3916872 100644 --- a/src/test/test-bpf-foreign-programs.c +++ b/src/test/test-bpf-foreign-programs.c @@ -304,7 +304,7 @@ int main(int argc, char *argv[]) { assert_se(runtime_dir = setup_fake_runtime_dir()); assert_se(manager_new(UNIT_FILE_USER, MANAGER_TEST_RUN_BASIC, &m) >= 0); - assert_se(manager_startup(m, NULL, NULL) >= 0); + assert_se(manager_startup(m, NULL, NULL, NULL) >= 0); assert_se(test_bpf_cgroup_programs(m, "single_prog.service", single_prog, ELEMENTSOF(single_prog)) >= 0); diff --git a/src/test/test-cgroup-mask.c b/src/test/test-cgroup-mask.c index 19e159b9ff..184b393dd5 100644 --- a/src/test/test-cgroup-mask.c +++ b/src/test/test-cgroup-mask.c @@ -60,7 +60,7 @@ static int test_cgroup_mask(void) { m->default_tasks_accounting = false; m->default_tasks_max = TASKS_MAX_UNSET; - assert_se(manager_startup(m, NULL, NULL) >= 0); + assert_se(manager_startup(m, NULL, NULL, NULL) >= 0); /* Load units and verify hierarchy. */ assert_se(manager_load_startable_unit_or_warn(m, "parent.slice", NULL, &parent) >= 0); diff --git a/src/test/test-cgroup-unit-default.c b/src/test/test-cgroup-unit-default.c index 225d138e41..0fae2f64cb 100644 --- a/src/test/test-cgroup-unit-default.c +++ b/src/test/test-cgroup-unit-default.c @@ -33,7 +33,7 @@ static int test_default_memory_low(void) { } assert_se(r >= 0); - assert_se(manager_startup(m, NULL, NULL) >= 0); + assert_se(manager_startup(m, NULL, NULL, NULL) >= 0); /* dml.slice has DefaultMemoryLow=50. Beyond that, individual subhierarchies look like this: * diff --git a/src/test/test-engine.c b/src/test/test-engine.c index 6dc16193d3..880af36fb5 100644 --- a/src/test/test-engine.c +++ b/src/test/test-engine.c @@ -95,7 +95,7 @@ int main(int argc, char *argv[]) { if (manager_errno_skip_test(r)) return log_tests_skipped_errno(r, "manager_new"); assert_se(r >= 0); - assert_se(manager_startup(m, NULL, NULL) >= 0); + assert_se(manager_startup(m, NULL, NULL, NULL) >= 0); printf("Load1:\n"); assert_se(manager_load_startable_unit_or_warn(m, "a.service", NULL, &a) >= 0); diff --git a/src/test/test-execute.c b/src/test/test-execute.c index 125e0bbf4f..a0481f1194 100644 --- a/src/test/test-execute.c +++ b/src/test/test-execute.c @@ -844,7 +844,7 @@ static int run_tests(UnitFileScope scope, const test_entry tests[], char **patte if (manager_errno_skip_test(r)) return log_tests_skipped_errno(r, "manager_new"); assert_se(r >= 0); - assert_se(manager_startup(m, NULL, NULL) >= 0); + assert_se(manager_startup(m, NULL, NULL, NULL) >= 0); for (const test_entry *test = tests; test->f; test++) if (strv_fnmatch_or_empty(patterns, test->name, FNM_NOESCAPE)) diff --git a/src/test/test-load-fragment.c b/src/test/test-load-fragment.c index b41a8abf7b..e0ba99199c 100644 --- a/src/test/test-load-fragment.c +++ b/src/test/test-load-fragment.c @@ -104,7 +104,7 @@ static void test_config_parse_exec(void) { } assert_se(r >= 0); - assert_se(manager_startup(m, NULL, NULL) >= 0); + assert_se(manager_startup(m, NULL, NULL, NULL) >= 0); assert_se(u = unit_new(m, sizeof(Service))); @@ -448,7 +448,7 @@ static void test_config_parse_log_extra_fields(void) { } assert_se(r >= 0); - assert_se(manager_startup(m, NULL, NULL) >= 0); + assert_se(manager_startup(m, NULL, NULL, NULL) >= 0); assert_se(u = unit_new(m, sizeof(Service))); diff --git a/src/test/test-path.c b/src/test/test-path.c index 490fb136a7..04cb4fa37c 100644 --- a/src/test/test-path.c +++ b/src/test/test-path.c @@ -38,7 +38,7 @@ static int setup_test(Manager **m) { if (manager_errno_skip_test(r)) return log_tests_skipped_errno(r, "manager_new"); assert_se(r >= 0); - assert_se(manager_startup(tmp, NULL, NULL) >= 0); + assert_se(manager_startup(tmp, NULL, NULL, NULL) >= 0); STRV_FOREACH(test_path, tests_path) { _cleanup_free_ char *p = NULL; diff --git a/src/test/test-sched-prio.c b/src/test/test-sched-prio.c index 1f125b1d1e..35f7be491a 100644 --- a/src/test/test-sched-prio.c +++ b/src/test/test-sched-prio.c @@ -34,7 +34,7 @@ int main(int argc, char *argv[]) { if (manager_errno_skip_test(r)) return log_tests_skipped_errno(r, "manager_new"); assert_se(r >= 0); - assert_se(manager_startup(m, NULL, NULL) >= 0); + assert_se(manager_startup(m, NULL, NULL, NULL) >= 0); /* load idle ok */ assert_se(manager_load_startable_unit_or_warn(m, "sched_idle_ok.service", NULL, &idle_ok) >= 0); diff --git a/src/test/test-socket-bind.c b/src/test/test-socket-bind.c index 989172eee3..ecad86baeb 100644 --- a/src/test/test-socket-bind.c +++ b/src/test/test-socket-bind.c @@ -138,7 +138,7 @@ int main(int argc, char *argv[]) { assert_se(runtime_dir = setup_fake_runtime_dir()); assert_se(manager_new(UNIT_FILE_USER, MANAGER_TEST_RUN_BASIC, &m) >= 0); - assert_se(manager_startup(m, NULL, NULL) >= 0); + assert_se(manager_startup(m, NULL, NULL, NULL) >= 0); assert_se(test_socket_bind(m, "socket_bind_test.service", netcat_path, "2000", STRV_MAKE("2000"), STRV_MAKE("any")) >= 0); assert_se(test_socket_bind(m, "socket_bind_test.service", netcat_path, "2000", STRV_MAKE("ipv6:2001-2002"), STRV_MAKE("any")) >= 0); diff --git a/src/test/test-watch-pid.c b/src/test/test-watch-pid.c index 4afc46f10f..885ed802d4 100644 --- a/src/test/test-watch-pid.c +++ b/src/test/test-watch-pid.c @@ -27,7 +27,7 @@ int main(int argc, char *argv[]) { assert_se(runtime_dir = setup_fake_runtime_dir()); assert_se(manager_new(UNIT_FILE_USER, MANAGER_TEST_RUN_BASIC, &m) >= 0); - assert_se(manager_startup(m, NULL, NULL) >= 0); + assert_se(manager_startup(m, NULL, NULL, NULL) >= 0); assert_se(a = unit_new(m, sizeof(Service))); assert_se(unit_add_name(a, "a.service") >= 0);