daemon: Allow daemon child process to report success or failure to parent.

There are conflicting pressures in startup of a daemon process:

    * The parent process should exit with an error code if the daemon
      cannot start up successfully.

    * Some startup actions must be performed in the child process, not in
      the parent.  The most obvious of these are file locking, since
      child processes do not inherit locks, and anything that requires
      knowing the child process's PID (e.g. unixctl sockets).

Until now, this conflict has usually been handled by giving up part of the
first property, i.e. in some cases the parent process would exit
successfully and the child immediately afterward exit with a failure code.

This commit introduces a better approach, by allowing daemons to perform
startup work in the child and only then signal the parent that they have
successfully started.  If the child instead exits without signaling
success, the parent passes this exit code along to its own parent.

This commit also modifies the daemons that can usefully take advantage of
this new feature to do so.

diff --git a/lib/daemon.c b/lib/daemon.c
index 50cc3352142755887baf5bfadc450303e4b4fc46..c4effa9b0b91c425326b29e365a94febe0631da7 100644 (file)
--- a/lib/daemon.c
+++ b/lib/daemon.c
@@ -20,6 +20,7 @@
 #include <fcntl.h>
 #include <stdlib.h>
 #include <string.h>
+#include <sys/wait.h>
 #include <unistd.h>
 #include "fatal-signal.h"
 #include "dirs.h"
@@ -39,9 +40,12 @@ static char *pidfile;
 /* Create pidfile even if one already exists and is locked? */
 static bool overwrite_pidfile;
 
-/* Should we chdir to "/". */
+/* Should we chdir to "/"? */
 static bool chdir_ = true;
 
+/* File descriptors used by daemonize_start() and daemonize_complete(). */
+static int daemonize_fds[2];
+
 /* Returns the file name that would be used for a pidfile if 'name' were
  * provided to set_pidfile().  The caller must free the returned string. */
 char *
@@ -210,48 +214,83 @@ make_pidfile(void)
  * detaches from the foreground session.  */
 void
 daemonize(void)
