From 71e529fcf18c00e4dd51fb46f7f289dc5eb11fbd Mon Sep 17 00:00:00 2001 From: Jouke Witteveen Date: Thu, 24 Nov 2016 21:05:47 +0100 Subject: service: only fail notify services on empty cgroup during start We stay in the SERVICE_START while no READY=1 notification message has been received. When we are in the SERVICE_START_POST state, we have already received a ready notification. Hence we should not fail when the cgroup becomes empty in that state. --- src/core/service.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/core/service.c b/src/core/service.c index 180854b57c..39b3589e6b 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -2579,11 +2579,16 @@ static void service_notify_cgroup_empty_event(Unit *u) { * SIGCHLD for. */ case SERVICE_START: - case SERVICE_START_POST: - if (s->type == SERVICE_NOTIFY) + if (s->type == SERVICE_NOTIFY) { /* No chance of getting a ready notification anymore */ service_enter_signal(s, SERVICE_FINAL_SIGTERM, SERVICE_FAILURE_PROTOCOL); - else if (s->pid_file_pathspec) { + break; + } + + /* Fall through */ + + case SERVICE_START_POST: + if (s->pid_file_pathspec) { /* Give up hoping for the daemon to write its PID file */ log_unit_warning(u, "Daemon never wrote its PID file. Failing."); -- cgit v1.2.3-54-g00ecf From 3c9512c71d49d42513755cfa4329275c0360f397 Mon Sep 17 00:00:00 2001 From: Jouke Witteveen Date: Sat, 26 Nov 2016 10:16:47 +0100 Subject: service: prevent registering control pids as the main pid We assume a process can be only one of the two in service_sigchld_event. --- src/core/service.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'src') diff --git a/src/core/service.c b/src/core/service.c index 39b3589e6b..bb67bdf84c 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -3071,6 +3071,8 @@ static void service_notify_message(Unit *u, pid_t pid, char **tags, FDSet *fds) if (e && IN_SET(s->state, SERVICE_START, SERVICE_START_POST, SERVICE_RUNNING, SERVICE_RELOAD)) { if (parse_pid(e, &pid) < 0) log_unit_warning(u, "Failed to parse MAINPID= field in notification message: %s", e); + else if (pid == s->control_pid) + log_unit_warning(u, "A control process cannot also be the main process"); else { service_set_main_pid(s, pid); unit_watch_pid(UNIT(s), pid); -- cgit v1.2.3-54-g00ecf From 7ed0a4c537d405544d1dd772ed08ea20143cf2d6 Mon Sep 17 00:00:00 2001 From: Jouke Witteveen Date: Tue, 29 Nov 2016 09:36:20 +0100 Subject: bus-util: add protocol error type explanation --- man/systemd.exec.xml | 6 +++--- src/shared/bus-unit-util.c | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/man/systemd.exec.xml b/man/systemd.exec.xml index abc275aad0..ab83876eba 100644 --- a/man/systemd.exec.xml +++ b/man/systemd.exec.xml @@ -1772,9 +1772,9 @@ Only defined for the service unit type, this environment variable is passed to all ExecStop= and ExecStopPost= processes, and encodes the service "result". Currently, the following values are defined: protocol (in case of a protocol - violation; if a service did not take the steps required by its configuration), timeout (in - case of an operation timeout), exit-code (if a service process exited with a non-zero exit - code; see $EXIT_CODE below for the actual exit code returned), signal + violation; if a service did not take the steps required by its unit configuration), timeout + (in case of an operation timeout), exit-code (if a service process exited with a non-zero + exit code; see $EXIT_CODE below for the actual exit code returned), signal (if a service process was terminated abnormally by a signal; see $EXIT_CODE below for the actual signal used for the termination), core-dump (if a service process terminated abnormally and dumped core), watchdog (if the watchdog keep-alive ping was enabled for the diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index 388b391342..3114275c85 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -764,6 +764,7 @@ static const struct { const char *result, *explanation; } explanations [] = { { "resources", "of unavailable resources or another system error" }, + { "protocol", "the service did not take the steps required by its unit configuration" }, { "timeout", "a timeout was exceeded" }, { "exit-code", "the control process exited with error code" }, { "signal", "a fatal signal was delivered to the control process" }, -- cgit v1.2.3-54-g00ecf From 6375bd2007412b401e8aa528d70d5866057f6fe5 Mon Sep 17 00:00:00 2001 From: Jouke Witteveen Date: Thu, 24 Nov 2016 10:56:32 +0100 Subject: service: new NotifyAccess= value for control processes (#4212) Setting NotifyAccess=exec allows notifications coming directly from any control process. --- man/systemd.service.xml | 13 ++++++++----- src/core/service.c | 35 +++++++++++++++++++++++++++++++++-- src/core/service.h | 1 + 3 files changed, 42 insertions(+), 7 deletions(-) (limited to 'src') diff --git a/man/systemd.service.xml b/man/systemd.service.xml index 3ba6ab34db..67c68d2f8b 100644 --- a/man/systemd.service.xml +++ b/man/systemd.service.xml @@ -798,11 +798,14 @@ notification socket, as accessible via the sd_notify3 call. Takes one of (the default), - or . If - , no daemon status updates are accepted - from the service processes, all status update messages are - ignored. If , only service updates sent - from the main process of the service are accepted. If + , or + . If , no daemon status + updates are accepted from the service processes, all status + update messages are ignored. If , only + service updates sent from the main process of the service are + accepted. If , only service updates sent + from any of the control processes originating from one of the + Exec*= commands are accepted. If , all services updates from all members of the service's control group are accepted. This option should be set to open access to the notification socket when using diff --git a/src/core/service.c b/src/core/service.c index bb67bdf84c..c68a7122b6 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -1179,6 +1179,25 @@ static int service_collect_fds(Service *s, int **fds, char ***fd_names) { return rn_fds; } +static bool service_exec_needs_notify_socket(Service *s, ExecFlags flags) { + assert(s); + + /* Notifications are accepted depending on the process and + * the access setting of the service: + * process: \ access: NONE MAIN EXEC ALL + * main no yes yes yes + * control no no yes yes + * other (forked) no no no yes */ + + if (flags & EXEC_IS_CONTROL) + /* A control process */ + return IN_SET(s->notify_access, NOTIFY_EXEC, NOTIFY_ALL); + + /* We only spawn main processes and control processes, so any + * process that is not a control process is a main process */ + return s->notify_access != NOTIFY_NONE; +} + static int service_spawn( Service *s, ExecCommand *c, @@ -1252,7 +1271,7 @@ static int service_spawn( if (!our_env) return -ENOMEM; - if ((flags & EXEC_IS_CONTROL) ? s->notify_access == NOTIFY_ALL : s->notify_access != NOTIFY_NONE) + if (service_exec_needs_notify_socket(s, flags)) if (asprintf(our_env + n_env++, "NOTIFY_SOCKET=%s", UNIT(s)->manager->notify_socket) < 0) return -ENOMEM; @@ -3061,7 +3080,18 @@ static void service_notify_message(Unit *u, pid_t pid, char **tags, FDSet *fds) if (s->main_pid != 0) log_unit_warning(u, "Got notification message from PID "PID_FMT", but reception only permitted for main PID "PID_FMT, pid, s->main_pid); else - log_unit_debug(u, "Got notification message from PID "PID_FMT", but reception only permitted for main PID which is currently not known", pid); + log_unit_warning(u, "Got notification message from PID "PID_FMT", but reception only permitted for main PID which is currently not known", pid); + return; + } else if (s->notify_access == NOTIFY_EXEC && pid != s->main_pid && pid != s->control_pid) { + if (s->main_pid != 0 && s->control_pid != 0) + log_unit_warning(u, "Got notification message from PID "PID_FMT", but reception only permitted for main PID "PID_FMT" and control PID "PID_FMT, + pid, s->main_pid, s->control_pid); + else if (s->main_pid != 0) + log_unit_warning(u, "Got notification message from PID "PID_FMT", but reception only permitted for main PID "PID_FMT, pid, s->main_pid); + else if (s->control_pid != 0) + log_unit_warning(u, "Got notification message from PID "PID_FMT", but reception only permitted for control PID "PID_FMT, pid, s->control_pid); + else + log_unit_warning(u, "Got notification message from PID "PID_FMT", but reception only permitted for main PID and control PID which are currently not known", pid); return; } else log_unit_debug(u, "Got notification message from PID "PID_FMT" (%s)", pid, isempty(cc) ? "n/a" : cc); @@ -3388,6 +3418,7 @@ DEFINE_STRING_TABLE_LOOKUP(service_exec_command, ServiceExecCommand); static const char* const notify_access_table[_NOTIFY_ACCESS_MAX] = { [NOTIFY_NONE] = "none", [NOTIFY_MAIN] = "main", + [NOTIFY_EXEC] = "exec", [NOTIFY_ALL] = "all" }; diff --git a/src/core/service.h b/src/core/service.h index 278cc1ceb8..e09722a952 100644 --- a/src/core/service.h +++ b/src/core/service.h @@ -65,6 +65,7 @@ typedef enum NotifyAccess { NOTIFY_NONE, NOTIFY_ALL, NOTIFY_MAIN, + NOTIFY_EXEC, _NOTIFY_ACCESS_MAX, _NOTIFY_ACCESS_INVALID = -1 } NotifyAccess; -- cgit v1.2.3-54-g00ecf