From 66b2d758c53dbb9da0127043097996991ec5b54f Mon Sep 17 00:00:00 2001 From: =?utf8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 1 Sep 2025 13:52:35 +0200 Subject: [PATCH] various: add a fixed name to log about plugin execution Function execute_directories logged in a way that was meaningless without additional context: systemd[1]: No executables found. In execute_strv this was partially rectified by extracting the directory name from one of the directories and using this as the identifier. But the directory name is not always meaningful, and can also be set from an environment variable. Let's simplify things by providing a fixed name that can be used consistently in all log messages. In particular this will make error messages easier to understand if users report just the error without additional context. --- src/core/manager.c | 2 ++ src/kernel-install/kernel-install.c | 2 +- src/libsystemd/sd-path/path-lookup.c | 1 - src/shared/exec-util.c | 27 ++++++++++++--------------- src/shared/exec-util.h | 1 + src/shutdown/shutdown.c | 10 +++++++++- src/sleep/sleep.c | 21 ++++++++++++++++++--- src/test/test-exec-util.c | 22 +++++++++++++++------- 8 files changed, 58 insertions(+), 28 deletions(-) diff --git a/src/core/manager.c b/src/core/manager.c index d85896577f3..5bba7e81b0c 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -3953,6 +3953,7 @@ static int manager_run_environment_generators(Manager *m) { WITH_UMASK(0022) r = execute_directories( + "environment-generators", (const char* const*) paths, DEFAULT_TIMEOUT_USEC, gather_environment, @@ -4070,6 +4071,7 @@ static int manager_execute_generators(Manager *m, char * const *paths, bool remo BLOCK_WITH_UMASK(0022); return execute_directories( + "generators", (const char* const*) paths, DEFAULT_TIMEOUT_USEC, /* callbacks= */ NULL, /* callback_args= */ NULL, diff --git a/src/kernel-install/kernel-install.c b/src/kernel-install/kernel-install.c index f17e99004b4..a2faa0f8506 100644 --- a/src/kernel-install/kernel-install.c +++ b/src/kernel-install/kernel-install.c @@ -1061,7 +1061,7 @@ static int context_execute(Context *c) { } ret = execute_strv( - /* name = */ NULL, + "plugins", c->plugins, /* root = */ NULL, USEC_INFINITY, diff --git a/src/libsystemd/sd-path/path-lookup.c b/src/libsystemd/sd-path/path-lookup.c index 3ddbda0e9f8..55d9d4353a1 100644 --- a/src/libsystemd/sd-path/path-lookup.c +++ b/src/libsystemd/sd-path/path-lookup.c @@ -678,7 +678,6 @@ static const char* const user_env_generator_paths[] = { }; char** generator_binary_paths_internal(RuntimeScope scope, bool env_generator) { - static const struct { const char *env_name; const char * const *paths[_RUNTIME_SCOPE_MAX]; diff --git a/src/shared/exec-util.c b/src/shared/exec-util.c index a64bcfe1251..193b97c4675 100644 --- a/src/shared/exec-util.c +++ b/src/shared/exec-util.c @@ -243,13 +243,13 @@ int execute_strv( pid_t executor_pid; int r; + assert(name); assert(!FLAGS_SET(flags, EXEC_DIR_PARALLEL | EXEC_DIR_SKIP_REMAINING)); if (strv_isempty(paths)) return 0; if (callbacks) { - assert(name); assert(callbacks[STDOUT_GENERATE]); assert(callbacks[STDOUT_COLLECT]); assert(callbacks[STDOUT_CONSUME]); @@ -257,14 +257,16 @@ int execute_strv( fd = open_serialization_fd(name); if (fd < 0) - return log_error_errno(fd, "Failed to open serialization file: %m"); + return log_error_errno(fd, "Failed to open serialization file for %s: %m", name); } /* 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. */ - r = safe_fork("(sd-exec-strv)", FORK_RESET_SIGNALS|FORK_DEATHSIG_SIGTERM|FORK_LOG, &executor_pid); + const char *process_name = strjoina("(", name, ")"); + + r = safe_fork(process_name, FORK_RESET_SIGNALS|FORK_DEATHSIG_SIGTERM|FORK_LOG, &executor_pid); if (r < 0) return r; if (r == 0) { @@ -272,7 +274,7 @@ int execute_strv( _exit(r < 0 ? EXIT_FAILURE : r); } - r = wait_for_terminate_and_check("(sd-exec-strv)", executor_pid, 0); + r = wait_for_terminate_and_check(process_name, executor_pid, 0); if (r < 0) return r; if (!FLAGS_SET(flags, EXEC_DIR_IGNORE_ERRORS) && r > 0) @@ -283,16 +285,17 @@ int execute_strv( r = finish_serialization_fd(fd); if (r < 0) - return log_error_errno(r, "Failed to finish serialization fd: %m"); + return log_error_errno(r, "Failed to finish serialization fd for %s: %m", name); r = callbacks[STDOUT_CONSUME](TAKE_FD(fd), callback_args[STDOUT_CONSUME]); if (r < 0) - return log_error_errno(r, "Failed to parse returned data: %m"); + return log_error_errno(r, "Failed to parse returned data for %s: %m", name); return 0; } int execute_directories( + const char *name, const char * const *directories, usec_t timeout, gather_stdout_callback_t const callbacks[_STDOUT_CONSUME_MAX], @@ -302,9 +305,9 @@ int execute_directories( ExecDirFlags flags) { _cleanup_strv_free_ char **paths = NULL; - _cleanup_free_ char *name = NULL; int r; + assert(name); assert(!strv_isempty((char* const*) directories)); r = conf_files_list_strv( @@ -314,19 +317,13 @@ int execute_directories( CONF_FILES_EXECUTABLE|CONF_FILES_REGULAR|CONF_FILES_FILTER_MASKED, directories); if (r < 0) - return log_error_errno(r, "Failed to enumerate executables: %m"); + return log_error_errno(r, "%s: failed to enumerate executables: %m", name); if (strv_isempty(paths)) { - log_debug("No executables found."); + log_debug("%s: no executables found.", name); return 0; } - if (callbacks) { - r = path_extract_filename(directories[0], &name); - if (r < 0) - return log_error_errno(r, "Failed to extract file name from '%s': %m", directories[0]); - } - return execute_strv(name, paths, /* root = */ NULL, timeout, callbacks, callback_args, argv, envp, flags); } diff --git a/src/shared/exec-util.h b/src/shared/exec-util.h index fcc1ad065fd..d6af5de5101 100644 --- a/src/shared/exec-util.h +++ b/src/shared/exec-util.h @@ -32,6 +32,7 @@ int execute_strv( ExecDirFlags flags); int execute_directories( + const char *name, const char * const *directories, usec_t timeout, gather_stdout_callback_t const callbacks[_STDOUT_CONSUME_MAX], diff --git a/src/shutdown/shutdown.c b/src/shutdown/shutdown.c index 3317068e478..9a114d9be54 100644 --- a/src/shutdown/shutdown.c +++ b/src/shutdown/shutdown.c @@ -575,7 +575,15 @@ int main(int argc, char *argv[]) { arg_verb, NULL, }; - (void) execute_directories(dirs, DEFAULT_TIMEOUT_USEC, NULL, NULL, (char**) arguments, NULL, EXEC_DIR_PARALLEL | EXEC_DIR_IGNORE_ERRORS); + (void) execute_directories( + "system-shutdown", + dirs, + DEFAULT_TIMEOUT_USEC, + /* callbacks= */ NULL, + /* callback_args= */ NULL, + (char**) arguments, + /* envp= */ NULL, + EXEC_DIR_PARALLEL | EXEC_DIR_IGNORE_ERRORS); (void) rlimit_nofile_safe(); diff --git a/src/sleep/sleep.c b/src/sleep/sleep.c index 3390ebe0c08..3bfe4790888 100644 --- a/src/sleep/sleep.c +++ b/src/sleep/sleep.c @@ -311,7 +311,15 @@ static int execute( if (setenv("SYSTEMD_SLEEP_ACTION", action, /* overwrite = */ 1) < 0) log_warning_errno(errno, "Failed to set SYSTEMD_SLEEP_ACTION=%s, ignoring: %m", action); - (void) execute_directories(dirs, DEFAULT_TIMEOUT_USEC, NULL, NULL, (char **) arguments, NULL, EXEC_DIR_PARALLEL | EXEC_DIR_IGNORE_ERRORS); + (void) execute_directories( + "system-sleep", + dirs, + DEFAULT_TIMEOUT_USEC, + /* callbacks= */ NULL, + /* callback_args= */ NULL, + (char **) arguments, + /* envp= */ NULL, + EXEC_DIR_PARALLEL | EXEC_DIR_IGNORE_ERRORS); (void) lock_all_homes(); log_struct(LOG_INFO, @@ -332,8 +340,15 @@ static int execute( LOG_ITEM("SLEEP=%s", sleep_operation_to_string(arg_operation))); arguments[1] = "post"; - (void) execute_directories(dirs, DEFAULT_TIMEOUT_USEC, NULL, NULL, (char **) arguments, NULL, EXEC_DIR_PARALLEL | EXEC_DIR_IGNORE_ERRORS); - + (void) execute_directories( + "system-sleep", + dirs, + DEFAULT_TIMEOUT_USEC, + /* callbacks= */ NULL, + /* callback_args= */ NULL, + (char **) arguments, + /* envp= */ NULL, + EXEC_DIR_PARALLEL | EXEC_DIR_IGNORE_ERRORS); if (r >= 0) return 0; diff --git a/src/test/test-exec-util.c b/src/test/test-exec-util.c index 6accb28b565..e5258de8124 100644 --- a/src/test/test-exec-util.c +++ b/src/test/test-exec-util.c @@ -117,9 +117,9 @@ static void test_execute_directory_one(bool gather_stdout) { return; if (gather_stdout) - execute_directories(dirs, DEFAULT_TIMEOUT_USEC, ignore_stdout, ignore_stdout_args, NULL, NULL, EXEC_DIR_PARALLEL | EXEC_DIR_IGNORE_ERRORS); + execute_directories("test", 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, EXEC_DIR_PARALLEL | EXEC_DIR_IGNORE_ERRORS); + execute_directories("test", dirs, DEFAULT_TIMEOUT_USEC, NULL, NULL, NULL, NULL, EXEC_DIR_PARALLEL | EXEC_DIR_IGNORE_ERRORS); assert_se(chdir(tmp_lo) == 0); assert_se(access("it_works", F_OK) >= 0); @@ -189,7 +189,9 @@ TEST(execution_order) { if (access(name, X_OK) < 0 && ERRNO_IS_PRIVILEGE(errno)) return; - execute_directories(dirs, DEFAULT_TIMEOUT_USEC, ignore_stdout, ignore_stdout_args, NULL, NULL, EXEC_DIR_PARALLEL | EXEC_DIR_IGNORE_ERRORS); + execute_directories(__func__, + 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_STREQ(contents, "30-override\n80-foo\n90-bar\nlast\n"); @@ -270,7 +272,8 @@ TEST(stdout_gathering) { if (access(name, X_OK) < 0 && ERRNO_IS_PRIVILEGE(errno)) return; - r = execute_directories(dirs, DEFAULT_TIMEOUT_USEC, gather_stdouts, args, NULL, NULL, + r = execute_directories(__func__, + dirs, DEFAULT_TIMEOUT_USEC, gather_stdouts, args, NULL, NULL, EXEC_DIR_PARALLEL | EXEC_DIR_IGNORE_ERRORS); assert_se(r >= 0); @@ -337,7 +340,9 @@ TEST(environment_gathering) { if (access(name, X_OK) < 0 && ERRNO_IS_PRIVILEGE(errno)) return; - r = execute_directories(dirs, DEFAULT_TIMEOUT_USEC, gather_environment, args, NULL, NULL, EXEC_DIR_PARALLEL | EXEC_DIR_IGNORE_ERRORS); + r = execute_directories(__func__, + dirs, DEFAULT_TIMEOUT_USEC, gather_environment, args, NULL, NULL, + EXEC_DIR_PARALLEL | EXEC_DIR_IGNORE_ERRORS); assert_se(r >= 0); STRV_FOREACH(p, env) @@ -353,7 +358,9 @@ TEST(environment_gathering) { env = strv_new("PATH=" DEFAULT_PATH_WITHOUT_SBIN); assert_se(env); - r = execute_directories(dirs, DEFAULT_TIMEOUT_USEC, gather_environment, args, NULL, env, EXEC_DIR_PARALLEL | EXEC_DIR_IGNORE_ERRORS); + r = execute_directories(__func__, + dirs, DEFAULT_TIMEOUT_USEC, gather_environment, args, NULL, env, + EXEC_DIR_PARALLEL | EXEC_DIR_IGNORE_ERRORS); assert_se(r >= 0); STRV_FOREACH(p, env) @@ -399,7 +406,8 @@ TEST(error_catching) { if (access(name, X_OK) < 0 && ERRNO_IS_PRIVILEGE(errno)) return; - r = execute_directories(dirs, DEFAULT_TIMEOUT_USEC, + r = execute_directories(__func__, + dirs, DEFAULT_TIMEOUT_USEC, /* callbacks = */ NULL, /* callback_args = */ NULL, /* argv = */ NULL, /* envp = */ NULL, /* flags = */ 0); -- 2.47.3