1
1
mirror of https://github.com/systemd/systemd-stable.git synced 2025-01-03 01:17:45 +03:00

tree-wide: warn when sd_notify fails with READY=1 or FDSTOREREMOVE=1

Most sd_notify() calls are like log_info() — the result is only informative
and if they fail, it's best ignore this. But if a call with READY=1 fails,
the unit may enter a failed state, so we should warn about this. Similarly
for FSTOREREMOVE=1: the manager may be left with a stale fd, at least wasting
resources.
This commit is contained in:
Zbigniew Jędrzejewski-Szmek 2021-11-03 11:04:46 +01:00
parent 9214f2999b
commit 4bf4f50faa
8 changed files with 71 additions and 46 deletions

View File

@ -3457,27 +3457,38 @@ static void manager_notify_finished(Manager *m) {
} }
static void user_manager_send_ready(Manager *m) { static void user_manager_send_ready(Manager *m) {
int r;
assert(m); assert(m);
/* We send READY=1 on reaching basic.target only when running in --user mode. */ /* We send READY=1 on reaching basic.target only when running in --user mode. */
if (!MANAGER_IS_USER(m) || m->ready_sent) if (!MANAGER_IS_USER(m) || m->ready_sent)
return; return;
sd_notifyf(false, r = sd_notifyf(false,
"READY=1\n" "READY=1\n"
"STATUS=Reached " SPECIAL_BASIC_TARGET "."); "STATUS=Reached " SPECIAL_BASIC_TARGET ".");
if (r < 0)
log_warning_errno(r, "Failed to send readiness notification, ignoring: %m");
m->ready_sent = true; m->ready_sent = true;
m->status_ready = false; m->status_ready = false;
} }
static void manager_send_ready(Manager *m) { static void manager_send_ready(Manager *m) {
int r;
if (m->ready_sent && m->status_ready) if (m->ready_sent && m->status_ready)
/* Skip the notification if nothing changed. */ /* Skip the notification if nothing changed. */
return; return;
sd_notifyf(false, r = sd_notifyf(false,
"%sSTATUS=Ready.", "%sSTATUS=Ready.",
m->ready_sent ? "READY=1\n" : ""); m->ready_sent ? "READY=1\n" : "");
if (r < 0)
log_full_errno(m->ready_sent ? LOG_DEBUG : LOG_WARNING, r,
"Failed to send readiness notification, ignoring: %m");
m->ready_sent = m->status_ready = true; m->ready_sent = m->status_ready = true;
} }

View File

