From 9711848ff1fe8d74c7d100e4714c4dac30923e74 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 13 Nov 2017 17:44:00 +0100 Subject: [PATCH 01/10] core: only log about sd_notify() message contents, when debug logging is on Let's optimize things a bit for the non-debug case. No change in behaviour. Main reason to do this is not so much the speed benefit though, but merely to isolate the code from its surroundings more. --- src/core/service.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/core/service.c b/src/core/service.c index 672eaa9199..b36da3ad15 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -3312,14 +3312,11 @@ static int service_dispatch_watchdog(sd_event_source *source, usec_t usec, void static void service_notify_message(Unit *u, pid_t pid, char **tags, FDSet *fds) { Service *s = SERVICE(u); - _cleanup_free_ char *cc = NULL; bool notify_dbus = false; const char *e; assert(u); - cc = strv_join(tags, ", "); - if (s->notify_access == NOTIFY_NONE) { log_unit_warning(u, "Got notification message from PID "PID_FMT", but reception is disabled.", pid); return; @@ -3340,8 +3337,14 @@ static void service_notify_message(Unit *u, pid_t pid, char **tags, FDSet *fds) 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 + } + + if (log_get_max_level() >= LOG_DEBUG) { + _cleanup_free_ char *cc = NULL; + + cc = strv_join(tags, ", "); log_unit_debug(u, "Got notification message from PID "PID_FMT" (%s)", pid, isempty(cc) ? "n/a" : cc); + } /* Interpret MAINPID= */ e = strv_find_startswith(tags, "MAINPID="); From e32852377797398baa17b015d7c7496a27731bd2 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 13 Nov 2017 17:48:50 +0100 Subject: [PATCH 02/10] service: split out sd_notify() message authorization code into a function of its own Let's shorten service_notify_message() a bit, and do the authentication outside of the main function body. No functional changes. --- src/core/service.c | 55 +++++++++++++++++++++++++++++----------------- 1 file changed, 35 insertions(+), 20 deletions(-) diff --git a/src/core/service.c b/src/core/service.c index b36da3ad15..add054b785 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -3310,6 +3310,40 @@ static int service_dispatch_watchdog(sd_event_source *source, usec_t usec, void return 0; } +static bool service_notify_message_authorized(Service *s, pid_t pid, char **tags, FDSet *fds) { + assert(s); + + if (s->notify_access == NOTIFY_NONE) { + log_unit_warning(UNIT(s), "Got notification message from PID "PID_FMT", but reception is disabled.", pid); + return false; + } + + if (s->notify_access == NOTIFY_MAIN && pid != s->main_pid) { + if (s->main_pid != 0) + log_unit_warning(UNIT(s), "Got notification message from PID "PID_FMT", but reception only permitted for main PID "PID_FMT, pid, s->main_pid); + else + log_unit_warning(UNIT(s), "Got notification message from PID "PID_FMT", but reception only permitted for main PID which is currently not known", pid); + + return false; + } + + 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(UNIT(s), "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(UNIT(s), "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(UNIT(s), "Got notification message from PID "PID_FMT", but reception only permitted for control PID "PID_FMT, pid, s->control_pid); + else + log_unit_warning(UNIT(s), "Got notification message from PID "PID_FMT", but reception only permitted for main PID and control PID which are currently not known", pid); + + return false; + } + + return true; +} + static void service_notify_message(Unit *u, pid_t pid, char **tags, FDSet *fds) { Service *s = SERVICE(u); bool notify_dbus = false; @@ -3317,27 +3351,8 @@ static void service_notify_message(Unit *u, pid_t pid, char **tags, FDSet *fds) assert(u); - if (s->notify_access == NOTIFY_NONE) { - log_unit_warning(u, "Got notification message from PID "PID_FMT", but reception is disabled.", pid); + if (!service_notify_message_authorized(SERVICE(u), pid, tags, fds)) return; - } else if (s->notify_access == NOTIFY_MAIN && pid != s->main_pid) { - 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_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; - } if (log_get_max_level() >= LOG_DEBUG) { _cleanup_free_ char *cc = NULL; From c45d11cb30419959f2f5537bc4f0fa735e38b7a3 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 13 Nov 2017 18:06:09 +0100 Subject: [PATCH 03/10] service: reorder sd_notify() handling a bit Let's keep handling of WATCHDOG= and WATCHDOG_USEC= together. No functional changes. --- man/sd_notify.xml | 17 ++++++++--------- src/core/service.c | 18 +++++++++--------- src/systemd/sd-daemon.h | 10 +++++----- 3 files changed, 22 insertions(+), 23 deletions(-) diff --git a/man/sd_notify.xml b/man/sd_notify.xml index 078f1ff7e0..4a5e087af7 100644 --- a/man/sd_notify.xml +++ b/man/sd_notify.xml @@ -204,6 +204,14 @@ watchdog is enabled. + + WATCHDOG_USEC=… + + Reset watchdog_usec value during runtime. + Notice that this is not available when using sd_event_set_watchdog() + or sd_watchdog_enabled(). + Example : WATCHDOG_USEC=20000000 + FDSTORE=1 @@ -249,15 +257,6 @@ restrictions, it is ignored. - - WATCHDOG_USEC=… - - Reset watchdog_usec value during runtime. - Notice that this is not available when using sd_event_set_watchdog() - or sd_watchdog_enabled(). - Example : WATCHDOG_USEC=20000000 - - It is recommended to prefix variable names that are not diff --git a/src/core/service.c b/src/core/service.c index add054b785..0beb25b860 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -3457,6 +3457,15 @@ static void service_notify_message(Unit *u, pid_t pid, char **tags, FDSet *fds) if (strv_find(tags, "WATCHDOG=1")) service_reset_watchdog(s); + e = strv_find_startswith(tags, "WATCHDOG_USEC="); + if (e) { + usec_t watchdog_override_usec; + if (safe_atou64(e, &watchdog_override_usec) < 0) + log_unit_warning(u, "Failed to parse WATCHDOG_USEC=%s", e); + else + service_reset_watchdog_timeout(s, watchdog_override_usec); + } + if (strv_find(tags, "FDSTORE=1")) { const char *name; @@ -3469,15 +3478,6 @@ static void service_notify_message(Unit *u, pid_t pid, char **tags, FDSet *fds) service_add_fd_store_set(s, fds, name); } - e = strv_find_startswith(tags, "WATCHDOG_USEC="); - if (e) { - usec_t watchdog_override_usec; - if (safe_atou64(e, &watchdog_override_usec) < 0) - log_unit_warning(u, "Failed to parse WATCHDOG_USEC=%s", e); - else - service_reset_watchdog_timeout(s, watchdog_override_usec); - } - /* Notify clients about changed status or main pid */ if (notify_dbus) unit_add_to_dbus_queue(u); diff --git a/src/systemd/sd-daemon.h b/src/systemd/sd-daemon.h index 61cfdd8e0a..54e82562a7 100644 --- a/src/systemd/sd-daemon.h +++ b/src/systemd/sd-daemon.h @@ -204,17 +204,17 @@ int sd_is_mq(int fd, const char *path); timestamps to detect failed services. Also see sd_watchdog_enabled() below. + WATCHDOG_USEC=... + Reset watchdog_usec value during runtime. + To reset watchdog_usec value, start the service again. + Example: "WATCHDOG_USEC=20000000" + FDSTORE=1 Store the file descriptors passed along with the message in the per-service file descriptor store, and pass them to the main process again on next invocation. This variable is only supported with sd_pid_notify_with_fds(). - WATCHDOG_USEC=... - Reset watchdog_usec value during runtime. - To reset watchdog_usec value, start the service again. - Example: "WATCHDOG_USEC=20000000" - Daemons can choose to send additional variables. However, it is recommended to prefix variable names not listed above with X_. From cc2b7b11b45f82fb881ac788ec2203c632677f90 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 13 Nov 2017 18:14:20 +0100 Subject: [PATCH 04/10] core: only process one of READY=1, STOPPING=1 or RELOADING=1 in sd_notify() handling Of course, it's not really a valid sd_notify() message if multiple of these fields are used in one, but let's handle this somewhat gracefully, by only processing one of them, and ignoring the rest. --- src/core/service.c | 56 ++++++++++++++++++++--------------------- src/systemd/sd-daemon.h | 21 ++++++++++------ 2 files changed, 42 insertions(+), 35 deletions(-) diff --git a/src/core/service.c b/src/core/service.c index 0beb25b860..b25ed49cce 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -3348,6 +3348,7 @@ static void service_notify_message(Unit *u, pid_t pid, char **tags, FDSet *fds) Service *s = SERVICE(u); bool notify_dbus = false; const char *e; + char **i; assert(u); @@ -3377,44 +3378,43 @@ static void service_notify_message(Unit *u, pid_t pid, char **tags, FDSet *fds) } } - /* Interpret RELOADING= */ - if (strv_find(tags, "RELOADING=1")) { + /* Interpret READY=/STOPPING=/RELOADING=. Last one wins. */ + STRV_FOREACH_BACKWARDS(i, tags) { - s->notify_state = NOTIFY_RELOADING; + if (streq(*i, "READY=1")) { + s->notify_state = NOTIFY_READY; - if (s->state == SERVICE_RUNNING) - service_enter_reload_by_notify(s); + /* Type=notify services inform us about completed + * initialization with READY=1 */ + if (s->type == SERVICE_NOTIFY && s->state == SERVICE_START) + service_enter_start_post(s); - notify_dbus = true; - } + /* Sending READY=1 while we are reloading informs us + * that the reloading is complete */ + if (s->state == SERVICE_RELOAD && s->control_pid == 0) + service_enter_running(s, SERVICE_SUCCESS); - /* Interpret READY= */ - if (strv_find(tags, "READY=1")) { + notify_dbus = true; + break; - s->notify_state = NOTIFY_READY; + } else if (streq(*i, "RELOADING=1")) { + s->notify_state = NOTIFY_RELOADING; - /* Type=notify services inform us about completed - * initialization with READY=1 */ - if (s->type == SERVICE_NOTIFY && s->state == SERVICE_START) - service_enter_start_post(s); + if (s->state == SERVICE_RUNNING) + service_enter_reload_by_notify(s); - /* Sending READY=1 while we are reloading informs us - * that the reloading is complete */ - if (s->state == SERVICE_RELOAD && s->control_pid == 0) - service_enter_running(s, SERVICE_SUCCESS); + notify_dbus = true; + break; - notify_dbus = true; - } + } else if (streq(*i, "STOPPING=1")) { + s->notify_state = NOTIFY_STOPPING; - /* Interpret STOPPING= */ - if (strv_find(tags, "STOPPING=1")) { + if (s->state == SERVICE_RUNNING) + service_enter_stop_by_notify(s); - s->notify_state = NOTIFY_STOPPING; - - if (s->state == SERVICE_RUNNING) - service_enter_stop_by_notify(s); - - notify_dbus = true; + notify_dbus = true; + break; + } } /* Interpret STATUS= */ diff --git a/src/systemd/sd-daemon.h b/src/systemd/sd-daemon.h index 54e82562a7..3fd0cc60e3 100644 --- a/src/systemd/sd-daemon.h +++ b/src/systemd/sd-daemon.h @@ -175,12 +175,22 @@ int sd_is_mq(int fd, const char *path); newline separated environment-style variable assignments in a string. The following variables are known: - READY=1 Tells systemd that daemon startup is finished (only - relevant for services of Type=notify). The passed - argument is a boolean "1" or "0". Since there is - little value in signaling non-readiness the only + MAINPID=... The main PID of a daemon, in case systemd did not + fork off the process itself. Example: "MAINPID=4711" + + READY=1 Tells systemd that daemon startup or daemon reload + is finished (only relevant for services of Type=notify). + The passed argument is a boolean "1" or "0". Since there + is little value in signaling non-readiness the only value daemons should send is "READY=1". + RELOADING=1 Tell systemd that the daemon began reloading its + configuration. When the configuration has been + reloaded completely, READY=1 should be sent to inform + systemd about this. + + STOPPING=1 Tells systemd that the daemon is about to go down. + STATUS=... Passes a single-line status string back to systemd that describes the daemon state. This is free-form and can be used for various purposes: general state @@ -195,9 +205,6 @@ int sd_is_mq(int fd, const char *path); BUSERROR=... If a daemon fails, the D-Bus error-style error code. Example: "BUSERROR=org.freedesktop.DBus.Error.TimedOut" - MAINPID=... The main pid of a daemon, in case systemd did not - fork off the process itself. Example: "MAINPID=4711" - WATCHDOG=1 Tells systemd to update the watchdog timestamp. Services using this feature should do this in regular intervals. A watchdog framework can use the From 8d1ab18a466b44a8641899f871fa322e395fdd36 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 13 Nov 2017 18:19:53 +0100 Subject: [PATCH 05/10] man: slightly improve the sd_notify() documentation regarding READY=1 READY=1 may be used to signal when a service finished startup, but also when it finished reloading. Say so. --- man/sd_notify.xml | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/man/sd_notify.xml b/man/sd_notify.xml index 4a5e087af7..3ba242b90b 100644 --- a/man/sd_notify.xml +++ b/man/sd_notify.xml @@ -124,12 +124,10 @@ READY=1 - Tells the service manager that service startup - is finished. This is only used by systemd if the service - definition file has Type=notify set. Since there is little - value in signaling non-readiness, the only value services - should send is READY=1 (i.e. - READY=0 is not defined). + Tells the service manager that service startup is finished, or the service finished loading its + configuration. This is only used by systemd if the service definition file has Type=notify + set. Since there is little value in signaling non-readiness, the only value services should send is + READY=1 (i.e. READY=0 is not defined). From e78ee06de1011aaa8de7a60d98fc18daee2edb26 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 13 Nov 2017 18:26:04 +0100 Subject: [PATCH 06/10] core: add a new sd_notify() message for removing fds from the FD store again Currenly the only way to remove fds from the fdstore is to fully stop the service, or to somehow trigger POLLERR/POLLHUP on the fd, in which case systemd will remove the fd automatically. Let's add another way: a new message that can be sent to remove fds explicitly, given their name. --- man/sd_notify.xml | 31 ++++++++++++++++--------------- src/core/service.c | 31 +++++++++++++++++++++++++++++-- src/systemd/sd-daemon.h | 9 +++++++++ 3 files changed, 54 insertions(+), 17 deletions(-) diff --git a/man/sd_notify.xml b/man/sd_notify.xml index 3ba242b90b..a53b1ab33e 100644 --- a/man/sd_notify.xml +++ b/man/sd_notify.xml @@ -234,25 +234,26 @@ to send messages with FDSTORE=1, see below. + + FDSTOREREMOVE=1 + + Removes file descriptors from the file descriptor store. This field needs to be combined with + FDNAME= to specify the name of the file descriptors to remove. + + FDNAME=… - When used in combination with - FDSTORE=1, specifies a name for the - submitted file descriptors. This name is passed to the service - during activation, and may be queried using + When used in combination with FDSTORE=1, specifies a name for the submitted + file descriptors. When used with FDSTOREREMOVE=1, specifies the name for the file + descriptors to remove. This name is passed to the service during activation, and may be queried using sd_listen_fds_with_names3. File - descriptors submitted without this field set, will implicitly - get the name stored assigned. Note that, if - multiple file descriptors are submitted at once, the specified - name will be assigned to all of them. In order to assign - different names to submitted file descriptors, submit them in - separate invocations of - sd_pid_notify_with_fds(). The name may - consist of any ASCII character, but must not contain control - characters or :. It may not be longer than - 255 characters. If a submitted name does not follow these - restrictions, it is ignored. + descriptors submitted without this field set, will implicitly get the name stored + assigned. Note that, if multiple file descriptors are submitted at once, the specified name will be assigned to + all of them. In order to assign different names to submitted file descriptors, submit them in separate + invocations of sd_pid_notify_with_fds(). The name may consist of arbitrary ASCII + characters except control characters or :. It may not be longer than 255 characters. If a + submitted name does not follow these restrictions, it is ignored. diff --git a/src/core/service.c b/src/core/service.c index b25ed49cce..bd187a2753 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -458,6 +458,21 @@ static int service_add_fd_store_set(Service *s, FDSet *fds, const char *name) { return 0; } +static void service_remove_fd_store(Service *s, const char *name) { + ServiceFDStore *fs, *n; + + assert(s); + assert(name); + + LIST_FOREACH_SAFE(fd_store, fs, n, s->fd_store) { + if (!streq(fs->fdname, name)) + continue; + + log_unit_debug(UNIT(s), "Got explicit request to remove fd %i (%s), closing.", fs->fd, name); + service_fd_store_unlink(fs); + } +} + static int service_arm_timer(Service *s, usec_t usec) { int r; @@ -3466,7 +3481,19 @@ static void service_notify_message(Unit *u, pid_t pid, char **tags, FDSet *fds) service_reset_watchdog_timeout(s, watchdog_override_usec); } - if (strv_find(tags, "FDSTORE=1")) { + /* Process FD store messages. Either FDSTOREREMOVE=1 for removal, or FDSTORE=1 for addition. In both cases, + * process FDNAME= for picking the file descriptor name to use. Note that FDNAME= is required when removing + * fds, but optional when pushing in new fds, for compatibility reasons. */ + if (strv_find(tags, "FDSTOREREMOVE=1")) { + const char *name; + + name = strv_find_startswith(tags, "FDNAME="); + if (!name || !fdname_is_valid(name)) + log_unit_warning(u, "FDSTOREREMOVE=1 requested, but no valid file descriptor name passed, ignoring."); + else + service_remove_fd_store(s, name); + + } else if (strv_find(tags, "FDSTORE=1")) { const char *name; name = strv_find_startswith(tags, "FDNAME="); @@ -3475,7 +3502,7 @@ static void service_notify_message(Unit *u, pid_t pid, char **tags, FDSet *fds) name = NULL; } - service_add_fd_store_set(s, fds, name); + (void) service_add_fd_store_set(s, fds, name); } /* Notify clients about changed status or main pid */ diff --git a/src/systemd/sd-daemon.h b/src/systemd/sd-daemon.h index 3fd0cc60e3..9772a05dad 100644 --- a/src/systemd/sd-daemon.h +++ b/src/systemd/sd-daemon.h @@ -222,6 +222,15 @@ int sd_is_mq(int fd, const char *path); invocation. This variable is only supported with sd_pid_notify_with_fds(). + FDSTOREREMOVE=1 + Remove one or more file descriptors from the file + descriptor store, identified by the name specified + in FDNAME=, see below. + + FDNAME= A name to assign to new file descriptors stored in the + file descriptor store, or the name of the file descriptors + to remove in case of FDSTOREREMOVE=1. + Daemons can choose to send additional variables. However, it is recommended to prefix variable names not listed above with X_. From 4050e4797603d3644707d58edfd9742b5311c7cf Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 13 Nov 2017 19:06:11 +0100 Subject: [PATCH 07/10] logind: use the new FDSTOREREMOVE=1 sd_notify() message Let's explicitly tell PID 1 that we don't need an fd anymore, instead of relying exclusively on POLLERR/POLLHUP for it to be removed. Fixes: #6908 --- src/login/logind-session-device.c | 43 +++++++++++++++++++++---------- src/login/logind-session-device.h | 1 + 2 files changed, 30 insertions(+), 14 deletions(-) diff --git a/src/login/logind-session-device.c b/src/login/logind-session-device.c index b55ce60be1..067e67a93d 100644 --- a/src/login/logind-session-device.c +++ b/src/login/logind-session-device.c @@ -410,6 +410,17 @@ error: void session_device_free(SessionDevice *sd) { assert(sd); + if (sd->pushed_fd) { + const char *m; + + /* Remove the pushed fd again, just in case. */ + + m = strjoina("FDSTOREREMOVE=1\n" + "FDNAME=session-", sd->session->id); + + (void) sd_notify(false, m); + } + session_device_stop(sd); session_device_notify(sd, SESSION_DEVICE_RELEASE); close_nointr(sd->fd); @@ -489,26 +500,30 @@ unsigned int session_device_try_pause_all(Session *s) { } int session_device_save(SessionDevice *sd) { - _cleanup_free_ char *state = NULL; + const char *m; int r; assert(sd); - /* Store device fd in PID1. It will send it back to us on - * restart so revocation will continue to work. To make things - * simple, send fds for all type of devices even if they don't - * support the revocation mechanism so we don't have to handle - * them differently later. + /* Store device fd in PID1. It will send it back to us on restart so revocation will continue to work. To make + * things simple, send fds for all type of devices even if they don't support the revocation mechanism so we + * don't have to handle them differently later. * - * Note: for device supporting revocation, PID1 will drop a - * stored fd automatically if the corresponding device is - * revoked. */ - r = asprintf(&state, "FDSTORE=1\n" - "FDNAME=session-%s", sd->session->id); - if (r < 0) - return -ENOMEM; + * Note: for device supporting revocation, PID1 will drop a stored fd automatically if the corresponding device + * is revoked. */ - return sd_pid_notify_with_fds(0, false, state, &sd->fd, 1); + if (sd->pushed_fd) + return 0; + + m = strjoina("FDSTORE=1\n" + "FDNAME=session", sd->session->id); + + r = sd_pid_notify_with_fds(0, false, m, &sd->fd, 1); + if (r < 0) + return r; + + sd->pushed_fd = true; + return 1; } void session_device_attach_fd(SessionDevice *sd, int fd, bool active) { diff --git a/src/login/logind-session-device.h b/src/login/logind-session-device.h index c263e16750..a1cf17af92 100644 --- a/src/login/logind-session-device.h +++ b/src/login/logind-session-device.h @@ -41,6 +41,7 @@ struct SessionDevice { int fd; bool active; DeviceType type; + bool pushed_fd; LIST_FIELDS(struct SessionDevice, sd_by_device); }; From 67ca6412ab3632daecd57d99cddb92aa9f7b77cd Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 13 Nov 2017 19:16:56 +0100 Subject: [PATCH 08/10] update TODO --- TODO | 2 -- 1 file changed, 2 deletions(-) diff --git a/TODO b/TODO index 424a75df1a..f7ed2627ba 100644 --- a/TODO +++ b/TODO @@ -58,8 +58,6 @@ Features: * rework ExecOutput and ExecInput enums so that EXEC_OUTPUT_NULL loses its magic meaning and is no longer upgraded to something else if set explicitly. -* add a way to remove fds from the fdstore by name, and make logind use it - * in the long run: permit a system with /etc/machine-id linked to /dev/null, to make it lose its identity, i.e. be anonymous. For this we'd have to patch through the whole tree to make all code deal with the case where no machine From 2fa40742a46de176c4424799b0d298cc54b0b9de Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 27 Nov 2017 16:53:03 +0100 Subject: [PATCH 09/10] service: use parse_errno() for parsing error numbers Let's always use the same logic when parsing error numbers, i.e. use parse_errno() here too, to unify some code, and tighten the checks a bit. This also allows clients to pass errors as symbolic names. Probably nothing we want to advertise too eagerly (since new daemons generating this on old service managers won't understand), but still pretty useful I think, in particular in scripting languages and such, where the numeric error numbers might not be readily available. --- src/core/service.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/core/service.c b/src/core/service.c index bd187a2753..e7e65d4270 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -3458,13 +3458,13 @@ static void service_notify_message(Unit *u, pid_t pid, char **tags, FDSet *fds) if (e) { int status_errno; - if (safe_atoi(e, &status_errno) < 0 || status_errno < 0) - log_unit_warning(u, "Failed to parse ERRNO= field in notification message: %s", e); - else { - if (s->status_errno != status_errno) { - s->status_errno = status_errno; - notify_dbus = true; - } + status_errno = parse_errno(e); + if (status_errno < 0) + log_unit_warning_errno(u, status_errno, + "Failed to parse ERRNO= field in notification message: %s", e); + else if (s->status_errno != status_errno) { + s->status_errno = status_errno; + notify_dbus = true; } } From f3b900311f1f6ed10adfc8e86267a2fbc4e1fb7b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 27 Nov 2017 16:58:46 +0100 Subject: [PATCH 10/10] service: shortcut operations if the MAINPID= doesn't actually cause a change --- src/core/service.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/service.c b/src/core/service.c index e7e65d4270..e7a829a564 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -3386,7 +3386,7 @@ static void service_notify_message(Unit *u, pid_t pid, char **tags, FDSet *fds) log_unit_warning(u, "A control process cannot also be the main process"); else if (pid == getpid_cached() || pid == 1) log_unit_warning(u, "Service manager can't be main process, ignoring sd_notify() MAINPID= field"); - else { + else if (pid != s->main_pid) { service_set_main_pid(s, pid); unit_watch_pid(UNIT(s), pid); notify_dbus = true;