From 0f734bdca580341ac67ddce1076b8f9cb000541a Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Tue, 8 Nov 2016 14:18:33 +0100 Subject: analyze: fix build without seccomp --- src/analyze/analyze.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/analyze/analyze.c b/src/analyze/analyze.c index f744a84501..51d881c5fb 100644 --- a/src/analyze/analyze.c +++ b/src/analyze/analyze.c @@ -36,7 +36,9 @@ #include "log.h" #include "pager.h" #include "parse-util.h" +#ifdef HAVE_SECCOMP #include "seccomp-util.h" +#endif #include "special.h" #include "strv.h" #include "strxcpyx.h" -- cgit v1.2.3-54-g00ecf From 51b9bb4f8e88d420ae557c3ecf1922dd9ac95fcc Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Thu, 10 Nov 2016 10:18:32 +0100 Subject: nfsflags: drop useless include file 'seccomp-util.h' This also fixes the build when seccomp is disabled. --- src/shared/nsflags.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/shared/nsflags.c b/src/shared/nsflags.c index 8fcbe97ba7..49afbede73 100644 --- a/src/shared/nsflags.c +++ b/src/shared/nsflags.c @@ -22,7 +22,6 @@ #include "alloc-util.h" #include "extract-word.h" #include "nsflags.h" -#include "seccomp-util.h" #include "string-util.h" const struct namespace_flag_map namespace_flag_map[] = { -- cgit v1.2.3-54-g00ecf From f80da6f3e9913cec4febcf35536b6b98cfa9d33a Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Wed, 2 Nov 2016 16:24:57 +0100 Subject: core: monitor the inotify file descriptor not the console one in acquire_terminal() When waiting for the terminal to be release in acquire_terminal(), we were monitoring the terminal fd instead of the inotify descriptor. Therefore any write accesses would wake up the waiting process instead of being wake up when the tty is closed only. --- src/basic/terminal-util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/basic/terminal-util.c b/src/basic/terminal-util.c index eafdea9eb3..4b04ccc246 100644 --- a/src/basic/terminal-util.c +++ b/src/basic/terminal-util.c @@ -455,7 +455,7 @@ int acquire_terminal( goto fail; } - r = fd_wait_for_event(fd, POLLIN, ts + timeout - n); + r = fd_wait_for_event(notify, POLLIN, ts + timeout - n); if (r < 0) goto fail; -- cgit v1.2.3-54-g00ecf From 42bf1ae17bad7fafc4ee8e7b6a3052da6decb9f7 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Wed, 2 Nov 2016 10:50:20 +0100 Subject: core: prevent the cylon when confirmation_spawn=yes (#2194) When booting with systemd.confirm_spawn=true, the eye of cylon animation kicks in pretty quickly so user doesn't have any chance to answer the questions which services to start before the confirmation message is screwed by the cylon. This basically breaks the confirm_spawn functionality completely. This patch prevents the cylon animation to kick in when confirmation_spawn=yes. Fixes: #2194 --- src/core/manager.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/core/manager.c b/src/core/manager.c index 52174eac07..d9f772d168 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -111,6 +111,12 @@ static void manager_watch_jobs_in_progress(Manager *m) { assert(m); + /* We do not want to show the cylon animation if the user + * needs to confirm service executions otherwise confirmation + * messages will be screwed by the cylon animation. */ + if (m->confirm_spawn) + return; + if (m->jobs_in_progress_event_source) return; -- cgit v1.2.3-54-g00ecf From 7d5ceb641659b29204598fde9110913765c2aa9e Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Wed, 2 Nov 2016 10:38:22 +0100 Subject: core: allow to redirect confirmation messages to a different console It's rather hard to parse the confirmation messages (enabled with systemd.confirm_spawn=true) amongst the status messages and the kernel ones (if enabled). This patch gives the possibility to the user to redirect the confirmation message to a different virtual console, either by giving its name or its path, so those messages are separated from the other ones and easier to read. --- man/systemd.xml | 12 ++++++++---- src/core/execute.c | 30 +++++++++++++----------------- src/core/execute.h | 17 +++++++++-------- src/core/main.c | 45 ++++++++++++++++++++++++++++++++++----------- src/core/manager.c | 45 ++++++++++++++++++++++++++++++++++++++++++++- src/core/manager.h | 4 +++- src/core/mount.c | 2 +- src/core/service.c | 2 +- src/core/socket.c | 2 +- src/core/swap.c | 2 +- 10 files changed, 115 insertions(+), 46 deletions(-) diff --git a/man/systemd.xml b/man/systemd.xml index 7f24a874ed..fb67ba7cb3 100644 --- a/man/systemd.xml +++ b/man/systemd.xml @@ -940,10 +940,14 @@ systemd.confirm_spawn= - Takes a boolean argument. If - , the system manager (PID 1) asks for - confirmation when spawning processes. Defaults to - . + Takes a boolean argument or a path to the + virtual console where the confirmation messages should be + emitted. If , the system manager (PID 1) + asks for confirmation when spawning processes using + . If a path or a console name + (such as ttyS0) is provided, the virtual + console pointed to by this path or described by the give name + will be used instead. Defaults to . diff --git a/src/core/execute.c b/src/core/execute.c index f666f7c6ce..43a0a5cafd 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -624,7 +624,7 @@ static int chown_terminal(int fd, uid_t uid) { return 0; } -static int setup_confirm_stdio(int *_saved_stdin, int *_saved_stdout) { +static int setup_confirm_stdio(const char *vc, int *_saved_stdin, int *_saved_stdout) { _cleanup_close_ int fd = -1, saved_stdin = -1, saved_stdout = -1; int r; @@ -639,12 +639,7 @@ static int setup_confirm_stdio(int *_saved_stdin, int *_saved_stdout) { if (saved_stdout < 0) return -errno; - fd = acquire_terminal( - "/dev/console", - false, - false, - false, - DEFAULT_CONFIRM_USEC); + fd = acquire_terminal(vc, false, false, false, DEFAULT_CONFIRM_USEC); if (fd < 0) return fd; @@ -674,13 +669,13 @@ static int setup_confirm_stdio(int *_saved_stdin, int *_saved_stdout) { return 0; } -_printf_(1, 2) static int write_confirm_message(const char *format, ...) { +_printf_(2, 3) static int write_confirm_message(const char *vc, const char *format, ...) { _cleanup_close_ int fd = -1; va_list ap; assert(format); - fd = open_terminal("/dev/console", O_WRONLY|O_NOCTTY|O_CLOEXEC); + fd = open_terminal(vc, O_WRONLY|O_NOCTTY|O_CLOEXEC); if (fd < 0) return fd; @@ -713,11 +708,11 @@ static int restore_confirm_stdio(int *saved_stdin, int *saved_stdout) { return r; } -static int ask_for_confirmation(char *response, char **argv) { +static int ask_for_confirmation(const char *vc, char *response, char **argv) { int saved_stdout = -1, saved_stdin = -1, r; _cleanup_free_ char *line = NULL; - r = setup_confirm_stdio(&saved_stdin, &saved_stdout); + r = setup_confirm_stdio(vc, &saved_stdin, &saved_stdout); if (r < 0) return r; @@ -2314,20 +2309,21 @@ static int exec_child( exec_context_tty_reset(context, params); - if (params->flags & EXEC_CONFIRM_SPAWN) { + if (params->confirm_spawn) { + const char *vc = params->confirm_spawn; char response; - r = ask_for_confirmation(&response, argv); + r = ask_for_confirmation(vc, &response, argv); if (r == -ETIMEDOUT) - write_confirm_message("Confirmation question timed out, assuming positive response.\n"); + write_confirm_message(vc, "Confirmation question timed out, assuming positive response.\n"); else if (r < 0) - write_confirm_message("Couldn't ask confirmation question, assuming positive response: %s\n", strerror(-r)); + write_confirm_message(vc, "Couldn't ask confirmation question, assuming positive response: %s\n", strerror(-r)); else if (response == 's') { - write_confirm_message("Skipping execution.\n"); + write_confirm_message(vc, "Skipping execution.\n"); *exit_status = EXIT_CONFIRM; return -ECANCELED; } else if (response == 'n') { - write_confirm_message("Failing execution.\n"); + write_confirm_message(vc, "Failing execution.\n"); *exit_status = 0; return 0; } diff --git a/src/core/execute.h b/src/core/execute.h index 56f880cffe..cd7cbcc5ab 100644 --- a/src/core/execute.h +++ b/src/core/execute.h @@ -226,16 +226,15 @@ static inline bool exec_context_restrict_namespaces_set(const ExecContext *c) { } typedef enum ExecFlags { - EXEC_CONFIRM_SPAWN = 1U << 0, - EXEC_APPLY_PERMISSIONS = 1U << 1, - EXEC_APPLY_CHROOT = 1U << 2, - EXEC_APPLY_TTY_STDIN = 1U << 3, + EXEC_APPLY_PERMISSIONS = 1U << 0, + EXEC_APPLY_CHROOT = 1U << 1, + EXEC_APPLY_TTY_STDIN = 1U << 2, /* The following are not used by execute.c, but by consumers internally */ - EXEC_PASS_FDS = 1U << 4, - EXEC_IS_CONTROL = 1U << 5, - EXEC_SETENV_RESULT = 1U << 6, - EXEC_SET_WATCHDOG = 1U << 7, + EXEC_PASS_FDS = 1U << 3, + EXEC_IS_CONTROL = 1U << 4, + EXEC_SETENV_RESULT = 1U << 5, + EXEC_SET_WATCHDOG = 1U << 6, } ExecFlags; struct ExecParameters { @@ -255,6 +254,8 @@ struct ExecParameters { const char *runtime_prefix; + const char *confirm_spawn; + usec_t watchdog_usec; int *idle_pipe; diff --git a/src/core/main.c b/src/core/main.c index f5f7df838d..5f9b1acad3 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -68,6 +68,7 @@ #include "mount-setup.h" #include "pager.h" #include "parse-util.h" +#include "path-util.h" #include "proc-cmdline.h" #include "process-util.h" #include "raw-clone.h" @@ -104,7 +105,7 @@ static bool arg_dump_core = true; static int arg_crash_chvt = -1; static bool arg_crash_shell = false; static bool arg_crash_reboot = false; -static bool arg_confirm_spawn = false; +static char *arg_confirm_spawn = NULL; static ShowStatus arg_show_status = _SHOW_STATUS_UNSET; static bool arg_switched_root = false; static bool arg_no_pager = false; @@ -294,6 +295,28 @@ static int parse_crash_chvt(const char *value) { return 0; } +static int parse_confirm_spawn(const char *value, char **console) { + char *s; + int r; + + r = value ? parse_boolean(value) : 1; + if (r == 0) { + *console = NULL; + return 0; + } + + if (r > 0) /* on with default tty */ + s = strdup("/dev/console"); + else if (is_path(value)) /* on with fully qualified path */ + s = strdup(value); + else /* on with only a tty file name, not a fully qualified path */ + s = strjoin("/dev/", value); + if (!s) + return -ENOMEM; + *console = s; + return 0; +} + static int set_machine_id(const char *m) { sd_id128_t t; assert(m); @@ -355,11 +378,11 @@ static int parse_proc_cmdline_item(const char *key, const char *value, void *dat } else if (streq(key, "systemd.confirm_spawn") && value) { - r = parse_boolean(value); + arg_confirm_spawn = mfree(arg_confirm_spawn); + + r = parse_confirm_spawn(value, &arg_confirm_spawn); if (r < 0) - log_warning("Failed to parse confirm spawn switch %s. Ignoring.", value); - else - arg_confirm_spawn = r; + log_warning_errno(r, "Failed to parse confirm_spawn switch %s. Ignoring.", value); } else if (streq(key, "systemd.show_status") && value) { @@ -952,12 +975,11 @@ static int parse_argv(int argc, char *argv[]) { break; case ARG_CONFIRM_SPAWN: - r = optarg ? parse_boolean(optarg) : 1; - if (r < 0) { - log_error("Failed to parse confirm spawn boolean %s.", optarg); - return r; - } - arg_confirm_spawn = r; + arg_confirm_spawn = mfree(arg_confirm_spawn); + + r = parse_confirm_spawn(optarg, &arg_confirm_spawn); + if (r < 0) + return log_error_errno(r, "Failed to parse confirm spawn option: %m"); break; case ARG_SHOW_STATUS: @@ -1991,6 +2013,7 @@ finish: arg_default_rlimit[j] = mfree(arg_default_rlimit[j]); arg_default_unit = mfree(arg_default_unit); + arg_confirm_spawn = mfree(arg_confirm_spawn); arg_join_controllers = strv_free_free(arg_join_controllers); arg_default_environment = strv_free(arg_default_environment); arg_syscall_archs = set_free(arg_syscall_archs); diff --git a/src/core/manager.c b/src/core/manager.c index d9f772d168..6ffbbd7389 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -2974,7 +2974,7 @@ void manager_check_finished(Manager *m) { manager_close_idle_pipe(m); /* Turn off confirm spawn now */ - m->confirm_spawn = false; + m->confirm_spawn = NULL; /* No need to update ask password status when we're going non-interactive */ manager_close_ask_password(m); @@ -3159,6 +3159,49 @@ static bool manager_get_show_status(Manager *m, StatusType type) { return false; } +const char *manager_get_confirm_spawn(Manager *m) { + static int last_errno = 0; + const char *vc = m->confirm_spawn; + struct stat st; + int r; + + /* Here's the deal: we want to test the validity of the console but don't want + * PID1 to go through the whole console process which might block. But we also + * want to warn the user only once if something is wrong with the console so we + * cannot do the sanity checks after spawning our children. So here we simply do + * really basic tests to hopefully trap common errors. + * + * If the console suddenly disappear at the time our children will really it + * then they will simply fail to acquire it and a positive answer will be + * assumed. New children will fallback to /dev/console though. + * + * Note: TTYs are devices that can come and go any time, and frequently aren't + * available yet during early boot (consider a USB rs232 dongle...). If for any + * reason the configured console is not ready, we fallback to the default + * console. */ + + if (!vc || path_equal(vc, "/dev/console")) + return vc; + + r = stat(vc, &st); + if (r < 0) + goto fail; + + if (!S_ISCHR(st.st_mode)) { + errno = ENOTTY; + goto fail; + } + + last_errno = 0; + return vc; +fail: + if (last_errno != errno) { + last_errno = errno; + log_warning_errno(errno, "Failed to open %s: %m, using default console", vc); + } + return "/dev/console"; +} + void manager_set_first_boot(Manager *m, bool b) { assert(m); diff --git a/src/core/manager.h b/src/core/manager.h index 35172fdba9..8b3db6e48b 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -246,7 +246,7 @@ struct Manager { uint8_t return_value; ShowStatus show_status; - bool confirm_spawn; + char *confirm_spawn; bool no_console_output; ExecOutput default_std_output, default_std_error; @@ -403,3 +403,5 @@ void manager_deserialize_gid_refs_one(Manager *m, const char *value); const char *manager_state_to_string(ManagerState m) _const_; ManagerState manager_state_from_string(const char *s) _pure_; + +const char *manager_get_confirm_spawn(Manager *m); diff --git a/src/core/mount.c b/src/core/mount.c index 43e0f1c746..1c2be28d55 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -747,7 +747,7 @@ static int mount_spawn(Mount *m, ExecCommand *c, pid_t *_pid) { return r; exec_params.environment = UNIT(m)->manager->environment; - exec_params.flags |= UNIT(m)->manager->confirm_spawn ? EXEC_CONFIRM_SPAWN : 0; + exec_params.confirm_spawn = manager_get_confirm_spawn(UNIT(m)->manager); exec_params.cgroup_supported = UNIT(m)->manager->cgroup_supported; exec_params.cgroup_path = UNIT(m)->cgroup_path; exec_params.cgroup_delegate = m->cgroup_context.delegate; diff --git a/src/core/service.c b/src/core/service.c index 7aa1fba572..9ad4cf5070 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -1335,7 +1335,7 @@ static int service_spawn( exec_params.fds = fds; exec_params.fd_names = fd_names; exec_params.n_fds = n_fds; - exec_params.flags |= UNIT(s)->manager->confirm_spawn ? EXEC_CONFIRM_SPAWN : 0; + exec_params.confirm_spawn = manager_get_confirm_spawn(UNIT(s)->manager); exec_params.cgroup_supported = UNIT(s)->manager->cgroup_supported; exec_params.cgroup_path = path; exec_params.cgroup_delegate = s->cgroup_context.delegate; diff --git a/src/core/socket.c b/src/core/socket.c index ebacd74a47..1a53d47f21 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -1778,7 +1778,7 @@ static int socket_spawn(Socket *s, ExecCommand *c, pid_t *_pid) { exec_params.argv = argv; exec_params.environment = UNIT(s)->manager->environment; - exec_params.flags |= UNIT(s)->manager->confirm_spawn ? EXEC_CONFIRM_SPAWN : 0; + exec_params.confirm_spawn = manager_get_confirm_spawn(UNIT(s)->manager); exec_params.cgroup_supported = UNIT(s)->manager->cgroup_supported; exec_params.cgroup_path = UNIT(s)->cgroup_path; exec_params.cgroup_delegate = s->cgroup_context.delegate; diff --git a/src/core/swap.c b/src/core/swap.c index b870ac88e3..bf404db8c3 100644 --- a/src/core/swap.c +++ b/src/core/swap.c @@ -636,7 +636,7 @@ static int swap_spawn(Swap *s, ExecCommand *c, pid_t *_pid) { goto fail; exec_params.environment = UNIT(s)->manager->environment; - exec_params.flags |= UNIT(s)->manager->confirm_spawn ? EXEC_CONFIRM_SPAWN : 0; + exec_params.confirm_spawn = manager_get_confirm_spawn(UNIT(s)->manager); exec_params.cgroup_supported = UNIT(s)->manager->cgroup_supported; exec_params.cgroup_path = UNIT(s)->cgroup_path; exec_params.cgroup_delegate = s->cgroup_context.delegate; -- cgit v1.2.3-54-g00ecf From 3b20f877ade5599dab124474055d2e56c3dcdb15 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Wed, 2 Nov 2016 13:51:02 +0100 Subject: core: rework ask_for_confirmation() Now the reponses are handled by ask_for_confirmation() as well as the report of any errors occuring during the process of retrieving the confirmation response. One benefit of this is that there's no need to open/close the console one more time when reporting error/status messages. The caller now just needs to care about the return values whose meanings are: - don't execute and pretend that the command failed - don't execute and pretend that the command succeeed - positive answer, execute the command Also some slight code reorganization and introduce write_confirm_error() and write_confirm_error_fd(). write_confim_message becomes unneeded. --- src/core/execute.c | 97 ++++++++++++++++++++++++++++++++++++------------------ 1 file changed, 65 insertions(+), 32 deletions(-) diff --git a/src/core/execute.c b/src/core/execute.c index 43a0a5cafd..8b09f71717 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -669,21 +669,27 @@ static int setup_confirm_stdio(const char *vc, int *_saved_stdin, int *_saved_st return 0; } -_printf_(2, 3) static int write_confirm_message(const char *vc, const char *format, ...) { +static void write_confirm_error_fd(int err, int fd) { + assert(err < 0); + + if (err == -ETIMEDOUT) + dprintf(fd, "Confirmation question timed out, assuming positive response.\n"); + else { + errno = -err; + dprintf(fd, "Couldn't ask confirmation: %m, assuming positive response.\n"); + } +} + +static void write_confirm_error(int err, const char *vc) { _cleanup_close_ int fd = -1; - va_list ap; - assert(format); + assert(vc); fd = open_terminal(vc, O_WRONLY|O_NOCTTY|O_CLOEXEC); if (fd < 0) - return fd; - - va_start(ap, format); - vdprintf(fd, format, ap); - va_end(ap); + return; - return 0; + write_confirm_error_fd(err, fd); } static int restore_confirm_stdio(int *saved_stdin, int *saved_stdout) { @@ -708,22 +714,48 @@ static int restore_confirm_stdio(int *saved_stdin, int *saved_stdout) { return r; } -static int ask_for_confirmation(const char *vc, char *response, char **argv) { +enum { + CONFIRM_PRETEND_FAILURE = -1, + CONFIRM_PRETEND_SUCCESS = 0, + CONFIRM_EXECUTE = 1, +}; + +static int ask_for_confirmation(const char *vc, const char *cmdline) { int saved_stdout = -1, saved_stdin = -1, r; - _cleanup_free_ char *line = NULL; + char c; + /* For any internal errors, assume a positive response. */ r = setup_confirm_stdio(vc, &saved_stdin, &saved_stdout); - if (r < 0) - return r; - - line = exec_command_line(argv); - if (!line) - return -ENOMEM; + if (r < 0) { + write_confirm_error(r, vc); + return CONFIRM_EXECUTE; + } - r = ask_char(response, "yns", "Execute %s? [Yes, No, Skip] ", line); + r = ask_char(&c, "yns", "Execute %s? [Yes, No, Skip] ", cmdline); + if (r < 0) { + write_confirm_error_fd(r, STDOUT_FILENO); + r = CONFIRM_EXECUTE; + goto restore_stdio; + } + + switch (c) { + case 'n': + printf("Failing execution.\n"); + r = CONFIRM_PRETEND_SUCCESS; + break; + case 's': + printf("Skipping execution.\n"); + r = CONFIRM_PRETEND_FAILURE; + break; + case 'y': + r = CONFIRM_EXECUTE; + break; + default: + assert_not_reached("Unhandled choice"); + } +restore_stdio: restore_confirm_stdio(&saved_stdin, &saved_stdout); - return r; } @@ -2311,21 +2343,22 @@ static int exec_child( if (params->confirm_spawn) { const char *vc = params->confirm_spawn; - char response; - - r = ask_for_confirmation(vc, &response, argv); - if (r == -ETIMEDOUT) - write_confirm_message(vc, "Confirmation question timed out, assuming positive response.\n"); - else if (r < 0) - write_confirm_message(vc, "Couldn't ask confirmation question, assuming positive response: %s\n", strerror(-r)); - else if (response == 's') { - write_confirm_message(vc, "Skipping execution.\n"); + _cleanup_free_ char *cmdline = NULL; + + cmdline = exec_command_line(argv); + if (!cmdline) { + *exit_status = EXIT_CONFIRM; + return -ENOMEM; + } + + r = ask_for_confirmation(vc, cmdline); + if (r != CONFIRM_EXECUTE) { + if (r == CONFIRM_PRETEND_SUCCESS) { + *exit_status = EXIT_SUCCESS; + return 0; + } *exit_status = EXIT_CONFIRM; return -ECANCELED; - } else if (response == 'n') { - write_confirm_message(vc, "Failing execution.\n"); - *exit_status = 0; - return 0; } } -- cgit v1.2.3-54-g00ecf From 2bcc330942d526b6004a67c92e284ad842bd5e59 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Sun, 13 Nov 2016 09:32:52 +0100 Subject: core: in confirm_spawn, the meaning of 'n' and 's' choices are confusing Before this patch we had: - "no" which gives "failing execution" but the command is actually assumed as succeed. - "skip" which gives "skipping", but the command is assumed to have failed, which ends up with "Failed to start ..." on the console. Now we have: - "fail" which gives "failing execution" and the command is indeed assumed as failed. - "skip" which gives "skipping execution" and the command is assumed as succeed. --- NEWS | 12 ++++++++++++ src/core/execute.c | 8 ++++---- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/NEWS b/NEWS index b10a6f538f..fbd9afa2cb 100644 --- a/NEWS +++ b/NEWS @@ -1,5 +1,17 @@ systemd System and Service Manager +CHANGES WITH 233 in spe + + * The confirmation spawn prompt has been reworked to offer the + following choices: + + (f)ail, don't execute the command and pretend it failed + (s)kip, don't execute the command and pretend it succeeded + (y)es, execute the command + + The 'n' choice for the confirmation spawn prompt has been removed, + because its meaning was confusing. + CHANGES WITH 232: * The new RemoveIPC= option can be used to remove IPC objects owned by diff --git a/src/core/execute.c b/src/core/execute.c index 8b09f71717..10e9dd7cc8 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -731,7 +731,7 @@ static int ask_for_confirmation(const char *vc, const char *cmdline) { return CONFIRM_EXECUTE; } - r = ask_char(&c, "yns", "Execute %s? [Yes, No, Skip] ", cmdline); + r = ask_char(&c, "yfs", "Execute %s? [Yes, Fail, Skip] ", cmdline); if (r < 0) { write_confirm_error_fd(r, STDOUT_FILENO); r = CONFIRM_EXECUTE; @@ -739,13 +739,13 @@ static int ask_for_confirmation(const char *vc, const char *cmdline) { } switch (c) { - case 'n': + case 'f': printf("Failing execution.\n"); - r = CONFIRM_PRETEND_SUCCESS; + r = CONFIRM_PRETEND_FAILURE; break; case 's': printf("Skipping execution.\n"); - r = CONFIRM_PRETEND_FAILURE; + r = CONFIRM_PRETEND_SUCCESS; break; case 'y': r = CONFIRM_EXECUTE; -- cgit v1.2.3-54-g00ecf From 3c670f8998a34e99f0981622f57350b974448887 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Mon, 7 Nov 2016 17:14:59 +0100 Subject: core: reprint the question every 2 sec in ask_char() ask_char() now reprints the question every 2sec automatically. It prefixes its output with '\r' to to bring the cursor to the beginning of the terminal line, and then print the message, redoing it every 2sec. As long as nothing interferes with out output this logic will have no visible effect as we constantly overprint the visible text with the exact same text. However, if something is dumped in the middle, then our question won't get lost, as we'll ask soon again. This is useful if the question is asked to a terminal that is also used to dump some other status messages/logs. For example when confirmation messages are enabled during the boot (systemd.confirm_spawn=1), the question can easily be lost if the kernel logs are also enabled and both use the same console. Idea suggested by Lennart Poettering. --- src/basic/terminal-util.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/basic/terminal-util.c b/src/basic/terminal-util.c index 4b04ccc246..9a8ef825c5 100644 --- a/src/basic/terminal-util.c +++ b/src/basic/terminal-util.c @@ -144,12 +144,14 @@ int read_one_char(FILE *f, char *ret, usec_t t, bool *need_nl) { return 0; } -int ask_char(char *ret, const char *replies, const char *text, ...) { +#define DEFAULT_ASK_REFRESH_USEC (2*USEC_PER_SEC) + +int ask_char(char *ret, const char *replies, const char *fmt, ...) { int r; assert(ret); assert(replies); - assert(text); + assert(fmt); for (;;) { va_list ap; @@ -159,8 +161,10 @@ int ask_char(char *ret, const char *replies, const char *text, ...) { if (colors_enabled()) fputs(ANSI_HIGHLIGHT, stdout); - va_start(ap, text); - vprintf(text, ap); + putchar('\r'); + + va_start(ap, fmt); + vprintf(fmt, ap); va_end(ap); if (colors_enabled()) @@ -168,9 +172,12 @@ int ask_char(char *ret, const char *replies, const char *text, ...) { fflush(stdout); - r = read_one_char(stdin, &c, USEC_INFINITY, &need_nl); + r = read_one_char(stdin, &c, DEFAULT_ASK_REFRESH_USEC, &need_nl); if (r < 0) { + if (r == -ETIMEDOUT) + continue; + if (r == -EBADMSG) { puts("Bad input, please try again."); continue; -- cgit v1.2.3-54-g00ecf From 2bcd3c26fe2a21cf1541a2850350194b3cdf5e83 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Mon, 7 Nov 2016 17:14:59 +0100 Subject: core: limit the length of the confirmation question MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When "confirmation_spawn=1", the confirmation question can look like: Execute /usr/bin/kmod static-nodes --format=tmpfiles --output=/run/tmpfiles.d/kmod.conf? [Yes, No, Skip] which is pretty verbose and might not fit in the console width size (which is usually 80 chars) and thus question will be splitted into 2 consecutive lines. However since the question is now refreshed every 2 secs, the reprinted question will overwrite the second line of the previous one... To prevent this, this patch makes sure that the command line won't be longer than 60 chars by ellipsizing it if the command is longer: Execute /usr/bin/kmod static-nodes --format=tmpfiles --output=/ru…nf? [Yes, No, View, Skip] A following patch will introduce a new choice that will allow the user to get details on the command to be executed so it will still be possible to see the full command line. --- src/core/execute.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/core/execute.c b/src/core/execute.c index 10e9dd7cc8..0273b1966f 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -722,6 +722,7 @@ enum { static int ask_for_confirmation(const char *vc, const char *cmdline) { int saved_stdout = -1, saved_stdin = -1, r; + _cleanup_free_ char *e = NULL; char c; /* For any internal errors, assume a positive response. */ @@ -731,7 +732,14 @@ static int ask_for_confirmation(const char *vc, const char *cmdline) { return CONFIRM_EXECUTE; } - r = ask_char(&c, "yfs", "Execute %s? [Yes, Fail, Skip] ", cmdline); + e = ellipsize(cmdline, 60, 100); + if (!e) { + log_oom(); + r = CONFIRM_EXECUTE; + goto restore_stdio; + } + + r = ask_char(&c, "yfs", "Execute %s? [Yes, Fail, Skip] ", e); if (r < 0) { write_confirm_error_fd(r, STDOUT_FILENO); r = CONFIRM_EXECUTE; -- cgit v1.2.3-54-g00ecf From d172b175f6d43d68929975e3baa3837da677bc68 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Mon, 7 Nov 2016 17:14:59 +0100 Subject: core: rework the confirmation spawn prompt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously it was "[Yes, Fail, Skip]" which is pretty misleading because it suggests that the whole word needs to be entered instead of a single char. Also this won't fit well when we'll extend the number of choices. This patch addresses this by changing the choice hint with "[y, f, s – h for help]" so it's now clear that a single letter has to be entered. It also introduces a new choice 'h' which describes all possible choices since a single letter can be not descriptive enough for new users. It also allow to stick with the same hint string regardless of how many choices we will support. --- NEWS | 1 + src/core/execute.c | 47 ++++++++++++++++++++++++++++------------------- 2 files changed, 29 insertions(+), 19 deletions(-) diff --git a/NEWS b/NEWS index fbd9afa2cb..fde3d6caf6 100644 --- a/NEWS +++ b/NEWS @@ -6,6 +6,7 @@ CHANGES WITH 233 in spe following choices: (f)ail, don't execute the command and pretend it failed + (h)elp (s)kip, don't execute the command and pretend it succeeded (y)es, execute the command diff --git a/src/core/execute.c b/src/core/execute.c index 0273b1966f..65ba9acf7a 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -739,27 +739,36 @@ static int ask_for_confirmation(const char *vc, const char *cmdline) { goto restore_stdio; } - r = ask_char(&c, "yfs", "Execute %s? [Yes, Fail, Skip] ", e); - if (r < 0) { - write_confirm_error_fd(r, STDOUT_FILENO); - r = CONFIRM_EXECUTE; - goto restore_stdio; - } + for (;;) { + r = ask_char(&c, "yfsh", "Execute %s? [y, f, s – h for help] ", e); + if (r < 0) { + write_confirm_error_fd(r, STDOUT_FILENO); + r = CONFIRM_EXECUTE; + goto restore_stdio; + } - switch (c) { - case 'f': - printf("Failing execution.\n"); - r = CONFIRM_PRETEND_FAILURE; - break; - case 's': - printf("Skipping execution.\n"); - r = CONFIRM_PRETEND_SUCCESS; - break; - case 'y': - r = CONFIRM_EXECUTE; + switch (c) { + case 'f': + printf("Failing execution.\n"); + r = CONFIRM_PRETEND_FAILURE; + break; + case 'h': + printf(" f - fail, don't execute the command and pretend it failed\n" + " h - help\n" + " s - skip, don't execute the command and pretend it succeeded\n" + " y - yes, execute the command\n"); + continue; + case 's': + printf("Skipping execution.\n"); + r = CONFIRM_PRETEND_SUCCESS; + break; + case 'y': + r = CONFIRM_EXECUTE; + break; + default: + assert_not_reached("Unhandled choice"); + } break; - default: - assert_not_reached("Unhandled choice"); } restore_stdio: -- cgit v1.2.3-54-g00ecf From eedf223a30cca99b5d82f620437d2366be0eaf30 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Sat, 12 Nov 2016 14:55:12 +0100 Subject: core: add 'i' in confirm spawn to give a short summary of the unit to spawn --- NEWS | 1 + src/core/execute.c | 13 ++++++++++--- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/NEWS b/NEWS index fde3d6caf6..a145c3ab6e 100644 --- a/NEWS +++ b/NEWS @@ -7,6 +7,7 @@ CHANGES WITH 233 in spe (f)ail, don't execute the command and pretend it failed (h)elp + (i)nfo, show a short summary of the unit (s)kip, don't execute the command and pretend it succeeded (y)es, execute the command diff --git a/src/core/execute.c b/src/core/execute.c index 65ba9acf7a..b48a5732f3 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -720,7 +720,7 @@ enum { CONFIRM_EXECUTE = 1, }; -static int ask_for_confirmation(const char *vc, const char *cmdline) { +static int ask_for_confirmation(const char *vc, Unit *u, const char *cmdline) { int saved_stdout = -1, saved_stdin = -1, r; _cleanup_free_ char *e = NULL; char c; @@ -740,7 +740,7 @@ static int ask_for_confirmation(const char *vc, const char *cmdline) { } for (;;) { - r = ask_char(&c, "yfsh", "Execute %s? [y, f, s – h for help] ", e); + r = ask_char(&c, "yfshi", "Execute %s? [y, f, s – h for help] ", e); if (r < 0) { write_confirm_error_fd(r, STDOUT_FILENO); r = CONFIRM_EXECUTE; @@ -755,9 +755,16 @@ static int ask_for_confirmation(const char *vc, const char *cmdline) { case 'h': printf(" f - fail, don't execute the command and pretend it failed\n" " h - help\n" + " i - info, show a short summary of the unit\n" " s - skip, don't execute the command and pretend it succeeded\n" " y - yes, execute the command\n"); continue; + case 'i': + printf(" Description: %s\n" + " Unit: %s\n" + " Command: %s\n", + u->id, u->description, cmdline); + continue; /* ask again */ case 's': printf("Skipping execution.\n"); r = CONFIRM_PRETEND_SUCCESS; @@ -2368,7 +2375,7 @@ static int exec_child( return -ENOMEM; } - r = ask_for_confirmation(vc, cmdline); + r = ask_for_confirmation(vc, unit, cmdline); if (r != CONFIRM_EXECUTE) { if (r == CONFIRM_PRETEND_SUCCESS) { *exit_status = EXIT_SUCCESS; -- cgit v1.2.3-54-g00ecf From dd6f9ac0d03d073f5dfbf1166f63c6bb64d7ce0c Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Sat, 12 Nov 2016 15:08:29 +0100 Subject: core: add 'D' in confirmat spawn to show a full dump of the unit to spawn --- NEWS | 1 + src/core/execute.c | 10 +++++++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/NEWS b/NEWS index a145c3ab6e..45d3336b1b 100644 --- a/NEWS +++ b/NEWS @@ -5,6 +5,7 @@ CHANGES WITH 233 in spe * The confirmation spawn prompt has been reworked to offer the following choices: + (D)ump, show the state of the unit (f)ail, don't execute the command and pretend it failed (h)elp (i)nfo, show a short summary of the unit diff --git a/src/core/execute.c b/src/core/execute.c index b48a5732f3..6c65bdc361 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -740,7 +740,7 @@ static int ask_for_confirmation(const char *vc, Unit *u, const char *cmdline) { } for (;;) { - r = ask_char(&c, "yfshi", "Execute %s? [y, f, s – h for help] ", e); + r = ask_char(&c, "yfshiD", "Execute %s? [y, f, s – h for help] ", e); if (r < 0) { write_confirm_error_fd(r, STDOUT_FILENO); r = CONFIRM_EXECUTE; @@ -748,17 +748,21 @@ static int ask_for_confirmation(const char *vc, Unit *u, const char *cmdline) { } switch (c) { + case 'D': + unit_dump(u, stdout, " "); + continue; /* ask again */ case 'f': printf("Failing execution.\n"); r = CONFIRM_PRETEND_FAILURE; break; case 'h': - printf(" f - fail, don't execute the command and pretend it failed\n" + printf(" D - dump, show the state of the unit\n" + " f - fail, don't execute the command and pretend it failed\n" " h - help\n" " i - info, show a short summary of the unit\n" " s - skip, don't execute the command and pretend it succeeded\n" " y - yes, execute the command\n"); - continue; + continue; /* ask again */ case 'i': printf(" Description: %s\n" " Unit: %s\n" -- cgit v1.2.3-54-g00ecf From 56fde33af1101bd7d01f1f8b2472d37ab04490c6 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Sun, 13 Nov 2016 16:28:04 +0100 Subject: core: add 'j' in confirmation_spawn to list the jobs that are in progress --- NEWS | 1 + src/core/execute.c | 6 +++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 45d3336b1b..5cb1151b6e 100644 --- a/NEWS +++ b/NEWS @@ -9,6 +9,7 @@ CHANGES WITH 233 in spe (f)ail, don't execute the command and pretend it failed (h)elp (i)nfo, show a short summary of the unit + (j)obs, show jobs that are in progress (s)kip, don't execute the command and pretend it succeeded (y)es, execute the command diff --git a/src/core/execute.c b/src/core/execute.c index 6c65bdc361..6a7ad66a21 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -740,7 +740,7 @@ static int ask_for_confirmation(const char *vc, Unit *u, const char *cmdline) { } for (;;) { - r = ask_char(&c, "yfshiD", "Execute %s? [y, f, s – h for help] ", e); + r = ask_char(&c, "yfshiDj", "Execute %s? [y, f, s – h for help] ", e); if (r < 0) { write_confirm_error_fd(r, STDOUT_FILENO); r = CONFIRM_EXECUTE; @@ -760,6 +760,7 @@ static int ask_for_confirmation(const char *vc, Unit *u, const char *cmdline) { " f - fail, don't execute the command and pretend it failed\n" " h - help\n" " i - info, show a short summary of the unit\n" + " j - jobs, show jobs that are in progress\n" " s - skip, don't execute the command and pretend it succeeded\n" " y - yes, execute the command\n"); continue; /* ask again */ @@ -769,6 +770,9 @@ static int ask_for_confirmation(const char *vc, Unit *u, const char *cmdline) { " Command: %s\n", u->id, u->description, cmdline); continue; /* ask again */ + case 'j': + manager_dump_jobs(u->manager, stdout, " "); + continue; /* ask again */ case 's': printf("Skipping execution.\n"); r = CONFIRM_PRETEND_SUCCESS; -- cgit v1.2.3-54-g00ecf From b0eb29449e63799f8b6b3440cd865d51b90a5423 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Tue, 15 Nov 2016 09:29:04 +0100 Subject: core: add 'c' in confirmation_spawn to resume the boot process --- NEWS | 1 + src/core/execute.c | 18 +++++++++++++++--- src/core/manager.c | 13 ++++++++++++- src/core/manager.h | 2 ++ 4 files changed, 30 insertions(+), 4 deletions(-) diff --git a/NEWS b/NEWS index 5cb1151b6e..7ca129db80 100644 --- a/NEWS +++ b/NEWS @@ -5,6 +5,7 @@ CHANGES WITH 233 in spe * The confirmation spawn prompt has been reworked to offer the following choices: + (c)ontinue, proceed without asking anymore (D)ump, show the state of the unit (f)ail, don't execute the command and pretend it failed (h)elp diff --git a/src/core/execute.c b/src/core/execute.c index 6a7ad66a21..10f73ee9b5 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -732,6 +732,12 @@ static int ask_for_confirmation(const char *vc, Unit *u, const char *cmdline) { return CONFIRM_EXECUTE; } + /* confirm_spawn might have been disabled while we were sleeping. */ + if (manager_is_confirm_spawn_disabled(u->manager)) { + r = 1; + goto restore_stdio; + } + e = ellipsize(cmdline, 60, 100); if (!e) { log_oom(); @@ -740,7 +746,7 @@ static int ask_for_confirmation(const char *vc, Unit *u, const char *cmdline) { } for (;;) { - r = ask_char(&c, "yfshiDj", "Execute %s? [y, f, s – h for help] ", e); + r = ask_char(&c, "yfshiDjc", "Execute %s? [y, f, s – h for help] ", e); if (r < 0) { write_confirm_error_fd(r, STDOUT_FILENO); r = CONFIRM_EXECUTE; @@ -748,6 +754,11 @@ static int ask_for_confirmation(const char *vc, Unit *u, const char *cmdline) { } switch (c) { + case 'c': + printf("Resuming normal execution.\n"); + manager_disable_confirm_spawn(); + r = 1; + break; case 'D': unit_dump(u, stdout, " "); continue; /* ask again */ @@ -756,7 +767,8 @@ static int ask_for_confirmation(const char *vc, Unit *u, const char *cmdline) { r = CONFIRM_PRETEND_FAILURE; break; case 'h': - printf(" D - dump, show the state of the unit\n" + printf(" c - continue, proceed without asking anymore\n" + " D - dump, show the state of the unit\n" " f - fail, don't execute the command and pretend it failed\n" " h - help\n" " i - info, show a short summary of the unit\n" @@ -2373,7 +2385,7 @@ static int exec_child( exec_context_tty_reset(context, params); - if (params->confirm_spawn) { + if (!manager_is_confirm_spawn_disabled(unit->manager)) { const char *vc = params->confirm_spawn; _cleanup_free_ char *cmdline = NULL; diff --git a/src/core/manager.c b/src/core/manager.c index 6ffbbd7389..b49e3b593a 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -114,7 +114,7 @@ static void manager_watch_jobs_in_progress(Manager *m) { /* We do not want to show the cylon animation if the user * needs to confirm service executions otherwise confirmation * messages will be screwed by the cylon animation. */ - if (m->confirm_spawn) + if (!manager_is_confirm_spawn_disabled(m)) return; if (m->jobs_in_progress_event_source) @@ -3218,6 +3218,17 @@ void manager_set_first_boot(Manager *m, bool b) { m->first_boot = b; } +void manager_disable_confirm_spawn(void) { + (void) touch("/run/systemd/confirm_spawn_disabled"); +} + +bool manager_is_confirm_spawn_disabled(Manager *m) { + if (!m->confirm_spawn) + return true; + + return access("/run/systemd/confirm_spawn_disabled", F_OK) >= 0; +} + void manager_status_printf(Manager *m, StatusType type, const char *status, const char *format, ...) { va_list ap; diff --git a/src/core/manager.h b/src/core/manager.h index 8b3db6e48b..4f17f1eea5 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -405,3 +405,5 @@ const char *manager_state_to_string(ManagerState m) _const_; ManagerState manager_state_from_string(const char *s) _pure_; const char *manager_get_confirm_spawn(Manager *m); +bool manager_is_confirm_spawn_disabled(Manager *m); +void manager_disable_confirm_spawn(void); -- cgit v1.2.3-54-g00ecf From 63d77c9254aee1f965df5574c98c2fa83b50567c Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Thu, 10 Nov 2016 10:07:42 +0100 Subject: core: include the unit name when notifying that a confirmation question timed out --- src/core/execute.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/core/execute.c b/src/core/execute.c index 10f73ee9b5..c7b324ffa8 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -669,18 +669,18 @@ static int setup_confirm_stdio(const char *vc, int *_saved_stdin, int *_saved_st return 0; } -static void write_confirm_error_fd(int err, int fd) { +static void write_confirm_error_fd(int err, int fd, const Unit *u) { assert(err < 0); if (err == -ETIMEDOUT) - dprintf(fd, "Confirmation question timed out, assuming positive response.\n"); + dprintf(fd, "Confirmation question timed out for %s, assuming positive response.\n", u->id); else { errno = -err; - dprintf(fd, "Couldn't ask confirmation: %m, assuming positive response.\n"); + dprintf(fd, "Couldn't ask confirmation for %s: %m, assuming positive response.\n", u->id); } } -static void write_confirm_error(int err, const char *vc) { +static void write_confirm_error(int err, const char *vc, const Unit *u) { _cleanup_close_ int fd = -1; assert(vc); @@ -689,7 +689,7 @@ static void write_confirm_error(int err, const char *vc) { if (fd < 0) return; - write_confirm_error_fd(err, fd); + write_confirm_error_fd(err, fd, u); } static int restore_confirm_stdio(int *saved_stdin, int *saved_stdout) { @@ -728,7 +728,7 @@ static int ask_for_confirmation(const char *vc, Unit *u, const char *cmdline) { /* For any internal errors, assume a positive response. */ r = setup_confirm_stdio(vc, &saved_stdin, &saved_stdout); if (r < 0) { - write_confirm_error(r, vc); + write_confirm_error(r, vc, u); return CONFIRM_EXECUTE; } @@ -748,7 +748,7 @@ static int ask_for_confirmation(const char *vc, Unit *u, const char *cmdline) { for (;;) { r = ask_char(&c, "yfshiDjc", "Execute %s? [y, f, s – h for help] ", e); if (r < 0) { - write_confirm_error_fd(r, STDOUT_FILENO); + write_confirm_error_fd(r, STDOUT_FILENO, u); r = CONFIRM_EXECUTE; goto restore_stdio; } -- cgit v1.2.3-54-g00ecf From c891efaf8a37aa13d91835fb8d194f6dd750a78f Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Mon, 14 Nov 2016 17:37:40 +0100 Subject: core: confirm_spawn: always accept units with same_pgrp set for now For some reasons units remaining in the same process group as PID 1 (same_pgrp=true) fail to acquire the console even if it's not taken by anyone. So always accept for units with same_pgrp set for now. --- src/core/execute.c | 2 +- src/core/unit.c | 11 +++++++++++ src/core/unit.h | 2 ++ 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/core/execute.c b/src/core/execute.c index c7b324ffa8..53aed1f287 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -2385,7 +2385,7 @@ static int exec_child( exec_context_tty_reset(context, params); - if (!manager_is_confirm_spawn_disabled(unit->manager)) { + if (unit_shall_confirm_spawn(unit)) { const char *vc = params->confirm_spawn; _cleanup_free_ char *cmdline = NULL; diff --git a/src/core/unit.c b/src/core/unit.c index bba0f5d357..7b78ba9a9e 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -1515,6 +1515,17 @@ int unit_start_limit_test(Unit *u) { return emergency_action(u->manager, u->start_limit_action, u->reboot_arg, "unit failed"); } +bool unit_shall_confirm_spawn(Unit *u) { + + if (manager_is_confirm_spawn_disabled(u->manager)) + return false; + + /* For some reasons units remaining in the same process group + * as PID 1 fail to acquire the console even if it's not used + * by any process. So skip the confirmation question for them. */ + return !unit_get_exec_context(u)->same_pgrp; +} + /* Errors: * -EBADR: This unit type does not support starting. * -EALREADY: Unit is already started. diff --git a/src/core/unit.h b/src/core/unit.h index 991543664b..1ef92f3263 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -654,6 +654,8 @@ void unit_notify_user_lookup(Unit *u, uid_t uid, gid_t gid); int unit_set_invocation_id(Unit *u, sd_id128_t id); int unit_acquire_invocation_id(Unit *u); +bool unit_shall_confirm_spawn(Unit *u); + /* Macros which append UNIT= or USER_UNIT= to the message */ #define log_unit_full(unit, level, error, ...) \ -- cgit v1.2.3-54-g00ecf From 539622bd8c0b425626cab8562c85a5b0e1dda502 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Thu, 17 Nov 2016 18:22:43 +0100 Subject: core: in confirm spawn, suggest 'f' when user selects 'n' choice --- src/core/execute.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/core/execute.c b/src/core/execute.c index 53aed1f287..084eca334c 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -746,7 +746,7 @@ static int ask_for_confirmation(const char *vc, Unit *u, const char *cmdline) { } for (;;) { - r = ask_char(&c, "yfshiDjc", "Execute %s? [y, f, s – h for help] ", e); + r = ask_char(&c, "yfshiDjcn", "Execute %s? [y, f, s – h for help] ", e); if (r < 0) { write_confirm_error_fd(r, STDOUT_FILENO, u); r = CONFIRM_EXECUTE; @@ -785,6 +785,10 @@ static int ask_for_confirmation(const char *vc, Unit *u, const char *cmdline) { case 'j': manager_dump_jobs(u->manager, stdout, " "); continue; /* ask again */ + case 'n': + /* 'n' was removed in favor of 'f'. */ + printf("Didn't understand 'n', did you mean 'f'?\n"); + continue; /* ask again */ case 's': printf("Skipping execution.\n"); r = CONFIRM_PRETEND_SUCCESS; -- cgit v1.2.3-54-g00ecf