From 4b05f0c9d9f6df4e0dab518f8e1ae537bf948f92 Mon Sep 17 00:00:00 2001 From: Matthias Klumpp Date: Sun, 9 Sep 2018 03:18:45 +0200 Subject: [PATCH] core: Allow to configure execute_directories execution behavior This adds a new bitfield to `execute_directories()` which allows to configure whether to ignore non-zero exit statuses of binaries run and whether to allow parallel execution of commands. In case errors are not ignored, the exit status of the failed script will now be returned for error reposrting purposes or other further future use. --- src/core/manager.c | 8 +++--- src/core/shutdown.c | 2 +- src/shared/exec-util.c | 53 ++++++++++++++++++++++++++------------- src/shared/exec-util.h | 9 ++++++- src/sleep/sleep.c | 4 +-- src/test/test-exec-util.c | 48 ++++++++++++++++++++++++++++++----- 6 files changed, 93 insertions(+), 31 deletions(-) diff --git a/src/core/manager.c b/src/core/manager.c index fc6f89f85b..f305dc6647 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -3811,8 +3811,8 @@ static int manager_run_environment_generators(Manager *m) { return 0; RUN_WITH_UMASK(0022) - r = execute_directories(paths, DEFAULT_TIMEOUT_USEC, gather_environment, args, NULL, m->transient_environment); - + r = execute_directories(paths, DEFAULT_TIMEOUT_USEC, gather_environment, + args, NULL, m->transient_environment, EXEC_DIR_PARALLEL | EXEC_DIR_IGNORE_ERRORS); return r; } @@ -3846,8 +3846,8 @@ static int manager_run_generators(Manager *m) { argv[4] = NULL; RUN_WITH_UMASK(0022) - (void) execute_directories((const char* const*) paths, DEFAULT_TIMEOUT_USEC, - NULL, NULL, (char**) argv, m->transient_environment); + (void) execute_directories((const char* const*) paths, DEFAULT_TIMEOUT_USEC, NULL, NULL, + (char**) argv, m->transient_environment, EXEC_DIR_PARALLEL | EXEC_DIR_IGNORE_ERRORS); r = 0; diff --git a/src/core/shutdown.c b/src/core/shutdown.c index cb47ee8984..56e9a2480a 100644 --- a/src/core/shutdown.c +++ b/src/core/shutdown.c @@ -442,7 +442,7 @@ int main(int argc, char *argv[]) { arguments[0] = NULL; arguments[1] = arg_verb; arguments[2] = NULL; - execute_directories(dirs, DEFAULT_TIMEOUT_USEC, NULL, NULL, arguments, NULL); + execute_directories(dirs, DEFAULT_TIMEOUT_USEC, NULL, NULL, arguments, NULL, EXEC_DIR_PARALLEL | EXEC_DIR_IGNORE_ERRORS); (void) rlimit_nofile_safe(); diff --git a/src/shared/exec-util.c b/src/shared/exec-util.c index 17a278a00f..2867f08a7a 100644 --- a/src/shared/exec-util.c +++ b/src/shared/exec-util.c @@ -78,24 +78,28 @@ static int do_execute( void* const callback_args[_STDOUT_CONSUME_MAX], int output_fd, char *argv[], - char *envp[]) { + char *envp[], + ExecDirFlags flags) { _cleanup_hashmap_free_free_ Hashmap *pids = NULL; _cleanup_strv_free_ char **paths = NULL; char **path, **e; int r; + bool parallel_execution; /* 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. + * We attempt to perform parallel execution if configured by the user, however + * if `callbacks` is nonnull, execution must be serial. */ + parallel_execution = FLAGS_SET(flags, EXEC_DIR_PARALLEL) && !callbacks; r = conf_files_list_strv(&paths, NULL, NULL, CONF_FILES_EXECUTABLE|CONF_FILES_REGULAR|CONF_FILES_FILTER_MASKED, (const char* const*) directories); if (r < 0) return log_error_errno(r, "Failed to enumerate executables: %m"); - if (!callbacks) { + if (parallel_execution) { pids = hashmap_new(NULL); if (!pids) return log_oom(); @@ -130,23 +134,28 @@ static int do_execute( if (r <= 0) continue; - if (pids) { + if (parallel_execution) { r = hashmap_put(pids, PID_TO_PTR(pid), t); if (r < 0) return log_oom(); t = NULL; } else { r = wait_for_terminate_and_check(t, pid, WAIT_LOG); - if (r < 0) - continue; + if (FLAGS_SET(flags, EXEC_DIR_IGNORE_ERRORS)) { + if (r < 0) + continue; + } else if (r > 0) + return r; - if (lseek(fd, 0, SEEK_SET) < 0) - return log_error_errno(errno, "Failed to seek on serialization fd: %m"); + if (callbacks) { + 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); + 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); + } } } @@ -166,7 +175,9 @@ static int do_execute( t = hashmap_remove(pids, PID_TO_PTR(pid)); assert(t); - (void) wait_for_terminate_and_check(t, pid, WAIT_LOG); + r = wait_for_terminate_and_check(t, pid, WAIT_LOG); + if (!FLAGS_SET(flags, EXEC_DIR_IGNORE_ERRORS) && r > 0) + return r; } return 0; @@ -178,12 +189,14 @@ int execute_directories( gather_stdout_callback_t const callbacks[_STDOUT_CONSUME_MAX], void* const callback_args[_STDOUT_CONSUME_MAX], char *argv[], - char *envp[]) { + char *envp[], + ExecDirFlags flags) { char **dirs = (char**) directories; _cleanup_close_ int fd = -1; char *name; int r; + pid_t executor_pid; assert(!strv_isempty(dirs)); @@ -205,14 +218,20 @@ int execute_directories( * 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. */ - r = safe_fork("(sd-executor)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_LOG|FORK_WAIT, NULL); + r = safe_fork("(sd-executor)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_LOG, &executor_pid); if (r < 0) return r; if (r == 0) { - r = do_execute(dirs, timeout, callbacks, callback_args, fd, argv, envp); - _exit(r < 0 ? EXIT_FAILURE : EXIT_SUCCESS); + r = do_execute(dirs, timeout, callbacks, callback_args, fd, argv, envp, flags); + _exit(r < 0 ? EXIT_FAILURE : r); } + r = wait_for_terminate_and_check("(sd-executor)", executor_pid, 0); + if (r < 0) + return r; + if (!FLAGS_SET(flags, EXEC_DIR_IGNORE_ERRORS) && r > 0) + return r; + if (!callbacks) return 0; diff --git a/src/shared/exec-util.h b/src/shared/exec-util.h index 6ac3c9000a..5b75a40229 100644 --- a/src/shared/exec-util.h +++ b/src/shared/exec-util.h @@ -14,12 +14,19 @@ enum { _STDOUT_CONSUME_MAX, }; +typedef enum { + EXEC_DIR_NONE = 0, /* No execdir flags */ + EXEC_DIR_PARALLEL = 1 << 0, /* Execute scripts in parallel, if possible */ + EXEC_DIR_IGNORE_ERRORS = 1 << 1, /* Ignore non-zero exit status of scripts */ +} ExecDirFlags; + 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[], - char *envp[]); + char *envp[], + ExecDirFlags flags); extern const gather_stdout_callback_t gather_environment[_STDOUT_CONSUME_MAX]; diff --git a/src/sleep/sleep.c b/src/sleep/sleep.c index 5b7984a6f2..4a7ffbd979 100644 --- a/src/sleep/sleep.c +++ b/src/sleep/sleep.c @@ -166,7 +166,7 @@ static int execute(char **modes, char **states) { return log_error_errno(r, "Failed to write mode to /sys/power/disk: %m");; } - execute_directories(dirs, DEFAULT_TIMEOUT_USEC, NULL, NULL, arguments, NULL); + execute_directories(dirs, DEFAULT_TIMEOUT_USEC, NULL, NULL, arguments, NULL, EXEC_DIR_PARALLEL | EXEC_DIR_IGNORE_ERRORS); log_struct(LOG_INFO, "MESSAGE_ID=" SD_MESSAGE_SLEEP_START_STR, @@ -186,7 +186,7 @@ static int execute(char **modes, char **states) { "SLEEP=%s", arg_verb); arguments[1] = (char*) "post"; - execute_directories(dirs, DEFAULT_TIMEOUT_USEC, NULL, NULL, arguments, NULL); + execute_directories(dirs, DEFAULT_TIMEOUT_USEC, NULL, NULL, arguments, NULL, EXEC_DIR_PARALLEL | EXEC_DIR_IGNORE_ERRORS); return r; } diff --git a/src/test/test-exec-util.c b/src/test/test-exec-util.c index 21a4538d74..25ca7a2bc9 100644 --- a/src/test/test-exec-util.c +++ b/src/test/test-exec-util.c @@ -117,9 +117,9 @@ static void test_execute_directory(bool gather_stdout) { assert_se(chmod(mask2e, 0755) == 0); if (gather_stdout) - execute_directories(dirs, DEFAULT_TIMEOUT_USEC, ignore_stdout, ignore_stdout_args, NULL, NULL); + execute_directories(dirs, DEFAULT_TIMEOUT_USEC, ignore_stdout, ignore_stdout_args, NULL, NULL, EXEC_DIR_PARALLEL | EXEC_DIR_IGNORE_ERRORS); else - execute_directories(dirs, DEFAULT_TIMEOUT_USEC, NULL, NULL, NULL, NULL); + execute_directories(dirs, DEFAULT_TIMEOUT_USEC, NULL, NULL, NULL, NULL, EXEC_DIR_PARALLEL | EXEC_DIR_IGNORE_ERRORS); assert_se(chdir(template_lo) == 0); assert_se(access("it_works", F_OK) >= 0); @@ -184,7 +184,7 @@ static void test_execution_order(void) { assert_se(chmod(override, 0755) == 0); assert_se(chmod(masked, 0755) == 0); - execute_directories(dirs, DEFAULT_TIMEOUT_USEC, ignore_stdout, ignore_stdout_args, NULL, NULL); + execute_directories(dirs, DEFAULT_TIMEOUT_USEC, ignore_stdout, ignore_stdout_args, NULL, NULL, EXEC_DIR_PARALLEL | EXEC_DIR_IGNORE_ERRORS); assert_se(read_full_file(output, &contents, NULL) >= 0); assert_se(streq(contents, "30-override\n80-foo\n90-bar\nlast\n")); @@ -266,7 +266,7 @@ static void test_stdout_gathering(void) { assert_se(chmod(name2, 0755) == 0); assert_se(chmod(name3, 0755) == 0); - r = execute_directories(dirs, DEFAULT_TIMEOUT_USEC, gather_stdout, args, NULL, NULL); + r = execute_directories(dirs, DEFAULT_TIMEOUT_USEC, gather_stdout, args, NULL, NULL, EXEC_DIR_PARALLEL | EXEC_DIR_IGNORE_ERRORS); assert_se(r >= 0); log_info("got: %s", output); @@ -332,7 +332,7 @@ static void test_environment_gathering(void) { r = setenv("PATH", "no-sh-built-in-path", 1); assert_se(r >= 0); - r = execute_directories(dirs, DEFAULT_TIMEOUT_USEC, gather_environment, args, NULL, NULL); + r = execute_directories(dirs, DEFAULT_TIMEOUT_USEC, gather_environment, args, NULL, NULL, EXEC_DIR_PARALLEL | EXEC_DIR_IGNORE_ERRORS); assert_se(r >= 0); STRV_FOREACH(p, env) @@ -349,7 +349,7 @@ static void test_environment_gathering(void) { env = strv_new("PATH=" DEFAULT_PATH); assert_se(env); - r = execute_directories(dirs, DEFAULT_TIMEOUT_USEC, gather_environment, args, NULL, env); + r = execute_directories(dirs, DEFAULT_TIMEOUT_USEC, gather_environment, args, NULL, env, EXEC_DIR_PARALLEL | EXEC_DIR_IGNORE_ERRORS); assert_se(r >= 0); STRV_FOREACH(p, env) @@ -364,6 +364,41 @@ static void test_environment_gathering(void) { (void) setenv("PATH", old, 1); } +static void test_error_catching(void) { + char template[] = "/tmp/test-exec-util.XXXXXXX"; + const char *dirs[] = {template, NULL}; + const char *name, *name2, *name3; + int r; + + 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\nexit 42\n", + WRITE_STRING_FILE_CREATE) == 0); + assert_se(write_string_file(name3, + "#!/bin/sh\nexit 12", + 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, NULL, NULL, NULL, NULL, EXEC_DIR_NONE); + + /* we should exit with the error code of the first script that failed */ + assert_se(r == 42); +} + int main(int argc, char *argv[]) { test_setup_logging(LOG_DEBUG); @@ -372,6 +407,7 @@ int main(int argc, char *argv[]) { test_execution_order(); test_stdout_gathering(); test_environment_gathering(); + test_error_catching(); return 0; }