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.

diff --git a/src/core/manager.c b/src/core/manager.c
index 167fa1a34aa61a3771513ebf034cc98ae7cde888..d48e0b08783745df2bd169cc805c2146f2bb8d55 100644 (file)
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -3457,27 +3457,38 @@ static void manager_notify_finished(Manager *m) {
 }
 
 static void user_manager_send_ready(Manager *m) {
+        int r;
+
         assert(m);
 
         /* We send READY=1 on reaching basic.target only when running in --user mode. */
         if (!MANAGER_IS_USER(m) || m->ready_sent)
                 return;
 
-        sd_notifyf(false,
-                   "READY=1\n"
-                   "STATUS=Reached " SPECIAL_BASIC_TARGET ".");
+        r = sd_notifyf(false,
+                       "READY=1\n"
+                       "STATUS=Reached " SPECIAL_BASIC_TARGET ".");
+        if (r < 0)
+                log_warning_errno(r, "Failed to send readiness notification, ignoring: %m");
+
         m->ready_sent = true;
         m->status_ready = false;
 }
 
 static void manager_send_ready(Manager *m) {
+        int r;
+
         if (m->ready_sent && m->status_ready)
                 /* Skip the notification if nothing changed. */
                 return;
 
-        sd_notifyf(false,
-                   "%sSTATUS=Ready.",
-                   m->ready_sent ? "READY=1\n" : "");
+        r = sd_notifyf(false,
+                       "%sSTATUS=Ready.",
+                       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;
 }
 

diff --git a/src/login/logind-session-device.c b/src/login/logind-session-device.c
index 1c4d543889edba318605562954d0fa2837d500e8..09e19a3509db27293415d38a7b0ebcb25b4ca5e3 100644 (file)
--- a/src/login/logind-session-device.c
+++ b/src/login/logind-session-device.c
@@ -384,14 +384,19 @@ error:
 }
 
 void session_device_free(SessionDevice *sd) {
+        int r;
+
         assert(sd);
 
         /* Make sure to remove the pushed fd. */
-        if (sd->pushed_fd)
-                (void) sd_notifyf(false,
-                                  "FDSTOREREMOVE=1\n"
-                                  "FDNAME=session-%s-device-%u-%u",
-                                  sd->session->id, major(sd->dev), minor(sd->dev));
+        if (sd->pushed_fd) {
+                r = sd_notifyf(false,
+                               "FDSTOREREMOVE=1\n"
+                               "FDNAME=session-%s-device-%u-%u",
+                               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_notify(sd, SESSION_DEVICE_RELEASE);

diff --git a/src/login/logind.c b/src/login/logind.c
index c128d64ed47afd6a43e336d8ffe0d6872587047c..6e1ebbf9c576b8858d27ba806666511f4905bee6 100644 (file)
--- a/src/login/logind.c
+++ b/src/login/logind.c
@@ -440,7 +440,7 @@ static int deliver_fd(Manager *m, const char *fdname, int fd) {
 
 static int manager_attach_fds(Manager *m) {
         _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
          * 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);
 
                 /* Remove from fdstore as well */
-                (void) sd_notifyf(false,
-                                  "FDSTOREREMOVE=1\n"
-                                  "FDNAME=%s", fdnames[i]);
+                r = sd_notifyf(false,
+                               "FDSTOREREMOVE=1\n"
+                               "FDNAME=%s", fdnames[i]);
+                if (r < 0)
+                        log_warning_errno(r, "Failed to remove file descriptor from the store, ignoring: %m");
         }
 
         return 0;

diff --git a/src/machine/machined.c b/src/machine/machined.c
index 241be42c913f7ba61dda077f6ce42ef762386599..4ab459d3cafe240db21b3d54f0775ef30d74f1c5 100644 (file)
--- a/src/machine/machined.c
+++ b/src/machine/machined.c
@@ -6,14 +6,13 @@
 #include <sys/types.h>
 #include <unistd.h>
 
-#include "sd-daemon.h"
-
 #include "alloc-util.h"
 #include "bus-error.h"
 #include "bus-locator.h"
 #include "bus-log-control-api.h"
 #include "bus-polkit.h"
 #include "cgroup-util.h"
+#include "daemon-util.h"
 #include "dirent-util.h"
 #include "discover-image.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");
 
         log_debug("systemd-machined running as pid "PID_FMT, getpid_cached());
-        (void) sd_notify(false,
-                         "READY=1\n"
-                         "STATUS=Processing requests...");
+        r = sd_notify(false, NOTIFY_READY);
+        if (r < 0)
+                log_warning_errno(r, "Failed to send readiness notification, ignoring: %m");
 
         r = manager_run(m);
 
+        (void) sd_notify(false, NOTIFY_STOPPING);
         log_debug("systemd-machined stopped as pid "PID_FMT, getpid_cached());
-        (void) sd_notify(false,
-                         "STOPPING=1\n"
-                         "STATUS=Shutting down...");
-
         return r;
 }
 

diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index 8c0bc99d727205d7a80ff3e6be59e4ccf76b40af..7dc9e068422c42d8d29e4db95bcdc8198c871007 100644 (file)
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -4205,6 +4205,7 @@ static int nspawn_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t r
         ssize_t n;
         pid_t inner_child_pid;
         _cleanup_strv_free_ char **tags = NULL;
+        int r;
 
         assert(userdata);
 
@@ -4243,8 +4244,11 @@ static int nspawn_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t r
         if (!tags)
                 return log_oom();
 
-        if (strv_find(tags, "READY=1"))
-                (void) sd_notifyf(false, "READY=1\n");
+        if (strv_find(tags, "READY=1")) {
+                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=");
         if (p)
@@ -5134,8 +5138,11 @@ static int run_container(
         (void) sd_notifyf(false,
                           "STATUS=Container running.\n"
                           "X_NSPAWN_LEADER_PID=" PID_FMT, *pid);
-        if (!arg_notify_ready)
-                (void) sd_notify(false, "READY=1\n");
+        if (!arg_notify_ready) {
+                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) {
                 /* Try to kill the init system on SIGINT or SIGTERM */

diff --git a/src/portable/portabled.c b/src/portable/portabled.c
index 3c8e20e0f36977e702a243cd147ef0930170b80c..2f9afdc8f275e5bf01a29c573471c979e86a4f67 100644 (file)
--- a/src/portable/portabled.c
+++ b/src/portable/portabled.c
@@ -4,11 +4,11 @@
 #include <sys/types.h>
 
 #include "sd-bus.h"
-#include "sd-daemon.h"
 
 #include "alloc-util.h"
 #include "bus-log-control-api.h"
 #include "bus-polkit.h"
+#include "daemon-util.h"
 #include "def.h"
 #include "main-func.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");
 
         log_debug("systemd-portabled running as pid " PID_FMT, getpid_cached());
-        sd_notify(false,
-                  "READY=1\n"
-                  "STATUS=Processing requests...");
+        r = sd_notify(false, NOTIFY_READY);
+        if (r < 0)
+                log_warning_errno(r, "Failed to send readiness notification, ignoring: %m");
 
         r = manager_run(m);
 
-        (void) sd_notify(false,
-                         "STOPPING=1\n"
-                         "STATUS=Shutting down...");
+        (void) sd_notify(false, NOTIFY_STOPPING);
         log_debug("systemd-portabled stopped as pid " PID_FMT, getpid_cached());
         return r;
 }

diff --git a/src/rfkill/rfkill.c b/src/rfkill/rfkill.c
index bff1a2886be3bd7afca6d3f52ecfd7d0ad68ef00..bca2f3b812298b6fec9afa830d316e5ccf7c96e7 100644 (file)
--- a/src/rfkill/rfkill.c
+++ b/src/rfkill/rfkill.c
@@ -317,7 +317,10 @@ static int run(int argc, char *argv[]) {
                         if (!ready) {
                                 /* Notify manager that we are now finished with processing whatever was
                                  * 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;
                         }
 

diff --git a/src/udev/udevd.c b/src/udev/udevd.c
index b73f4776e70e9128121127076d5d1919d6fc0879..beec6e62e7be0f69dce4afd802af6d9c94065bf7 100644 (file)
--- a/src/udev/udevd.c
+++ b/src/udev/udevd.c
@@ -315,9 +315,18 @@ static void manager_exit(Manager *manager) {
         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 */
 static void manager_reload(Manager *manager) {
-
         assert(manager);
 
         sd_notify(false,
@@ -328,9 +337,7 @@ static void manager_reload(Manager *manager) {
         manager->rules = udev_rules_free(manager->rules);
         udev_builtin_exit();
 
-        sd_notifyf(false,
-                   "READY=1\n"
-                   "STATUS=Processing with %u children at max", arg_children_max);
+        notify_ready();
 }
 
 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);
                 arg_children_max = value->intval;
 
-                (void) sd_notifyf(false,
-                                  "READY=1\n"
-                                  "STATUS=Processing with %u children at max", arg_children_max);
+                notify_ready();
                 break;
         case UDEV_CTRL_PING:
                 log_debug("Received udev control message (PING)");
@@ -1862,9 +1867,7 @@ static int main_loop(Manager *manager) {
         if (r < 0)
                 log_error_errno(r, "Failed to apply permissions on static device nodes: %m");
 
-        (void) sd_notifyf(false,
-                          "READY=1\n"
-                          "STATUS=Processing with %u children at max", arg_children_max);
+        notify_ready();
 
         r = sd_event_loop(manager->event);
         if (r < 0)