From 89711996b3f561522508306e0b5ecf34f6016638 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Sun, 22 Jan 2017 12:35:08 -0500 Subject: basic/util: move execute_directory() to separate file It's a fairly specialized function. Let's make new files for it and the tests. --- src/core/manager.c | 1 + src/core/shutdown.c | 1 + 2 files changed, 2 insertions(+) (limited to 'src/core') diff --git a/src/core/manager.c b/src/core/manager.c index e4da945777..0884534cc4 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -52,6 +52,7 @@ #include "dirent-util.h" #include "env-util.h" #include "escape.h" +#include "exec-util.h" #include "exit-status.h" #include "fd-util.h" #include "fileio.h" diff --git a/src/core/shutdown.c b/src/core/shutdown.c index a795d875bb..56a035e234 100644 --- a/src/core/shutdown.c +++ b/src/core/shutdown.c @@ -32,6 +32,7 @@ #include "alloc-util.h" #include "cgroup-util.h" #include "def.h" +#include "exec-util.h" #include "fileio.h" #include "killall.h" #include "log.h" -- cgit v1.2.3-54-g00ecf From 58f88d929f2b46e9470eb468f4890c1d4e8f51b7 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Sun, 22 Jan 2017 01:35:33 -0500 Subject: manager: fix handling of failure in initialization We would warn and continue after failure in manager_startup, but there's no way we can continue. We must fail. --- src/core/main.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'src/core') diff --git a/src/core/main.c b/src/core/main.c index ad2ce1330e..3c6b18229c 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -1830,8 +1830,10 @@ int main(int argc, char *argv[]) { before_startup = now(CLOCK_MONOTONIC); r = manager_startup(m, arg_serialization, fds); - if (r < 0) + if (r < 0) { log_error_errno(r, "Failed to fully start up daemon: %m"); + goto finish; + } /* This will close all file descriptors that were opened, but * not claimed by any unit. */ -- cgit v1.2.3-54-g00ecf From 504afd7c34e00eb84589e94e59cd14f2fffa2807 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Sat, 11 Feb 2017 18:33:16 -0500 Subject: core/manager: split out creation of serialization fd out to a helper There is a slight change in behaviour: the user manager for root will create a temporary file in /run/systemd, not /tmp. I don't think this matters, but simplifies implementation. --- src/basic/fileio.c | 19 +++++++++++++++++++ src/basic/fileio.h | 1 + src/core/manager.c | 17 ++++------------- src/test/test-fd-util.c | 10 ++++++++++ 4 files changed, 34 insertions(+), 13 deletions(-) (limited to 'src/core') diff --git a/src/basic/fileio.c b/src/basic/fileio.c index c43b0583a4..ac65fada35 100644 --- a/src/basic/fileio.c +++ b/src/basic/fileio.c @@ -1342,6 +1342,25 @@ int open_tmpfile_linkable(const char *target, int flags, char **ret_path) { return fd; } +int open_serialization_fd(const char *ident) { + int fd = -1; + + fd = memfd_create(ident, MFD_CLOEXEC); + if (fd < 0) { + const char *path; + + path = getpid() == 1 ? "/run/systemd" : "/tmp"; + fd = open_tmpfile_unlinkable(path, O_RDWR|O_CLOEXEC); + if (fd < 0) + return fd; + + log_debug("Serializing %s to %s.", ident, path); + } else + log_debug("Serializing %s to memfd.", ident); + + return fd; +} + int link_tmpfile(int fd, const char *path, const char *target) { assert(fd >= 0); diff --git a/src/basic/fileio.h b/src/basic/fileio.h index 17b38a5d60..64852b15a8 100644 --- a/src/basic/fileio.h +++ b/src/basic/fileio.h @@ -84,6 +84,7 @@ int fputs_with_space(FILE *f, const char *s, const char *separator, bool *space) int open_tmpfile_unlinkable(const char *directory, int flags); int open_tmpfile_linkable(const char *target, int flags, char **ret_path); +int open_serialization_fd(const char *ident); int link_tmpfile(int fd, const char *path, const char *target); diff --git a/src/core/manager.c b/src/core/manager.c index 0884534cc4..d432512a59 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -2437,22 +2437,14 @@ void manager_send_unit_plymouth(Manager *m, Unit *u) { } int manager_open_serialization(Manager *m, FILE **_f) { - int fd = -1; + int fd; FILE *f; assert(_f); - fd = memfd_create("systemd-serialization", MFD_CLOEXEC); - if (fd < 0) { - const char *path; - - path = MANAGER_IS_SYSTEM(m) ? "/run/systemd" : "/tmp"; - fd = open_tmpfile_unlinkable(path, O_RDWR|O_CLOEXEC); - if (fd < 0) - return -errno; - log_debug("Serializing state to %s.", path); - } else - log_debug("Serializing state to memfd."); + fd = open_serialization_fd("systemd-state"); + if (fd < 0) + return fd; f = fdopen(fd, "w+"); if (!f) { @@ -2461,7 +2453,6 @@ int manager_open_serialization(Manager *m, FILE **_f) { } *_f = f; - return 0; } diff --git a/src/test/test-fd-util.c b/src/test/test-fd-util.c index f555bb976c..4425b5fe5f 100644 --- a/src/test/test-fd-util.c +++ b/src/test/test-fd-util.c @@ -94,10 +94,20 @@ static void test_same_fd(void) { assert_se(same_fd(b, a) == 0); } +static void test_open_serialization_fd(void) { + _cleanup_close_ int fd = -1; + + fd = open_serialization_fd("test"); + assert_se(fd >= 0); + + write(fd, "test\n", 5); +} + int main(int argc, char *argv[]) { test_close_many(); test_close_nointr(); test_same_fd(); + test_open_serialization_fd(); return 0; } -- cgit v1.2.3-54-g00ecf From c6e47247a745f9245eb3dbfb29fc066ef72d3886 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Sun, 22 Jan 2017 15:22:37 -0500 Subject: basic/exec-util: add support for synchronous (ordered) execution The output of processes can be gathered, and passed back to the callee. (This commit just implements the basic functionality and tests.) After the preparation in previous commits, the change in functionality is relatively simple. For coding convenience, alarm is prepared *before* any children are executed, and not before. This shouldn't matter usually, since just forking of the children should be pretty quick. One could also argue that this is more correct, because we will also catch the case when (for whatever reason), forking itself is slow. Three callback functions and three levels of serialization are used: - from individual generator processes to the generator forker - from the forker back to the main process - deserialization in the main process v2: - replace an structure with an indexed array of callbacks --- src/basic/exec-util.c | 149 ++++++++++++++++++++++++------- src/basic/exec-util.h | 18 +++- src/core/manager.c | 3 +- src/core/shutdown.c | 2 +- src/sleep/sleep.c | 4 +- src/test/test-exec-util.c | 220 +++++++++++++++++++++++++++++++++++++++++++--- 6 files changed, 349 insertions(+), 47 deletions(-) (limited to 'src/core') diff --git a/src/basic/exec-util.c b/src/basic/exec-util.c index 46eafc7b90..d69039c489 100644 --- a/src/basic/exec-util.c +++ b/src/basic/exec-util.c @@ -22,10 +22,14 @@ #include #include #include +#include #include "alloc-util.h" #include "conf-files.h" +#include "env-util.h" #include "exec-util.h" +#include "fd-util.h" +#include "fileio.h" #include "hashmap.h" #include "macro.h" #include "process-util.h" @@ -34,12 +38,14 @@ #include "stat-util.h" #include "string-util.h" #include "strv.h" +#include "terminal-util.h" #include "util.h" /* Put this test here for a lack of better place */ assert_cc(EAGAIN == EWOULDBLOCK); -static int do_spawn(const char *path, char *argv[], pid_t *pid) { +static int do_spawn(const char *path, char *argv[], int stdout_fd, pid_t *pid) { + pid_t _pid; if (null_or_empty_path(path)) { @@ -55,6 +61,15 @@ static int do_spawn(const char *path, char *argv[], pid_t *pid) { assert_se(prctl(PR_SET_PDEATHSIG, SIGTERM) == 0); + if (stdout_fd >= 0) { + /* If the fd happens to be in the right place, go along with that */ + if (stdout_fd != STDOUT_FILENO && + dup2(stdout_fd, STDOUT_FILENO) < 0) + return -errno; + + fd_cloexec(STDOUT_FILENO, false); + } + if (!argv) { _argv[0] = (char*) path; _argv[1] = NULL; @@ -72,14 +87,24 @@ static int do_spawn(const char *path, char *argv[], pid_t *pid) { return 1; } -static int do_execute(char **directories, usec_t timeout, char *argv[]) { +static int do_execute( + char **directories, + usec_t timeout, + gather_stdout_callback_t const callbacks[_STDOUT_CONSUME_MAX], + void* const callback_args[_STDOUT_CONSUME_MAX], + int output_fd, + char *argv[]) { + _cleanup_hashmap_free_free_ Hashmap *pids = NULL; _cleanup_strv_free_ char **paths = NULL; char **path; int r; - /* We fork this all off from a child process so that we can - * somewhat cleanly make use of SIGALRM to set a time limit */ + /* We fork this all off from a child process so that we can somewhat cleanly make + * use of SIGALRM to set a time limit. + * + * If callbacks is nonnull, execution is serial. Otherwise, we default to parallel. + */ (void) reset_all_signal_handlers(); (void) reset_signal_mask(); @@ -90,35 +115,62 @@ static int do_execute(char **directories, usec_t timeout, char *argv[]) { if (r < 0) return r; - pids = hashmap_new(NULL); - if (!pids) - return log_oom(); + if (!callbacks) { + pids = hashmap_new(NULL); + if (!pids) + return log_oom(); + } + + /* Abort execution of this process after the timout. We simply rely on SIGALRM as + * default action terminating the process, and turn on alarm(). */ + + if (timeout != USEC_INFINITY) + alarm((timeout + USEC_PER_SEC - 1) / USEC_PER_SEC); STRV_FOREACH(path, paths) { _cleanup_free_ char *t = NULL; + _cleanup_close_ int fd = -1; pid_t pid; t = strdup(*path); if (!t) return log_oom(); - r = do_spawn(t, argv, &pid); + if (callbacks) { + fd = open_serialization_fd(basename(*path)); + if (fd < 0) + return log_error_errno(fd, "Failed to open serialization file: %m"); + } + + r = do_spawn(t, argv, fd, &pid); if (r <= 0) continue; - r = hashmap_put(pids, PID_TO_PTR(pid), t); - if (r < 0) - return log_oom(); - - t = NULL; + if (pids) { + r = hashmap_put(pids, PID_TO_PTR(pid), t); + if (r < 0) + return log_oom(); + t = NULL; + } else { + r = wait_for_terminate_and_warn(t, pid, true); + if (r < 0) + continue; + + if (lseek(fd, 0, SEEK_SET) < 0) + return log_error_errno(errno, "Failed to seek on serialization fd: %m"); + + r = callbacks[STDOUT_GENERATE](fd, callback_args[STDOUT_GENERATE]); + fd = -1; + if (r < 0) + return log_error_errno(r, "Failed to process output from %s: %m", *path); + } } - /* Abort execution of this process after the timout. We simply - * rely on SIGALRM as default action terminating the process, - * and turn on alarm(). */ - - if (timeout != USEC_INFINITY) - alarm((timeout + USEC_PER_SEC - 1) / USEC_PER_SEC); + if (callbacks) { + r = callbacks[STDOUT_COLLECT](output_fd, callback_args[STDOUT_COLLECT]); + if (r < 0) + return log_error_errno(r, "Callback two failed: %m"); + } while (!hashmap_isempty(pids)) { _cleanup_free_ char *t = NULL; @@ -136,31 +188,66 @@ static int do_execute(char **directories, usec_t timeout, char *argv[]) { return 0; } -void execute_directories(const char* const* directories, usec_t timeout, char *argv[]) { +int execute_directories( + const char* const* directories, + usec_t timeout, + gather_stdout_callback_t const callbacks[_STDOUT_CONSUME_MAX], + void* const callback_args[_STDOUT_CONSUME_MAX], + char *argv[]) { + pid_t executor_pid; - int r; char *name; char **dirs = (char**) directories; + _cleanup_close_ int fd = -1; + int r; assert(!strv_isempty(dirs)); name = basename(dirs[0]); assert(!isempty(name)); - /* Executes all binaries in the directories in parallel and waits - * for them to finish. Optionally a timeout is applied. If a file - * with the same name exists in more than one directory, the - * earliest one wins. */ + if (callbacks) { + assert(callback_args); + assert(callbacks[STDOUT_GENERATE]); + assert(callbacks[STDOUT_COLLECT]); + assert(callbacks[STDOUT_CONSUME]); + + fd = open_serialization_fd(name); + if (fd < 0) + return log_error_errno(fd, "Failed to open serialization file: %m"); + } + + /* Executes all binaries in the directories serially or in parallel and waits for + * them to finish. Optionally a timeout is applied. If a file with the same name + * exists in more than one directory, the earliest one wins. */ executor_pid = fork(); - if (executor_pid < 0) { - log_error_errno(errno, "Failed to fork: %m"); - return; + if (executor_pid < 0) + return log_error_errno(errno, "Failed to fork: %m"); - } else if (executor_pid == 0) { - r = do_execute(dirs, timeout, argv); + if (executor_pid == 0) { + r = do_execute(dirs, timeout, callbacks, callback_args, fd, argv); _exit(r < 0 ? EXIT_FAILURE : EXIT_SUCCESS); } - wait_for_terminate_and_warn(name, executor_pid, true); + r = wait_for_terminate_and_warn(name, executor_pid, true); + if (r < 0) + return log_error_errno(r, "Execution failed: %m"); + if (r > 0) { + /* non-zero return code from child */ + log_error("Forker process failed."); + return -EREMOTEIO; + } + + if (!callbacks) + return 0; + + if (lseek(fd, 0, SEEK_SET) < 0) + return log_error_errno(errno, "Failed to rewind serialization fd: %m"); + + r = callbacks[STDOUT_CONSUME](fd, callback_args[STDOUT_CONSUME]); + fd = -1; + if (r < 0) + return log_error_errno(r, "Failed to parse returned data: %m"); + return 0; } diff --git a/src/basic/exec-util.h b/src/basic/exec-util.h index 9f8daa9fc8..2c58e4bd5c 100644 --- a/src/basic/exec-util.h +++ b/src/basic/exec-util.h @@ -17,6 +17,22 @@ along with systemd; If not, see . ***/ +#include + #include "time-util.h" -void execute_directories(const char* const* directories, usec_t timeout, char *argv[]); +typedef int (*gather_stdout_callback_t) (int fd, void *arg); + +enum { + STDOUT_GENERATE, /* from generators to helper process */ + STDOUT_COLLECT, /* from helper process to main process */ + STDOUT_CONSUME, /* process data in main process */ + _STDOUT_CONSUME_MAX, +}; + +int execute_directories( + const char* const* directories, + usec_t timeout, + gather_stdout_callback_t const callbacks[_STDOUT_CONSUME_MAX], + void* const callback_args[_STDOUT_CONSUME_MAX], + char *argv[]); diff --git a/src/core/manager.c b/src/core/manager.c index d432512a59..73ac7499bd 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -3047,7 +3047,8 @@ static int manager_run_generators(Manager *m) { argv[4] = NULL; RUN_WITH_UMASK(0022) - execute_directories((const char* const*) paths, DEFAULT_TIMEOUT_USEC, (char**) argv); + execute_directories((const char* const*) paths, DEFAULT_TIMEOUT_USEC, + NULL, NULL, (char**) argv); finish: lookup_paths_trim_generator(&m->lookup_paths); diff --git a/src/core/shutdown.c b/src/core/shutdown.c index 56a035e234..a2309b7726 100644 --- a/src/core/shutdown.c +++ b/src/core/shutdown.c @@ -322,7 +322,7 @@ int main(int argc, char *argv[]) { arguments[0] = NULL; arguments[1] = arg_verb; arguments[2] = NULL; - execute_directories(dirs, DEFAULT_TIMEOUT_USEC, arguments); + execute_directories(dirs, DEFAULT_TIMEOUT_USEC, NULL, NULL, arguments); if (!in_container && !in_initrd() && access("/run/initramfs/shutdown", X_OK) == 0) { diff --git a/src/sleep/sleep.c b/src/sleep/sleep.c index b0f992fc9c..a6978b6273 100644 --- a/src/sleep/sleep.c +++ b/src/sleep/sleep.c @@ -107,7 +107,7 @@ static int execute(char **modes, char **states) { if (r < 0) return r; - execute_directories(dirs, DEFAULT_TIMEOUT_USEC, arguments); + execute_directories(dirs, DEFAULT_TIMEOUT_USEC, NULL, NULL, arguments); log_struct(LOG_INFO, LOG_MESSAGE_ID(SD_MESSAGE_SLEEP_START), @@ -126,7 +126,7 @@ static int execute(char **modes, char **states) { NULL); arguments[1] = (char*) "post"; - execute_directories(dirs, DEFAULT_TIMEOUT_USEC, arguments); + execute_directories(dirs, DEFAULT_TIMEOUT_USEC, NULL, NULL, arguments); return r; } diff --git a/src/test/test-exec-util.c b/src/test/test-exec-util.c index 26533f0bf6..41cbef74b1 100644 --- a/src/test/test-exec-util.c +++ b/src/test/test-exec-util.c @@ -24,21 +24,59 @@ #include #include +#include "alloc-util.h" +#include "copy.h" #include "def.h" #include "exec-util.h" +#include "fd-util.h" #include "fileio.h" #include "fs-util.h" #include "log.h" #include "macro.h" #include "rm-rf.h" #include "string-util.h" +#include "strv.h" -static void test_execute_directory(void) { - char template_lo[] = "/tmp/test-readlink_and_make_absolute-lo.XXXXXXX"; - char template_hi[] = "/tmp/test-readlink_and_make_absolute-hi.XXXXXXX"; +static int here = 0, here2 = 0, here3 = 0; +void *ignore_stdout_args[] = {&here, &here2, &here3}; + +/* noop handlers, just check that arguments are passed correctly */ +static int ignore_stdout_func(int fd, void *arg) { + assert(fd >= 0); + assert(arg == &here); + safe_close(fd); + + return 0; +} +static int ignore_stdout_func2(int fd, void *arg) { + assert(fd >= 0); + assert(arg == &here2); + safe_close(fd); + + return 0; +} +static int ignore_stdout_func3(int fd, void *arg) { + assert(fd >= 0); + assert(arg == &here3); + safe_close(fd); + + return 0; +} + +static const gather_stdout_callback_t ignore_stdout[] = { + ignore_stdout_func, + ignore_stdout_func2, + ignore_stdout_func3, +}; + +static void test_execute_directory(bool gather_stdout) { + char template_lo[] = "/tmp/test-exec-util.XXXXXXX"; + char template_hi[] = "/tmp/test-exec-util.XXXXXXX"; const char * dirs[] = {template_hi, template_lo, NULL}; const char *name, *name2, *name3, *overridden, *override, *masked, *mask; + log_info("/* %s (%s) */", __func__, gather_stdout ? "gathering stdout" : "asynchronous"); + assert_se(mkdtemp(template_lo)); assert_se(mkdtemp(template_hi)); @@ -50,20 +88,34 @@ static void test_execute_directory(void) { masked = strjoina(template_lo, "/masked"); mask = strjoina(template_hi, "/masked"); - assert_se(write_string_file(name, "#!/bin/sh\necho 'Executing '$0\ntouch $(dirname $0)/it_works", WRITE_STRING_FILE_CREATE) == 0); - assert_se(write_string_file(name2, "#!/bin/sh\necho 'Executing '$0\ntouch $(dirname $0)/it_works2", WRITE_STRING_FILE_CREATE) == 0); - assert_se(write_string_file(overridden, "#!/bin/sh\necho 'Executing '$0\ntouch $(dirname $0)/failed", WRITE_STRING_FILE_CREATE) == 0); - assert_se(write_string_file(override, "#!/bin/sh\necho 'Executing '$0", WRITE_STRING_FILE_CREATE) == 0); - assert_se(write_string_file(masked, "#!/bin/sh\necho 'Executing '$0\ntouch $(dirname $0)/failed", WRITE_STRING_FILE_CREATE) == 0); + assert_se(write_string_file(name, + "#!/bin/sh\necho 'Executing '$0\ntouch $(dirname $0)/it_works", + WRITE_STRING_FILE_CREATE) == 0); + assert_se(write_string_file(name2, + "#!/bin/sh\necho 'Executing '$0\ntouch $(dirname $0)/it_works2", + WRITE_STRING_FILE_CREATE) == 0); + assert_se(write_string_file(overridden, + "#!/bin/sh\necho 'Executing '$0\ntouch $(dirname $0)/failed", + WRITE_STRING_FILE_CREATE) == 0); + assert_se(write_string_file(override, + "#!/bin/sh\necho 'Executing '$0", + WRITE_STRING_FILE_CREATE) == 0); + assert_se(write_string_file(masked, + "#!/bin/sh\necho 'Executing '$0\ntouch $(dirname $0)/failed", + WRITE_STRING_FILE_CREATE) == 0); assert_se(symlink("/dev/null", mask) == 0); + assert_se(touch(name3) >= 0); + assert_se(chmod(name, 0755) == 0); assert_se(chmod(name2, 0755) == 0); assert_se(chmod(overridden, 0755) == 0); assert_se(chmod(override, 0755) == 0); assert_se(chmod(masked, 0755) == 0); - assert_se(touch(name3) >= 0); - execute_directories(dirs, DEFAULT_TIMEOUT_USEC, NULL); + if (gather_stdout) + execute_directories(dirs, DEFAULT_TIMEOUT_USEC, ignore_stdout, ignore_stdout_args, NULL); + else + execute_directories(dirs, DEFAULT_TIMEOUT_USEC, NULL, NULL, NULL); assert_se(chdir(template_lo) == 0); assert_se(access("it_works", F_OK) >= 0); @@ -77,11 +129,157 @@ static void test_execute_directory(void) { (void) rm_rf(template_hi, REMOVE_ROOT|REMOVE_PHYSICAL); } +static void test_execution_order(void) { + char template_lo[] = "/tmp/test-exec-util-lo.XXXXXXX"; + char template_hi[] = "/tmp/test-exec-util-hi.XXXXXXX"; + const char *dirs[] = {template_hi, template_lo, NULL}; + const char *name, *name2, *name3, *overridden, *override, *masked, *mask; + const char *output, *t; + _cleanup_free_ char *contents = NULL; + + assert_se(mkdtemp(template_lo)); + assert_se(mkdtemp(template_hi)); + + output = strjoina(template_hi, "/output"); + + log_info("/* %s >>%s */", __func__, output); + + /* write files in "random" order */ + name2 = strjoina(template_lo, "/90-bar"); + name = strjoina(template_hi, "/80-foo"); + name3 = strjoina(template_lo, "/last"); + overridden = strjoina(template_lo, "/30-override"); + override = strjoina(template_hi, "/30-override"); + masked = strjoina(template_lo, "/10-masked"); + mask = strjoina(template_hi, "/10-masked"); + + t = strjoina("#!/bin/sh\necho $(basename $0) >>", output); + assert_se(write_string_file(name, t, WRITE_STRING_FILE_CREATE) == 0); + + t = strjoina("#!/bin/sh\necho $(basename $0) >>", output); + assert_se(write_string_file(name2, t, WRITE_STRING_FILE_CREATE) == 0); + + t = strjoina("#!/bin/sh\necho $(basename $0) >>", output); + assert_se(write_string_file(name3, t, WRITE_STRING_FILE_CREATE) == 0); + + t = strjoina("#!/bin/sh\necho OVERRIDDEN >>", output); + assert_se(write_string_file(overridden, t, WRITE_STRING_FILE_CREATE) == 0); + + t = strjoina("#!/bin/sh\necho $(basename $0) >>", output); + assert_se(write_string_file(override, t, WRITE_STRING_FILE_CREATE) == 0); + + t = strjoina("#!/bin/sh\necho MASKED >>", output); + assert_se(write_string_file(masked, t, WRITE_STRING_FILE_CREATE) == 0); + + assert_se(symlink("/dev/null", mask) == 0); + + assert_se(chmod(name, 0755) == 0); + assert_se(chmod(name2, 0755) == 0); + assert_se(chmod(name3, 0755) == 0); + assert_se(chmod(overridden, 0755) == 0); + assert_se(chmod(override, 0755) == 0); + assert_se(chmod(masked, 0755) == 0); + + execute_directories(dirs, DEFAULT_TIMEOUT_USEC, ignore_stdout, ignore_stdout_args, NULL); + + assert_se(read_full_file(output, &contents, NULL) >= 0); + assert_se(streq(contents, "30-override\n80-foo\n90-bar\nlast\n")); + + (void) rm_rf(template_lo, REMOVE_ROOT|REMOVE_PHYSICAL); + (void) rm_rf(template_hi, REMOVE_ROOT|REMOVE_PHYSICAL); +} + +static int gather_stdout_one(int fd, void *arg) { + char ***s = arg, *t; + char buf[128] = {}; + + assert_se(s); + assert_se(read(fd, buf, sizeof buf) >= 0); + safe_close(fd); + + assert_se(t = strndup(buf, sizeof buf)); + assert_se(strv_push(s, t) >= 0); + + return 0; +} +static int gather_stdout_two(int fd, void *arg) { + char ***s = arg, **t; + + STRV_FOREACH(t, *s) + assert_se(write(fd, *t, strlen(*t)) == (ssize_t) strlen(*t)); + safe_close(fd); + + return 0; +} +static int gather_stdout_three(int fd, void *arg) { + char **s = arg; + char buf[128] = {}; + + assert_se(read(fd, buf, sizeof buf - 1) > 0); + safe_close(fd); + assert_se(*s = strndup(buf, sizeof buf)); + + return 0; +} + +const gather_stdout_callback_t const gather_stdout[] = { + gather_stdout_one, + gather_stdout_two, + gather_stdout_three, +}; + + +static void test_stdout_gathering(void) { + char template[] = "/tmp/test-exec-util.XXXXXXX"; + const char *dirs[] = {template, NULL}; + const char *name, *name2, *name3; + int r; + + char **tmp = NULL; /* this is only used in the forked process, no cleanup here */ + _cleanup_free_ char *output = NULL; + + void* args[] = {&tmp, &tmp, &output}; + + assert_se(mkdtemp(template)); + + log_info("/* %s */", __func__); + + /* write files */ + name = strjoina(template, "/10-foo"); + name2 = strjoina(template, "/20-bar"); + name3 = strjoina(template, "/30-last"); + + assert_se(write_string_file(name, + "#!/bin/sh\necho a\necho b\necho c\n", + WRITE_STRING_FILE_CREATE) == 0); + assert_se(write_string_file(name2, + "#!/bin/sh\necho d\n", + WRITE_STRING_FILE_CREATE) == 0); + assert_se(write_string_file(name3, + "#!/bin/sh\nsleep 1", + WRITE_STRING_FILE_CREATE) == 0); + + assert_se(chmod(name, 0755) == 0); + assert_se(chmod(name2, 0755) == 0); + assert_se(chmod(name3, 0755) == 0); + + r = execute_directories(dirs, DEFAULT_TIMEOUT_USEC, gather_stdout, args, NULL); + assert_se(r >= 0); + + log_info("got: %s", output); + + assert_se(streq(output, "a\nb\nc\nd\n")); +} + int main(int argc, char *argv[]) { + log_set_max_level(LOG_DEBUG); log_parse_environment(); log_open(); - test_execute_directory(); + test_execute_directory(true); + test_execute_directory(false); + test_execution_order(); + test_stdout_gathering(); return 0; } -- cgit v1.2.3-54-g00ecf From 71cb7d306ab42712f274337c457ac5cbdeff363c Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Fri, 10 Feb 2017 15:41:42 -0500 Subject: core/manager: fix grammar in comment --- src/core/manager.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'src/core') diff --git a/src/core/manager.c b/src/core/manager.c index 73ac7499bd..eb9c4e65c6 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -531,9 +531,9 @@ static int manager_default_environment(Manager *m) { if (MANAGER_IS_SYSTEM(m)) { /* The system manager always starts with a clean * environment for its children. It does not import - * the kernel or the parents exported variables. + * the kernel's or the parents' exported variables. * - * The initial passed environ is untouched to keep + * The initial passed environment is untouched to keep * /proc/self/environ valid; it is used for tagging * the init process inside containers. */ m->environment = strv_new("PATH=" DEFAULT_PATH, @@ -541,11 +541,10 @@ static int manager_default_environment(Manager *m) { /* Import locale variables LC_*= from configuration */ locale_setup(&m->environment); - } else { + } else /* The user manager passes its own environment * along to its children. */ m->environment = strv_copy(environ); - } if (!m->environment) return -ENOMEM; -- cgit v1.2.3-54-g00ecf From fe902fa496abb4583c5befaf671a2402b650cd14 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Fri, 10 Feb 2017 21:44:21 -0500 Subject: core/manager: move environment serialization out to basic/env-util.c This protocol is generally useful, we might just as well reuse it for the env. generators. The implementation is changed a bit: instead of making a new strv and freeing the old one, just mutate the original. This is much faster with larger arrays, while in fact atomicity is preserved, since we only either insert the new entry or not, without being in inconsistent state. v2: - fix confusion with return value --- src/basic/env-util.c | 34 ++++++++++++++++++++++++++++++++++ src/basic/env-util.h | 4 ++++ src/core/manager.c | 30 ++++-------------------------- 3 files changed, 42 insertions(+), 26 deletions(-) (limited to 'src/core') diff --git a/src/basic/env-util.c b/src/basic/env-util.c index 96da38d45e..4645ec781e 100644 --- a/src/basic/env-util.c +++ b/src/basic/env-util.c @@ -26,6 +26,7 @@ #include "alloc-util.h" #include "env-util.h" +#include "escape.h" #include "extract-word.h" #include "macro.h" #include "parse-util.h" @@ -644,3 +645,36 @@ int getenv_bool(const char *p) { return parse_boolean(e); } + +int serialize_environment(FILE *f, char **environment) { + char **e; + + STRV_FOREACH(e, environment) { + _cleanup_free_ char *ce; + + ce = cescape(*e); + if (!ce) + return -ENOMEM; + + fprintf(f, "env=%s\n", *e); + } + + /* caller should call ferror() */ + + return 0; +} + +int deserialize_environment(char ***environment, const char *line) { + char *uce = NULL; + int r; + + assert(line); + assert(environment); + + assert(startswith(line, "env=")); + r = cunescape(line + 4, UNESCAPE_RELAX, &uce); + if (r < 0) + return r; + + return strv_env_replace(environment, uce); +} diff --git a/src/basic/env-util.h b/src/basic/env-util.h index 8cb0fc2131..90df5b1cd9 100644 --- a/src/basic/env-util.h +++ b/src/basic/env-util.h @@ -21,6 +21,7 @@ #include #include +#include #include "macro.h" @@ -50,3 +51,6 @@ char *strv_env_get_n(char **l, const char *name, size_t k) _pure_; char *strv_env_get(char **x, const char *n) _pure_; int getenv_bool(const char *p); + +int serialize_environment(FILE *f, char **environment); +int deserialize_environment(char ***environment, const char *line); diff --git a/src/core/manager.c b/src/core/manager.c index eb9c4e65c6..6bbda1af0d 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -2459,7 +2459,6 @@ int manager_serialize(Manager *m, FILE *f, FDSet *fds, bool switching_root) { Iterator i; Unit *u; const char *t; - char **e; int r; assert(m); @@ -2489,17 +2488,8 @@ int manager_serialize(Manager *m, FILE *f, FDSet *fds, bool switching_root) { dual_timestamp_serialize(f, "units-load-finish-timestamp", &m->units_load_finish_timestamp); } - if (!switching_root) { - STRV_FOREACH(e, m->environment) { - _cleanup_free_ char *ce; - - ce = cescape(*e); - if (!ce) - return -ENOMEM; - - fprintf(f, "env=%s\n", *e); - } - } + if (!switching_root) + (void) serialize_environment(f, m->environment); if (m->notify_fd >= 0) { int copy; @@ -2662,21 +2652,9 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) { else if ((val = startswith(l, "units-load-finish-timestamp="))) dual_timestamp_deserialize(val, &m->units_load_finish_timestamp); else if (startswith(l, "env=")) { - _cleanup_free_ char *uce = NULL; - char **e; - - r = cunescape(l + 4, UNESCAPE_RELAX, &uce); + r = deserialize_environment(&m->environment, l); if (r < 0) - goto finish; - - e = strv_env_set(m->environment, uce); - if (!e) { - r = -ENOMEM; - goto finish; - } - - strv_free(m->environment); - m->environment = e; + return r; } else if ((val = startswith(l, "notify-fd="))) { int fd; -- cgit v1.2.3-54-g00ecf From 64691d2024d3bd202c85c52069f185afee2b8e43 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Sun, 22 Jan 2017 01:13:47 -0500 Subject: manager: run environment generators Environment file generators are a lot like unit file generators, but not exactly: 1. environment file generators are run for each manager instance, and their output is (or at least can be) individualized. The generators themselves are system-wide, the same for all users. 2. environment file generators are run sequentially, in priority order. Thus, the lifetime of those files is tied to lifecycle of the manager instance. Because generators are run sequentially, later generators can use or modify the output of earlier generators. Each generator is run with no arguments, and the whole state is stored in the environment variables. The generator can echo a set of variable assignments to standard output: VAR_A=something VAR_B=something else This output is parsed, and the next and subsequent generators run with those updated variables in the environment. After the last generator is done, the environment that the manager itself exports is updated. Each generator must return 0, otherwise the output is ignored. The generators in */user-env-generator are for the user session managers, including root, and the ones in */system-env-generator are for pid1. --- Makefile.am | 6 ++++ src/core/macros.systemd.in | 2 ++ src/core/manager.c | 70 ++++++++++++++++++++++++++++++++++++++-------- 3 files changed, 66 insertions(+), 12 deletions(-) (limited to 'src/core') diff --git a/Makefile.am b/Makefile.am index 003ec9bfb7..f6699252d1 100644 --- a/Makefile.am +++ b/Makefile.am @@ -80,6 +80,8 @@ networkdir=$(rootprefix)/lib/systemd/network pkgincludedir=$(includedir)/systemd systemgeneratordir=$(rootlibexecdir)/system-generators usergeneratordir=$(prefix)/lib/systemd/user-generators +systemenvgeneratordir=$(prefix)/lib/systemd/system-environment-generators +userenvgeneratordir=$(prefix)/lib/systemd/user-environment-generators systemshutdowndir=$(rootlibexecdir)/system-shutdown systemsleepdir=$(rootlibexecdir)/system-sleep systemunitdir=$(rootprefix)/lib/systemd/system @@ -205,6 +207,8 @@ AM_CPPFLAGS = \ -DSYSTEMD_CRYPTSETUP_PATH=\"$(rootlibexecdir)/systemd-cryptsetup\" \ -DSYSTEM_GENERATOR_PATH=\"$(systemgeneratordir)\" \ -DUSER_GENERATOR_PATH=\"$(usergeneratordir)\" \ + -DSYSTEM_ENV_GENERATOR_PATH=\"$(systemenvgeneratordir)\" \ + -DUSER_ENV_GENERATOR_PATH=\"$(userenvgeneratordir)\" \ -DSYSTEM_SHUTDOWN_PATH=\"$(systemshutdowndir)\" \ -DSYSTEM_SLEEP_PATH=\"$(systemsleepdir)\" \ -DSYSTEMD_KBD_MODEL_MAP=\"$(pkgdatadir)/kbd-model-map\" \ @@ -6210,6 +6214,8 @@ substitutions = \ '|sysctldir=$(sysctldir)|' \ '|systemgeneratordir=$(systemgeneratordir)|' \ '|usergeneratordir=$(usergeneratordir)|' \ + '|systemenvgeneratordir=$(systemenvgeneratordir)|' \ + '|userenvgeneratordir=$(userenvgeneratordir)|' \ '|CERTIFICATEROOT=$(CERTIFICATEROOT)|' \ '|PACKAGE_VERSION=$(PACKAGE_VERSION)|' \ '|PACKAGE_NAME=$(PACKAGE_NAME)|' \ diff --git a/src/core/macros.systemd.in b/src/core/macros.systemd.in index 8d7ce1c238..a2a7edd1ee 100644 --- a/src/core/macros.systemd.in +++ b/src/core/macros.systemd.in @@ -31,6 +31,8 @@ %_binfmtdir @binfmtdir@ %_systemdgeneratordir @systemgeneratordir@ %_systemdusergeneratordir @usergeneratordir@ +%_systemd_system_env_generator_dir @systemenvgeneratordir@ +%_systemd_user_env_generator_dir @userenvgeneratordir@ %systemd_requires \ Requires(post): systemd \ diff --git a/src/core/manager.c b/src/core/manager.c index 6bbda1af0d..aefe4ca780 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -103,6 +103,7 @@ static int manager_dispatch_idle_pipe_fd(sd_event_source *source, int fd, uint32 static int manager_dispatch_user_lookup_fd(sd_event_source *source, int fd, uint32_t revents, void *userdata); static int manager_dispatch_jobs_in_progress(sd_event_source *source, usec_t usec, void *userdata); static int manager_dispatch_run_queue(sd_event_source *source, void *userdata); +static int manager_run_environment_generators(Manager *m); static int manager_run_generators(Manager *m); static void manager_watch_jobs_in_progress(Manager *m) { @@ -1262,6 +1263,10 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds) { if (r < 0) return r; + r = manager_run_environment_generators(m); + if (r < 0) + return r; + /* Make sure the transient directory always exists, so that it remains in the search path */ if (!m->test_run) { r = mkdir_p_label(m->lookup_paths.transient, 0755); @@ -2795,6 +2800,10 @@ int manager_reload(Manager *m) { if (q < 0 && r >= 0) r = q; + q = manager_run_environment_generators(m); + if (q < 0 && r >= 0) + r = q; + /* Find new unit paths */ q = manager_run_generators(m); if (q < 0 && r >= 0) @@ -2986,10 +2995,56 @@ void manager_check_finished(Manager *m) { manager_invalidate_startup_units(m); } +static bool generator_path_any(const char* const* paths) { + char **path; + bool found = false; + + /* Optimize by skipping the whole process by not creating output directories + * if no generators are found. */ + STRV_FOREACH(path, (char**) paths) + if (access(*path, F_OK) == 0) + found = true; + else if (errno != ENOENT) + log_warning_errno(errno, "Failed to open generator directory %s: %m", *path); + + return found; +} + +static const char* system_env_generator_binary_paths[] = { + "/run/systemd/system-environment-generators", + "/etc/systemd/system-environment-generators", + "/usr/local/lib/systemd/system-environment-generators", + SYSTEM_ENV_GENERATOR_PATH, + NULL +}; + +static const char* user_env_generator_binary_paths[] = { + "/run/systemd/user-environment-generators", + "/etc/systemd/user-environment-generators", + "/usr/local/lib/systemd/user-environment-generators", + USER_ENV_GENERATOR_PATH, + NULL +}; + +static int manager_run_environment_generators(Manager *m) { + char **tmp = NULL; /* this is only used in the forked process, no cleanup here */ + const char **paths; + void* args[] = {&tmp, &tmp, &m->environment}; + + if (m->test_run) + return 0; + + paths = MANAGER_IS_SYSTEM(m) ? system_env_generator_binary_paths : user_env_generator_binary_paths; + + if (!generator_path_any(paths)) + return 0; + + return execute_directories(paths, DEFAULT_TIMEOUT_USEC, gather_environment, args, NULL); +} + static int manager_run_generators(Manager *m) { _cleanup_strv_free_ char **paths = NULL; const char *argv[5]; - char **path; int r; assert(m); @@ -3001,18 +3056,9 @@ static int manager_run_generators(Manager *m) { if (!paths) return log_oom(); - /* Optimize by skipping the whole process by not creating output directories - * if no generators are found. */ - STRV_FOREACH(path, paths) { - if (access(*path, F_OK) >= 0) - goto found; - if (errno != ENOENT) - log_warning_errno(errno, "Failed to open generator directory %s: %m", *path); - } - - return 0; + if (!generator_path_any((const char* const*) paths)) + return 0; - found: r = lookup_paths_mkdir_generator(&m->lookup_paths); if (r < 0) goto finish; -- cgit v1.2.3-54-g00ecf