Re: [libvirt] [PATCH 10/21] Don't let parent of daemon exit until basic initialization is done

The daemonizing code lets the parent exit almost immediately. This means that it may think it has successfully started even when important failures occur like not being able to acquire the PID file. It also means network sockets are not yet open. To address this when daemonizing the parent passes an open pipe file descriptor to the child. The child does its basic initialization and then writes a status code to the pipe indicating either success, or failure. This ensures that when daemonizing, the parent does not exit until the pidfile is acquired & basic network sockets are open. Initialization of the libvirt drivers is still done asynchronously since this may take a very long time. * daemon/libvirtd.c: Force parent to stay around until basic config file, pidfile & network socket init is completed --- daemon/libvirtd.c | 120 +++++++++++++++++++++++++++++++++++++++++++++-------- 1 files changed, 102 insertions(+), 18 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 4e268a2..b118da8 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -185,6 +185,30 @@ static int max_client_requests = 5; static sig_atomic_t sig_errors = 0; static int sig_lasterrno = 0; +enum { + VIR_DAEMON_ERR_NONE = 0, + VIR_DAEMON_ERR_PIDFILE, + VIR_DAEMON_ERR_RUNDIR, + VIR_DAEMON_ERR_INIT, + VIR_DAEMON_ERR_SIGNAL, + VIR_DAEMON_ERR_PRIVS, + VIR_DAEMON_ERR_NETWORK, + VIR_DAEMON_ERR_CONFIG, + + VIR_DAEMON_ERR_LAST +}; + +VIR_ENUM_DECL(virDaemonErr) +VIR_ENUM_IMPL(virDaemonErr, VIR_DAEMON_ERR_LAST, + "Initialization successful", + "Unable to obtain pidfile", + "Unable to create rundir", + "Unable to initialize libvirt", + "Unable to setup signal handlers", + "Unable to drop privileges", + "Unable to initialize network sockets", + "Unable to load configuration file") + static void sig_handler(int sig, siginfo_t * siginfo, void* context ATTRIBUTE_UNUSED) { int origerrno; @@ -375,7 +399,11 @@ qemudDispatchSignalEvent(int watch ATTRIBUTE_UNUSED, } -static int qemudGoDaemon(void) { +static int daemonForkIntoBackground(void) { + int statuspipe[2]; + if (pipe(statuspipe) < 0) + return -1; + int pid = fork(); switch (pid) { case 0: @@ -384,6 +412,8 @@ static int qemudGoDaemon(void) { int stdoutfd = -1; int nextpid; + close(statuspipe[0]); + if ((stdinfd = open("/dev/null", O_RDONLY)) < 0) goto cleanup; if ((stdoutfd = open("/dev/null", O_WRONLY)) < 0) @@ -407,7 +437,7 @@ static int qemudGoDaemon(void) { nextpid = fork(); switch (nextpid) { case 0: - return 0; + return statuspipe[1]; case -1: return -1; default: @@ -428,15 +458,29 @@ static int qemudGoDaemon(void) { default: { - int got, status = 0; - /* We wait to make sure the next child forked - successfully */ - if ((got = waitpid(pid, &status, 0)) < 0 || + int got, exitstatus = 0; + int ret; + char status; + + close(statuspipe[1]); + + /* We wait to make sure the first child forked successfully */ + if ((got = waitpid(pid, &exitstatus, 0)) < 0 || got != pid || status != 0) { return -1; } - _exit(0); + + /* Now block until the second child initializes successfully */ + again: + ret = read(statuspipe[0], &status, 1); + if (ret == -1 && errno == EINTR) + goto again; + + if (ret == 1 && status != 0) { + fprintf(stderr, "error: %s\n", virDaemonErrTypeToString(status)); + } + _exit(ret == 1 && status == 0 ? 0 : 1); } } } @@ -871,8 +915,6 @@ static struct qemud_server *qemudInitialize(void) { virEventUpdateTimeoutImpl, virEventRemoveTimeoutImpl); - virStateInitialize(server->privileged); - return server; } @@ -2902,6 +2944,7 @@ int main(int argc, char **argv) { struct qemud_server *server = NULL; const char *pid_file = NULL; const char *remote_config_file = NULL; + int statuswrite = -1; int ret = 1; struct option opts[] = { @@ -2985,7 +3028,7 @@ int main(int argc, char **argv) { if (godaemon) { char ebuf[1024]; - if (qemudGoDaemon() < 0) { + if ((statuswrite = daemonForkIntoBackground()) < 0) { VIR_ERROR(_("Failed to fork as daemon: %s"), virStrerror(errno, ebuf, sizeof ebuf)); goto error; @@ -3000,8 +3043,11 @@ int main(int argc, char **argv) { /* If we have a pidfile set, claim it now, exiting if already taken */ if (pid_file != NULL && - qemudWritePidFile (pid_file) < 0) + qemudWritePidFile (pid_file) < 0) { + pid_file = NULL; /* Prevent unlinking of someone else's pid ! */ + ret = VIR_DAEMON_ERR_PIDFILE; goto error; + } /* Ensure the rundir exists (on tmpfs on some systems) */ if (geteuid() == 0) { @@ -3010,7 +3056,8 @@ int main(int argc, char **argv) { if (mkdir (rundir, 0755)) { if (errno != EEXIST) { VIR_ERROR0 (_("unable to create rundir")); - return -1; + ret = VIR_DAEMON_ERR_RUNDIR; + goto error; } } } @@ -3021,34 +3068,71 @@ int main(int argc, char **argv) { * which is also passed into all libvirt stateful * drivers */ - if (qemudSetupPrivs() < 0) + if (qemudSetupPrivs() < 0) { + ret = VIR_DAEMON_ERR_PRIVS; goto error; + } if (!(server = qemudInitialize())) { - ret = 2; + ret = VIR_DAEMON_ERR_INIT; goto error; } - if ((daemonSetupSignals(server)) < 0) + if ((daemonSetupSignals(server)) < 0) { + ret = VIR_DAEMON_ERR_SIGNAL; goto error; + } /* Read the config file (if it exists). */ - if (remoteReadConfigFile (server, remote_config_file) < 0) + if (remoteReadConfigFile (server, remote_config_file) < 0) { + ret = VIR_DAEMON_ERR_CONFIG; goto error; + } /* Disable error func, now logging is setup */ virSetErrorFunc(NULL, virshErrorHandler); if (qemudNetworkInit(server) < 0) { - ret = 2; + ret = VIR_DAEMON_ERR_NETWORK; goto error; } - qemudRunLoop(server); + /* Tell parent of daemon that basic initialization is complete + * In particular we're ready to accept net connections & have + * written the pidfile + */ + if (statuswrite != -1) { + char status = 0; + while (write(statuswrite, &status, 1) == -1 && + errno == EINTR) + ; + close(statuswrite); + statuswrite = -1; + } + + /* Start the stateful HV drivers + * This is delibrately done after telling the parent process + * we're ready, since it can take a long time and this will + * seriously delay OS bootup process */ + if (virStateInitialize(server->privileged) < 0) { + VIR_ERROR0("Driver state initialization failed"); + goto error; + }