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 50cc335..c4effa9 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 06280ac..8ded63f 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 3bd5bc5..125001f 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 bb55c7f..e2816cf 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 7f79a52..ba97faf 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 9094673..62faaaa 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 52a80e0..4cefc40 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);