From e0a3da1fd2b832929e210dab4a84c91b8b984102 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 16 Sep 2017 11:19:43 +0200 Subject: [PATCH 1/3] Make test_run into a flags field and disable generators again Now generators are only run in systemd --test mode, where this makes most sense (how are you going to test what would happen otherwise?). Fixes #6842. v2: - rename test_run to test_run_flags --- src/analyze/analyze-verify.c | 2 +- src/core/cgroup.c | 2 +- src/core/main.c | 4 +++- src/core/manager.c | 26 ++++++++++++++++---------- src/core/manager.h | 14 ++++++++++++-- src/test/test-cgroup-mask.c | 2 +- src/test/test-engine.c | 2 +- src/test/test-execute.c | 2 +- src/test/test-path.c | 2 +- src/test/test-sched-prio.c | 2 +- src/test/test-unit-file.c | 2 +- src/test/test-unit-name.c | 2 +- 12 files changed, 40 insertions(+), 22 deletions(-) diff --git a/src/analyze/analyze-verify.c b/src/analyze/analyze-verify.c index 0ce0276d92..375b457ae8 100644 --- a/src/analyze/analyze-verify.c +++ b/src/analyze/analyze-verify.c @@ -264,7 +264,7 @@ int verify_units(char **filenames, UnitFileScope scope, bool check_man) { assert_se(set_unit_path(var) >= 0); - r = manager_new(scope, true, &m); + r = manager_new(scope, MANAGER_TEST_RUN_ENV_GENERATORS, &m); if (r < 0) return log_error_errno(r, "Failed to initialize manager: %m"); diff --git a/src/core/cgroup.c b/src/core/cgroup.c index 94a4f9ae29..c806d6b7cb 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -1813,7 +1813,7 @@ int manager_setup_cgroup(Manager *m) { log_debug("Using cgroup controller " SYSTEMD_CGROUP_CONTROLLER_LEGACY ". File system hierarchy is at %s.", path); } - if (!m->test_run) { + if (!m->test_run_flags) { const char *scope_path; /* 3. Install agent */ diff --git a/src/core/main.c b/src/core/main.c index d1a53a57fd..fbf8876a2d 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -1817,7 +1817,9 @@ int main(int argc, char *argv[]) { (void) bump_rlimit_nofile(&saved_rlimit_nofile); } - r = manager_new(arg_system ? UNIT_FILE_SYSTEM : UNIT_FILE_USER, arg_action == ACTION_TEST, &m); + r = manager_new(arg_system ? UNIT_FILE_SYSTEM : UNIT_FILE_USER, + arg_action == ACTION_TEST ? MANAGER_TEST_FULL : 0, + &m); if (r < 0) { log_emergency_errno(r, "Failed to allocate manager object: %m"); error_message = "Failed to allocate manager object"; diff --git a/src/core/manager.c b/src/core/manager.c index 5bfb74be9d..46036aa50c 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -358,7 +358,7 @@ static int manager_setup_time_change(Manager *m) { assert(m); assert_cc(sizeof(time_t) == sizeof(TIME_T_MAX)); - if (m->test_run) + if (m->test_run_flags) return 0; /* Uses TFD_TIMER_CANCEL_ON_SET to get notifications whenever @@ -390,7 +390,7 @@ static int enable_special_signals(Manager *m) { assert(m); - if (m->test_run) + if (m->test_run_flags) return 0; /* Enable that we get SIGINT on control-alt-del. In containers @@ -600,7 +600,7 @@ static int manager_setup_prefix(Manager *m) { return 0; } -int manager_new(UnitFileScope scope, bool test_run, Manager **_m) { +int manager_new(UnitFileScope scope, unsigned test_run_flags, Manager **_m) { Manager *m; int r; @@ -650,7 +650,7 @@ int manager_new(UnitFileScope scope, bool test_run, Manager **_m) { m->have_ask_password = -EINVAL; /* we don't know */ m->first_boot = -1; - m->test_run = test_run; + m->test_run_flags = test_run_flags; /* Reboot immediately if the user hits C-A-D more often than 7x per 2s */ RATELIMIT_INIT(m->ctrl_alt_del_ratelimit, 2 * USEC_PER_SEC, 7); @@ -731,7 +731,7 @@ fail: static int manager_setup_notify(Manager *m) { int r; - if (m->test_run) + if (m->test_run_flags) return 0; if (m->notify_fd < 0) { @@ -811,7 +811,7 @@ static int manager_setup_cgroups_agent(Manager *m) { * to it. The system instance hence listens on this special socket, but the user instances listen on the system * bus for these messages. */ - if (m->test_run) + if (m->test_run_flags) return 0; if (!MANAGER_IS_SYSTEM(m)) @@ -925,7 +925,7 @@ static int manager_connect_bus(Manager *m, bool reexecuting) { assert(m); - if (m->test_run) + if (m->test_run_flags) return 0; u = manager_get_unit(m, SPECIAL_DBUS_SERVICE); @@ -1313,7 +1313,7 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds) { /* If we are running in test mode, we still want to run the generators, * but we should not touch the real generator directories. */ r = lookup_paths_init(&m->lookup_paths, m->unit_file_scope, - m->test_run ? LOOKUP_PATHS_TEMPORARY_GENERATED : 0, + m->test_run_flags ? LOOKUP_PATHS_TEMPORARY_GENERATED : 0, NULL); if (r < 0) return r; @@ -1336,7 +1336,7 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds) { if (m->first_boot > 0 && m->unit_file_scope == UNIT_FILE_SYSTEM && - !m->test_run) { + !m->test_run_flags) { q = unit_file_preset_all(UNIT_FILE_SYSTEM, 0, NULL, UNIT_FILE_PRESET_ENABLE_ONLY, NULL, 0); if (q < 0) @@ -3003,7 +3003,7 @@ static void manager_notify_finished(Manager *m) { char userspace[FORMAT_TIMESPAN_MAX], initrd[FORMAT_TIMESPAN_MAX], kernel[FORMAT_TIMESPAN_MAX], sum[FORMAT_TIMESPAN_MAX]; usec_t firmware_usec, loader_usec, kernel_usec, initrd_usec, userspace_usec, total_usec; - if (m->test_run) + if (m->test_run_flags) return; if (MANAGER_IS_SYSTEM(m) && detect_container() <= 0) { @@ -3148,6 +3148,9 @@ static int manager_run_environment_generators(Manager *m) { const char **paths; void* args[] = {&tmp, &tmp, &m->environment}; + if (m->test_run_flags && !(m->test_run_flags & MANAGER_TEST_RUN_ENV_GENERATORS)) + return 0; + paths = MANAGER_IS_SYSTEM(m) ? system_env_generator_binary_paths : user_env_generator_binary_paths; if (!generator_path_any(paths)) @@ -3163,6 +3166,9 @@ static int manager_run_generators(Manager *m) { assert(m); + if (m->test_run_flags && !(m->test_run_flags & MANAGER_TEST_RUN_GENERATORS)) + return 0; + paths = generator_binary_paths(m->unit_file_scope); if (!paths) return log_oom(); diff --git a/src/core/manager.h b/src/core/manager.h index 8d621da4e1..713d2db70c 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -74,6 +74,15 @@ typedef enum StatusType { #include "show-status.h" #include "unit-name.h" +enum { + /* 0 = run normally */ + MANAGER_TEST_RUN_MINIMAL = 1, /* run test w/o generators */ + MANAGER_TEST_RUN_ENV_GENERATORS = 2, /* also run env generators */ + MANAGER_TEST_RUN_GENERATORS = 4, /* also run unit generators */ + MANAGER_TEST_FULL = MANAGER_TEST_RUN_ENV_GENERATORS | MANAGER_TEST_RUN_GENERATORS, +}; +assert_cc((MANAGER_TEST_FULL & UINT8_MAX) == MANAGER_TEST_FULL); + struct Manager { /* Note that the set of units we know of is allowed to be * inconsistent. However the subset of it that is loaded may @@ -238,7 +247,8 @@ struct Manager { bool dispatching_dbus_queue:1; bool taint_usr:1; - bool test_run:1; + + unsigned test_run_flags:8; /* If non-zero, exit with the following value when the systemd * process terminate. Useful for containers: systemd-nspawn could get @@ -326,7 +336,7 @@ struct Manager { #define MANAGER_IS_RELOADING(m) ((m)->n_reloading > 0) -int manager_new(UnitFileScope scope, bool test_run, Manager **m); +int manager_new(UnitFileScope scope, unsigned test_run_flags, Manager **m); Manager* manager_free(Manager *m); void manager_enumerate(Manager *m); diff --git a/src/test/test-cgroup-mask.c b/src/test/test-cgroup-mask.c index 4a13d710dc..15d76bddda 100644 --- a/src/test/test-cgroup-mask.c +++ b/src/test/test-cgroup-mask.c @@ -39,7 +39,7 @@ static int test_cgroup_mask(void) { /* Prepare the manager. */ assert_se(set_unit_path(get_testdata_dir("")) >= 0); assert_se(runtime_dir = setup_fake_runtime_dir()); - r = manager_new(UNIT_FILE_USER, true, &m); + r = manager_new(UNIT_FILE_USER, MANAGER_TEST_RUN_MINIMAL, &m); if (r == -EPERM || r == -EACCES) { puts("manager_new: Permission denied. Skipping test."); return EXIT_TEST_SKIP; diff --git a/src/test/test-engine.c b/src/test/test-engine.c index b5d3da0d38..6916f838d4 100644 --- a/src/test/test-engine.c +++ b/src/test/test-engine.c @@ -42,7 +42,7 @@ int main(int argc, char *argv[]) { /* prepare the test */ assert_se(set_unit_path(get_testdata_dir("")) >= 0); assert_se(runtime_dir = setup_fake_runtime_dir()); - r = manager_new(UNIT_FILE_USER, true, &m); + r = manager_new(UNIT_FILE_USER, MANAGER_TEST_RUN_MINIMAL, &m); if (MANAGER_SKIP_TEST(r)) { log_notice_errno(r, "Skipping test: manager_new: %m"); return EXIT_TEST_SKIP; diff --git a/src/test/test-execute.c b/src/test/test-execute.c index 9d1dcfc58e..a38efc7cba 100644 --- a/src/test/test-execute.c +++ b/src/test/test-execute.c @@ -464,7 +464,7 @@ static int run_tests(UnitFileScope scope, const test_function_t *tests) { assert_se(tests); - r = manager_new(scope, true, &m); + r = manager_new(scope, MANAGER_TEST_RUN_MINIMAL, &m); if (MANAGER_SKIP_TEST(r)) { log_notice_errno(r, "Skipping test: manager_new: %m"); return EXIT_TEST_SKIP; diff --git a/src/test/test-path.c b/src/test/test-path.c index 6fc3f79d80..c1915017df 100644 --- a/src/test/test-path.c +++ b/src/test/test-path.c @@ -47,7 +47,7 @@ static int setup_test(Manager **m) { enter_cgroup_subroot(); - r = manager_new(UNIT_FILE_USER, true, &tmp); + r = manager_new(UNIT_FILE_USER, MANAGER_TEST_RUN_MINIMAL, &tmp); if (MANAGER_SKIP_TEST(r)) { log_notice_errno(r, "Skipping test: manager_new: %m"); return -EXIT_TEST_SKIP; diff --git a/src/test/test-sched-prio.c b/src/test/test-sched-prio.c index 062a02baf5..9bed4b3832 100644 --- a/src/test/test-sched-prio.c +++ b/src/test/test-sched-prio.c @@ -39,7 +39,7 @@ int main(int argc, char *argv[]) { /* prepare the test */ assert_se(set_unit_path(get_testdata_dir("")) >= 0); assert_se(runtime_dir = setup_fake_runtime_dir()); - r = manager_new(UNIT_FILE_USER, true, &m); + r = manager_new(UNIT_FILE_USER, MANAGER_TEST_RUN_MINIMAL, &m); if (MANAGER_SKIP_TEST(r)) { log_notice_errno(r, "Skipping test: manager_new: %m"); return EXIT_TEST_SKIP; diff --git a/src/test/test-unit-file.c b/src/test/test-unit-file.c index df2d9593e8..578e09447f 100644 --- a/src/test/test-unit-file.c +++ b/src/test/test-unit-file.c @@ -115,7 +115,7 @@ static void test_config_parse_exec(void) { Manager *m = NULL; Unit *u = NULL; - r = manager_new(UNIT_FILE_USER, true, &m); + r = manager_new(UNIT_FILE_USER, MANAGER_TEST_RUN_MINIMAL, &m); if (MANAGER_SKIP_TEST(r)) { log_notice_errno(r, "Skipping test: manager_new: %m"); return; diff --git a/src/test/test-unit-name.c b/src/test/test-unit-name.c index dcccc9080c..1992357e1b 100644 --- a/src/test/test-unit-name.c +++ b/src/test/test-unit-name.c @@ -211,7 +211,7 @@ static int test_unit_printf(void) { assert_se(get_home_dir(&home) >= 0); assert_se(get_shell(&shell) >= 0); - r = manager_new(UNIT_FILE_USER, true, &m); + r = manager_new(UNIT_FILE_USER, MANAGER_TEST_RUN_MINIMAL, &m); if (MANAGER_SKIP_TEST(r)) { log_notice_errno(r, "Skipping test: manager_new: %m"); return EXIT_TEST_SKIP; From 641c0fd14e47b800fb64291af7d96172cbaf3d49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 16 Sep 2017 11:29:34 +0200 Subject: [PATCH 2/3] analyze-verify: add --generators switch to enable generators again --- man/systemd-analyze.xml | 12 +++++++++++- src/analyze/analyze-verify.c | 6 ++++-- src/analyze/analyze-verify.h | 6 +++++- src/analyze/analyze.c | 21 ++++++++++++++++++++- 4 files changed, 40 insertions(+), 5 deletions(-) diff --git a/man/systemd-analyze.xml b/man/systemd-analyze.xml index a3a3ce0e54..e74739498c 100644 --- a/man/systemd-analyze.xml +++ b/man/systemd-analyze.xml @@ -297,13 +297,23 @@ - + Do not invoke man to verify the existence of man pages listed in Documentation=. + + + + Invoke unit generators, see + systemd.generator7. + Some generators require root privileges. When run under a + normal users, enabling generators will generally result in + some warnings. + + diff --git a/src/analyze/analyze-verify.c b/src/analyze/analyze-verify.c index 375b457ae8..f34c24c53a 100644 --- a/src/analyze/analyze-verify.c +++ b/src/analyze/analyze-verify.c @@ -242,7 +242,7 @@ static int verify_unit(Unit *u, bool check_man) { return r; } -int verify_units(char **filenames, UnitFileScope scope, bool check_man) { +int verify_units(char **filenames, UnitFileScope scope, bool check_man, bool run_generators) { _cleanup_(sd_bus_error_free) sd_bus_error err = SD_BUS_ERROR_NULL; _cleanup_free_ char *var = NULL; Manager *m = NULL; @@ -253,6 +253,8 @@ int verify_units(char **filenames, UnitFileScope scope, bool check_man) { Unit *units[strv_length(filenames)]; int i, count = 0; + const uint8_t flags = MANAGER_TEST_RUN_ENV_GENERATORS | + run_generators * MANAGER_TEST_RUN_GENERATORS; if (strv_isempty(filenames)) return 0; @@ -264,7 +266,7 @@ int verify_units(char **filenames, UnitFileScope scope, bool check_man) { assert_se(set_unit_path(var) >= 0); - r = manager_new(scope, MANAGER_TEST_RUN_ENV_GENERATORS, &m); + r = manager_new(scope, flags, &m); if (r < 0) return log_error_errno(r, "Failed to initialize manager: %m"); diff --git a/src/analyze/analyze-verify.h b/src/analyze/analyze-verify.h index d8204dc69c..d5466ecdf9 100644 --- a/src/analyze/analyze-verify.h +++ b/src/analyze/analyze-verify.h @@ -23,4 +23,8 @@ #include "path-lookup.h" -int verify_units(char **filenames, UnitFileScope scope, bool check_man); +int verify_units( + char **filenames, + UnitFileScope scope, + bool check_man, + bool run_generators); diff --git a/src/analyze/analyze.c b/src/analyze/analyze.c index a6a36a3d2e..0e0fb08922 100644 --- a/src/analyze/analyze.c +++ b/src/analyze/analyze.c @@ -79,6 +79,7 @@ static BusTransport arg_transport = BUS_TRANSPORT_LOCAL; static char *arg_host = NULL; static bool arg_user = false; static bool arg_man = true; +static bool arg_generators = false; struct boot_times { usec_t firmware_time; @@ -1413,6 +1414,7 @@ static void help(void) { " --fuzz=SECONDS Also print also services which finished SECONDS\n" " earlier than the latest in the branch\n" " --man[=BOOL] Do [not] check for existence of man pages\n\n" + " --generators[=BOOL] Do [not] run unit generators (requires privileges)\n\n" "Commands:\n" " time Print time spent in the kernel\n" " blame Print list of running units ordered by time to init\n" @@ -1445,6 +1447,7 @@ static int parse_argv(int argc, char *argv[]) { ARG_FUZZ, ARG_NO_PAGER, ARG_MAN, + ARG_GENERATORS, }; static const struct option options[] = { @@ -1459,6 +1462,7 @@ static int parse_argv(int argc, char *argv[]) { { "fuzz", required_argument, NULL, ARG_FUZZ }, { "no-pager", no_argument, NULL, ARG_NO_PAGER }, { "man", optional_argument, NULL, ARG_MAN }, + { "generators", optional_argument, NULL, ARG_GENERATORS }, { "host", required_argument, NULL, 'H' }, { "machine", required_argument, NULL, 'M' }, {} @@ -1541,6 +1545,20 @@ static int parse_argv(int argc, char *argv[]) { break; + case ARG_GENERATORS: + if (optarg) { + r = parse_boolean(optarg); + if (r < 0) { + log_error("Failed to parse --generators= argument."); + return -EINVAL; + } + + arg_generators = !!r; + } else + arg_generators = true; + + break; + case '?': return -EINVAL; @@ -1566,7 +1584,8 @@ int main(int argc, char *argv[]) { if (streq_ptr(argv[optind], "verify")) r = verify_units(argv+optind+1, arg_user ? UNIT_FILE_USER : UNIT_FILE_SYSTEM, - arg_man); + arg_man, + arg_generators); else { _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; From 23115a607e27e87c0778442ae2c2d79a8bbac0ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 16 Sep 2017 11:31:16 +0200 Subject: [PATCH 3/3] path-lookup: fix minor memleak Introduced in a1f31f4715. --- src/shared/path-lookup.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/shared/path-lookup.c b/src/shared/path-lookup.c index 0b067ddf01..a289511be5 100644 --- a/src/shared/path-lookup.c +++ b/src/shared/path-lookup.c @@ -701,6 +701,7 @@ void lookup_paths_free(LookupPaths *p) { p->runtime_control = mfree(p->runtime_control); p->root_dir = mfree(p->root_dir); + p->temporary_dir = mfree(p->temporary_dir); } int lookup_paths_reduce(LookupPaths *p) {