@ -384,14 +384,19 @@ error:
} }
void session_device_free(SessionDevice *sd) { void session_device_free(SessionDevice *sd) {
int r;
assert(sd); assert(sd);
/* Make sure to remove the pushed fd. */ /* Make sure to remove the pushed fd. */
if (sd->pushed_fd) if (sd->pushed_fd) {
(void) sd_notifyf(false, r = sd_notifyf(false,
"FDSTOREREMOVE=1\n" "FDSTOREREMOVE=1\n"
"FDNAME=session-%s-device-%u-%u", "FDNAME=session-%s-device-%u-%u",
sd->session->id, major(sd->dev), minor(sd->dev)); sd->session->id, major(sd->dev), minor(sd->dev));
if (r < 0)
log_warning_errno(r, "Failed to remove file descriptor from the store, ignoring: %m");
}
session_device_stop(sd); session_device_stop(sd);
session_device_notify(sd, SESSION_DEVICE_RELEASE); session_device_notify(sd, SESSION_DEVICE_RELEASE);

View File

@ -440,7 +440,7 @@ static int deliver_fd(Manager *m, const char *fdname, int fd) {
static int manager_attach_fds(Manager *m) { static int manager_attach_fds(Manager *m) {
_cleanup_strv_free_ char **fdnames = NULL; _cleanup_strv_free_ char **fdnames = NULL;
int n; int r, n;
/* Upon restart, PID1 will send us back all fds of session devices that we previously opened. Each /* Upon restart, PID1 will send us back all fds of session devices that we previously opened. Each
* file descriptor is associated with a given session. The session ids are passed through FDNAMES. */ * file descriptor is associated with a given session. The session ids are passed through FDNAMES. */
@ -461,9 +461,11 @@ static int manager_attach_fds(Manager *m) {
safe_close(fd); safe_close(fd);
/* Remove from fdstore as well */ /* Remove from fdstore as well */
(void) sd_notifyf(false, r = sd_notifyf(false,
"FDSTOREREMOVE=1\n" "FDSTOREREMOVE=1\n"
"FDNAME=%s", fdnames[i]); "FDNAME=%s", fdnames[i]);
if (r < 0)
log_warning_errno(r, "Failed to remove file descriptor from the store, ignoring: %m");
} }
return 0; return 0;

View File

@ -6,14 +6,13 @@
#include <sys/types.h> #include <sys/types.h>
#include <unistd.h> #include <unistd.h>
#include "sd-daemon.h"
#include "alloc-util.h" #include "alloc-util.h"
#include "bus-error.h" #include "bus-error.h"
#include "bus-locator.h" #include "bus-locator.h"
#include "bus-log-control-api.h" #include "bus-log-control-api.h"
#include "bus-polkit.h" #include "bus-polkit.h"
#include "cgroup-util.h" #include "cgroup-util.h"
#include "daemon-util.h"
#include "dirent-util.h" #include "dirent-util.h"
#include "discover-image.h" #include "discover-image.h"
#include "fd-util.h" #include "fd-util.h"
@ -352,17 +351,14 @@ static int run(int argc, char *argv[]) {
return log_error_errno(r, "Failed to fully start up daemon: %m"); return log_error_errno(r, "Failed to fully start up daemon: %m");
log_debug("systemd-machined running as pid "PID_FMT, getpid_cached()); log_debug("systemd-machined running as pid "PID_FMT, getpid_cached());
(void) sd_notify(false, r = sd_notify(false, NOTIFY_READY);
"READY=1\n" if (r < 0)
"STATUS=Processing requests..."); log_warning_errno(r, "Failed to send readiness notification, ignoring: %m");
r = manager_run(m); r = manager_run(m);
(void) sd_notify(false, NOTIFY_STOPPING);
log_debug("systemd-machined stopped as pid "PID_FMT, getpid_cached()); log_debug("systemd-machined stopped as pid "PID_FMT, getpid_cached());
(void) sd_notify(false,
"STOPPING=1\n"
"STATUS=Shutting down...");
return r; return r;
} }

View File

@ -4205,6 +4205,7 @@ static int nspawn_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t r
ssize_t n; ssize_t n;
pid_t inner_child_pid; pid_t inner_child_pid;
_cleanup_strv_free_ char **tags = NULL; _cleanup_strv_free_ char **tags = NULL;
int r;
assert(userdata); assert(userdata);
@ -4243,8 +4244,11 @@ static int nspawn_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t r
if (!tags) if (!tags)
return log_oom(); return log_oom();
if (strv_find(tags, "READY=1")) if (strv_find(tags, "READY=1")) {
(void) sd_notifyf(false, "READY=1\n"); r = sd_notifyf(false, "READY=1\n");
if (r < 0)
log_warning_errno(r, "Failed to send readiness notification, ignoring: %m");
}
p = strv_find_startswith(tags, "STATUS="); p = strv_find_startswith(tags, "STATUS=");
if (p) if (p)
@ -5134,8 +5138,11 @@ static int run_container(
(void) sd_notifyf(false, (void) sd_notifyf(false,
"STATUS=Container running.\n" "STATUS=Container running.\n"
"X_NSPAWN_LEADER_PID=" PID_FMT, *pid); "X_NSPAWN_LEADER_PID=" PID_FMT, *pid);
if (!arg_notify_ready) if (!arg_notify_ready) {
(void) sd_notify(false, "READY=1\n"); r = sd_notify(false, "READY=1\n");
if (r < 0)
log_warning_errno(r, "Failed to send readiness notification, ignoring: %m");
}
if (arg_kill_signal > 0) { if (arg_kill_signal > 0) {
/* Try to kill the init system on SIGINT or SIGTERM */ /* Try to kill the init system on SIGINT or SIGTERM */

View File

@ -4,11 +4,11 @@
#include <sys/types.h> #include <sys/types.h>
#include "sd-bus.h" #include "sd-bus.h"
#include "sd-daemon.h"
#include "alloc-util.h" #include "alloc-util.h"
#include "bus-log-control-api.h" #include "bus-log-control-api.h"
#include "bus-polkit.h" #include "bus-polkit.h"
#include "daemon-util.h"
#include "def.h" #include "def.h"
#include "main-func.h" #include "main-func.h"
#include "portabled-bus.h" #include "portabled-bus.h"
@ -154,15 +154,13 @@ static int run(int argc, char *argv[]) {
return log_error_errno(r, "Failed to fully start up daemon: %m"); return log_error_errno(r, "Failed to fully start up daemon: %m");
log_debug("systemd-portabled running as pid " PID_FMT, getpid_cached()); log_debug("systemd-portabled running as pid " PID_FMT, getpid_cached());
sd_notify(false, r = sd_notify(false, NOTIFY_READY);
"READY=1\n" if (r < 0)
"STATUS=Processing requests..."); log_warning_errno(r, "Failed to send readiness notification, ignoring: %m");
r = manager_run(m); r = manager_run(m);
(void) sd_notify(false, (void) sd_notify(false, NOTIFY_STOPPING);
"STOPPING=1\n"
"STATUS=Shutting down...");
log_debug("systemd-portabled stopped as pid " PID_FMT, getpid_cached()); log_debug("systemd-portabled stopped as pid " PID_FMT, getpid_cached());
return r; return r;
} }

View File

@ -317,7 +317,10 @@ static int run(int argc, char *argv[]) {
if (!ready) { if (!ready) {
/* Notify manager that we are now finished with processing whatever was /* Notify manager that we are now finished with processing whatever was
* queued */ * queued */
(void) sd_notify(false, "READY=1"); r = sd_notify(false, "READY=1");
if (r < 0)
log_warning_errno(r, "Failed to send readiness notification, ignoring: %m");
ready = true; ready = true;
} }

View File

@ -315,9 +315,18 @@ static void manager_exit(Manager *manager) {
manager_kill_workers(manager, true); manager_kill_workers(manager, true);
} }
static void notify_ready(void) {
int r;
r = sd_notifyf(false,
"READY=1\n"
"STATUS=Processing with %u children at max", arg_children_max);
if (r < 0)
log_warning_errno(r, "Failed to send readiness notification, ignoring: %m");
}
/* reload requested, HUP signal received, rules changed, builtin changed */ /* reload requested, HUP signal received, rules changed, builtin changed */
static void manager_reload(Manager *manager) { static void manager_reload(Manager *manager) {
assert(manager); assert(manager);
sd_notify(false, sd_notify(false,
@ -328,9 +337,7 @@ static void manager_reload(Manager *manager) {
manager->rules = udev_rules_free(manager->rules); manager->rules = udev_rules_free(manager->rules);
udev_builtin_exit(); udev_builtin_exit();
sd_notifyf(false, notify_ready();
"READY=1\n"
"STATUS=Processing with %u children at max", arg_children_max);
} }
static int on_kill_workers_event(sd_event_source *s, uint64_t usec, void *userdata) { static int on_kill_workers_event(sd_event_source *s, uint64_t usec, void *userdata) {
@ -1199,9 +1206,7 @@ static int on_ctrl_msg(UdevCtrl *uctrl, UdevCtrlMessageType type, const UdevCtrl
log_debug("Received udev control message (SET_MAX_CHILDREN), setting children_max=%i", value->intval); log_debug("Received udev control message (SET_MAX_CHILDREN), setting children_max=%i", value->intval);
arg_children_max = value->intval; arg_children_max = value->intval;
(void) sd_notifyf(false, notify_ready();
"READY=1\n"
"STATUS=Processing with %u children at max", arg_children_max);
break; break;
case UDEV_CTRL_PING: case UDEV_CTRL_PING:
log_debug("Received udev control message (PING)"); log_debug("Received udev control message (PING)");
@ -1862,9 +1867,7 @@ static int main_loop(Manager *manager) {
if (r < 0) if (r < 0)
log_error_errno(r, "Failed to apply permissions on static device nodes: %m"); log_error_errno(r, "Failed to apply permissions on static device nodes: %m");
(void) sd_notifyf(false, notify_ready();
"READY=1\n"
"STATUS=Processing with %u children at max", arg_children_max);
r = sd_event_loop(manager->event); r = sd_event_loop(manager->event);
if (r < 0) if (r < 0)