+{
+    daemonize_start();
+    daemonize_complete();
+}
+
+/* If daemonization is configured, then starts daemonization, by forking and
+ * returning in the child process.  The parent process hangs around until the
+ * child lets it know either that it completed startup successfully (by calling
+ * daemon_complete()) or that it failed to start up (by exiting with a nonzero
+ * exit code). */
+void
+daemonize_start(void)
 {
     if (detach) {
-        char c = 0;
-        int fds[2];
-        if (pipe(fds) < 0) {
+        pid_t pid;
+
+        if (pipe(daemonize_fds) < 0) {
             ovs_fatal(errno, "pipe failed");
         }
 
-        switch (fork()) {
-        default:
-            /* Parent process: wait for child to create pidfile, then exit. */
-            close(fds[1]);
+        pid = fork();
+        if (pid > 0) {
+            /* Running in parent process. */
+            char c;
+
+            close(daemonize_fds[1]);
             fatal_signal_fork();
-            if (read(fds[0], &c, 1) != 1) {
+            if (read(daemonize_fds[0], &c, 1) != 1) {
+                int retval;
+                int status;
+
+                do {
+                    retval = waitpid(pid, &status, 0);
+                } while (retval == -1 && errno == EINTR);
+
+                if (retval == pid
+                    && WIFEXITED(status)
+                    && WEXITSTATUS(status)) {
+                    /* Child exited with an error.  Convey the same error to
+                     * our parent process as a courtesy. */
+                    exit(WEXITSTATUS(status));
+                }
+
                 ovs_fatal(errno, "daemon child failed to signal startup");
             }
             exit(0);
-
-        case 0:
-            /* Child process. */
-            close(fds[0]);
+        } else if (!pid) {
+            /* Running in child process. */
+            close(daemonize_fds[0]);
             make_pidfile();
-            ignore(write(fds[1], &c, 1));
-            close(fds[1]);
-            setsid();
-            if (chdir_) {
-                ignore(chdir("/"));
-            }
             time_postfork();
             lockfile_postfork();
-            break;
-
-        case -1:
-            /* Error. */
+        } else {
             ovs_fatal(errno, "could not fork");
-            break;
         }
     } else {
         make_pidfile();
     }
 }
 
+/* If daemonization is configured, then this function notifies the parent
+ * process that the child process has completed startup successfully. */
+void
+daemonize_complete(void)
+{
+    if (detach) {
+        char c = 0;
+
+        ignore(write(daemonize_fds[1], &c, 1));
+        close(daemonize_fds[1]);
+        setsid();
+        if (chdir_) {
+            ignore(chdir("/"));
+        }
+    }
+}
+
 void
 daemon_usage(void)
 {

diff --git a/lib/daemon.h b/lib/daemon.h
index 06280ac24c3dbf60d2874f1d64354a36d7f50306..8ded63f1a03ef946f6858f85f5b2d251c7270562 100644 (file)
--- a/lib/daemon.h
+++ b/lib/daemon.h
@@ -59,6 +59,8 @@ bool is_chdir_enabled(void);
 void set_detach(void);
 bool get_detach(void);
 void daemonize(void);
+void daemonize_start(void);
+void daemonize_complete(void);
 void die_if_already_running(void);
 void ignore_existing_pidfile(void);
 void daemon_usage(void);

diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
index 3bd5bc50c723aff295d607ab4862f5778fad658a..125001fe6783c6cb9f4587792a8f5c24142b106f 100644 (file)
--- a/ovsdb/ovsdb-server.c
+++ b/ovsdb/ovsdb-server.c
@@ -58,12 +58,10 @@ main(int argc, char *argv[])
     struct unixctl_server *unixctl;
     struct ovsdb_jsonrpc_server *jsonrpc;
     struct svec active, passive;
-    struct pstream **listeners;
     struct ovsdb_error *error;
     struct ovsdb *db;
     const char *name;
     char *file_name;
-    bool do_chdir;
     bool exiting;
     int retval;
     size_t i;
@@ -77,31 +75,8 @@ main(int argc, char *argv[])
 
     parse_options(argc, argv, &file_name, &active, &passive, &unixctl_path);
 
-    /* Open all the passive sockets before detaching, to avoid race with
-     * processes that start up later. */
-    listeners = xmalloc(passive.n * sizeof *listeners);
-    for (i = 0; i < passive.n; i++) {
-        int error;
-
-        error = pstream_open(passive.names[i], &listeners[i]);
-        if (error) {
-            ovs_fatal(error, "failed to listen on \"%s\"", passive.names[i]);
-        }
-    }
-
-    if (get_detach() && is_chdir_enabled()) {
-        /* We need to skip chdir("/") in daemonize() and do it later, because
-         * we need to open the database and possible set up up Unix domain
-         * sockets in the current working directory after we daemonize.  We
-         * can't open the database before we daemonize because file locks
-         * aren't inherited by child processes.  */
-        do_chdir = true;
-        set_no_chdir();
-    } else {
-        do_chdir = false;
-    }
     die_if_already_running();
-    daemonize();
+    daemonize_start();
 
     error = ovsdb_file_open(file_name, false, &db);
     if (error) {
@@ -113,7 +88,14 @@ main(int argc, char *argv[])
         ovsdb_jsonrpc_server_connect(jsonrpc, name);
     }
     for (i = 0; i < passive.n; i++) {
-        ovsdb_jsonrpc_server_listen(jsonrpc, listeners[i]);
+        struct pstream *pstream;
+        int error;
+
+        error = pstream_open(passive.names[i], &pstream);
+        if (error) {
+            ovs_fatal(error, "failed to listen on \"%s\"", passive.names[i]);
+        }
+        ovsdb_jsonrpc_server_listen(jsonrpc, pstream);
     }
     svec_destroy(&active);
     svec_destroy(&passive);
@@ -123,11 +105,9 @@ main(int argc, char *argv[])
         ovs_fatal(retval, "could not listen for control connections");
     }
 
-    unixctl_command_register("exit", ovsdb_server_exit, &exiting);
+    daemonize_complete();
 
-    if (do_chdir) {
-        ignore(chdir("/"));
-    }
+    unixctl_command_register("exit", ovsdb_server_exit, &exiting);
 
     exiting = false;
     while (!exiting) {

diff --git a/utilities/ovs-controller.c b/utilities/ovs-controller.c
index bb55c7fefcfecb1416d058520b72e2051fcc629e..e2816cfc347d9bf1bb724d66e897916cea1a856b 100644 (file)
--- a/utilities/ovs-controller.c
+++ b/utilities/ovs-controller.c
@@ -127,13 +127,15 @@ main(int argc, char *argv[])
     }
 
     die_if_already_running();
-    daemonize();
+    daemonize_start();
 
     retval = unixctl_server_create(NULL, &unixctl);
     if (retval) {
         ovs_fatal(retval, "Could not listen for unixctl connections");
     }
 
+    daemonize_complete();
+
     while (n_switches > 0 || n_listeners > 0) {
         int iteration;
         int i;

diff --git a/utilities/ovs-openflowd.c b/utilities/ovs-openflowd.c
index 7f79a5270666e78705aeeb96f5522b1239d4a2ab..ba97faf6fef4b0f51df96f809d211a9b2d3340b7 100644 (file)
--- a/utilities/ovs-openflowd.c
+++ b/utilities/ovs-openflowd.c
@@ -125,7 +125,7 @@ main(int argc, char *argv[])
     signal(SIGPIPE, SIG_IGN);
 
     die_if_already_running();
-    daemonize();
+    daemonize_start();
 
     /* Start listening for ovs-appctl requests. */
     error = unixctl_server_create(NULL, &unixctl);
@@ -218,6 +218,8 @@ main(int argc, char *argv[])
         }
     }
 
+    daemonize_complete();
+
     while (ofproto_is_alive(ofproto)) {
         error = ofproto_run(ofproto);
         if (error) {

diff --git a/vswitchd/ovs-brcompatd.c b/vswitchd/ovs-brcompatd.c
index 909467316709b6f7558b3c2d4fc6b75d08acb5da..62faaaa54c58eb21b39a7748ccd4bc2945b3b1b4 100644 (file)
--- a/vswitchd/ovs-brcompatd.c
+++ b/vswitchd/ovs-brcompatd.c
@@ -1158,7 +1158,7 @@ main(int argc, char *argv[])
     process_init();
 
     die_if_already_running();
-    daemonize();
+    daemonize_start();
 
     retval = unixctl_server_create(NULL, &unixctl);
     if (retval) {
@@ -1176,6 +1176,8 @@ main(int argc, char *argv[])
         }
     }
 
+    daemonize_complete();
+
     idl = ovsdb_idl_create(remote, &ovsrec_idl_class);
 
     for (;;) {

diff --git a/vswitchd/ovs-vswitchd.c b/vswitchd/ovs-vswitchd.c
index 52a80e090d0577623dbbd531a2f4ed018f5dac64..4cefc403199ed43e21a429470ea098fda23ee732 100644 (file)
--- a/vswitchd/ovs-vswitchd.c
+++ b/vswitchd/ovs-vswitchd.c
@@ -73,13 +73,15 @@ main(int argc, char *argv[])
     process_init();
 
     die_if_already_running();
-    daemonize();
+    daemonize_start();
 
     retval = unixctl_server_create(NULL, &unixctl);
     if (retval) {
         ovs_fatal(retval, "could not listen for control connections");
     }
 
+    daemonize_complete();
+
     idl = ovsdb_idl_create(remote, &ovsrec_idl_class);
     idl_seqno = ovsdb_idl_get_seqno(idl);