From: Ben Pfaff Date: Thu, 17 Dec 2009 18:56:01 +0000 (-0800) Subject: daemon: Allow daemon child process to report success or failure to parent. X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=95440284bdf8ac9a94c3e119d011d76acab577a7;p=openvswitch 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 50cc3352..c4effa9b 100644 --- a/lib/daemon.c +++ b/lib/daemon.c @@ -20,6 +20,7 @@ #include #include #include +#include #include #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 06280ac2..8ded63f1 100644 --- 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 3bd5bc50..125001fe 100644 --- 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 bb55c7fe..e2816cfc 100644 --- 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 7f79a527..ba97faf6 100644 --- 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 90946731..62faaaa5 100644 --- 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 52a80e09..4cefc403 100644 --- 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);