core: Allow to configure execute_directories execution behavior
authorMatthias Klumpp <matthias@tenstral.net>
Sun, 9 Sep 2018 01:18:45 +0000 (03:18 +0200)
committerLennart Poettering <lennart@poettering.net>
Mon, 18 Feb 2019 15:16:02 +0000 (16:16 +0100)
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
src/core/shutdown.c
src/shared/exec-util.c
src/shared/exec-util.h
src/sleep/sleep.c
src/test/test-exec-util.c

index fc6f89f..f305dc6 100644 (file)
@@ -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;
 
index cb47ee8..56e9a24 100644 (file)
@@ -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();
 
index 17a278a..2867f08 100644 (file)
@@ -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 (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);
+                        if (FLAGS_SET(flags, EXEC_DIR_IGNORE_ERRORS)) {
+                                if (r < 0)
+                                        continue;
+                        } else if (r > 0)
+                                return r;
+
+                        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);
+                        }
                 }
         }
 
@@ -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;
 
index 6ac3c90..5b75a40 100644 (file)
@@ -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];
index 5b7984a..4a7ffbd 100644 (file)
@@ -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;
 }
index 21a4538..25ca7a2 100644 (file)
@@ -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;
 }