[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] PATCH: Write pidfile earlier in startup



On Fri, May 16, 2008 at 05:05:08PM +0100, Daniel P. Berrange wrote:
> When used with the --daemon arg, libvirtd will write out a pid file to
> /var/run/libvirtd.pid and exit if this file already exists. Unfortuantely
> it does this quite late in its startup procedure - in particular *after*
> the call to qemudInitialize(), which in turns initializes the drivers,
> which in turn autostarts all the VMs and networks.
> 
> So if you start a 2nd libvirtd instance it would do autostart before it
> got to checking for existing PID file. This is clearly very bad because
> the same VM would be started twice resulting in data corruption. Fortunately
> the Red Hat / Fedora initscript also checks the PID file before even starting
> the daemon, but this can affect people starting the daemon directly.
> 
> So this patch makes the goDaemon() / pidfile creation the very first thing
> the daemon does after parsing command line arguments. This also results in
> greatly increased startup time for OS, since the initscript doesn't have
> to wait for it to auto-start all the VMs & networks before it daemonizes.
> It also initializes the signal handlers before calling qemudInitialize()
> since these really need to be setup before we start any VMs / child procs.

There were actually some more changes needed. It needs to write the PID file
even if not running with --daemon mode to be truely safe, because some
init software won't run it in daemon mode at all. So this updated patch will
always write a PID file if run as root.  It also fixes a few flaws in the
cleanup process to ensure it only unlinks the pidfile on failure if it owned
the pidfile, and removes a pointless warning when running non-root.

Dan.

Index: qemud/qemud.c
===================================================================
RCS file: /data/cvs/libvirt/qemud/qemud.c,v
retrieving revision 1.100
diff -u -p -r1.100 qemud.c
--- qemud/qemud.c	15 May 2008 06:12:32 -0000	1.100
+++ qemud/qemud.c	16 May 2008 16:36:42 -0000
@@ -2143,6 +2143,26 @@ int main(int argc, char **argv) {
         }
     }
 
+    if (godaemon) {
+        openlog("libvirtd", 0, 0);
+        if (qemudGoDaemon() < 0) {
+            qemudLog(QEMUD_ERR, _("Failed to fork as daemon: %s"),
+                     strerror(errno));
+            goto error1;
+        }
+    }
+
+    /* If running as root and no PID file is set, use the default */
+    if (pid_file == NULL &&
+        getuid() == 0 &&
+        REMOTE_PID_FILE[0] != '\0')
+        pid_file = REMOTE_PID_FILE;
+
+    /* If we have a pidfile set, claim it now, exiting if already taken */
+    if (pid_file != NULL &&
+        qemudWritePidFile (pid_file) < 0)
+        goto error1;
+
     if (pipe(sigpipe) < 0 ||
         qemudSetNonBlock(sigpipe[0]) < 0 ||
         qemudSetNonBlock(sigpipe[1]) < 0 ||
@@ -2150,24 +2170,34 @@ int main(int argc, char **argv) {
         qemudSetCloseExec(sigpipe[1]) < 0) {
         qemudLog(QEMUD_ERR, _("Failed to create pipe: %s"),
                  strerror(errno));
-        goto error1;
+        goto error2;
     }
     sigwrite = sigpipe[1];
 
+    sig_action.sa_sigaction = sig_handler;
+    sig_action.sa_flags = SA_SIGINFO;
+    sigemptyset(&sig_action.sa_mask);
+
+    sigaction(SIGHUP, &sig_action, NULL);
+    sigaction(SIGINT, &sig_action, NULL);
+    sigaction(SIGQUIT, &sig_action, NULL);
+    sigaction(SIGTERM, &sig_action, NULL);
+    sigaction(SIGCHLD, &sig_action, NULL);
+
+    sig_action.sa_handler = SIG_IGN;
+    sigaction(SIGPIPE, &sig_action, NULL);
+
     if (!(server = qemudInitialize(sigpipe[0]))) {
         ret = 2;
-        goto error1;
+        goto error2;
     }
 
     /* Read the config file (if it exists). */
     if (remoteReadConfigFile (server, remote_config_file) < 0)
-        goto error1;
+        goto error2;
 
     /* Change the group ownership of /var/run/libvirt to unix_sock_gid */
-    if (getuid() != 0) {
-        qemudLog (QEMUD_WARN,
-                  "%s", _("Cannot set group ownership when not running as root"));
-    } else {
+    if (getuid() == 0) {
         const char *sockdirname = LOCAL_STATE_DIR "/run/libvirt";
 
         if (chown(sockdirname, -1, unix_sock_gid) < 0)
@@ -2175,37 +2205,6 @@ int main(int argc, char **argv) {
                      sockdirname);
     }
 
-    if (godaemon) {
-        openlog("libvirtd", 0, 0);
-        if (qemudGoDaemon() < 0) {
-            qemudLog(QEMUD_ERR, _("Failed to fork as daemon: %s"),
-                     strerror(errno));
-            goto error1;
-        }
-
-        /* Choose the name of the PID file. */
-        if (!pid_file) {
-            if (REMOTE_PID_FILE[0] != '\0')
-                pid_file = REMOTE_PID_FILE;
-        }
-
-        if (pid_file && qemudWritePidFile (pid_file) < 0)
-            goto error1;
-    }
-
-    sig_action.sa_sigaction = sig_handler;
-    sig_action.sa_flags = SA_SIGINFO;
-    sigemptyset(&sig_action.sa_mask);
-
-    sigaction(SIGHUP, &sig_action, NULL);
-    sigaction(SIGINT, &sig_action, NULL);
-    sigaction(SIGQUIT, &sig_action, NULL);
-    sigaction(SIGTERM, &sig_action, NULL);
-    sigaction(SIGCHLD, &sig_action, NULL);
-
-    sig_action.sa_handler = SIG_IGN;
-    sigaction(SIGPIPE, &sig_action, NULL);
-
     if (virEventAddHandleImpl(sigpipe[0],
                               POLLIN,
                               qemudDispatchSignalEvent,
@@ -2223,19 +2222,17 @@ int main(int argc, char **argv) {
 
     qemudRunLoop(server);
 
-    close(sigwrite);
-
-    if (godaemon)
-        closelog();
-
     ret = 0;
 
- error2:
-    if (godaemon && pid_file)
-        unlink (pid_file);
-
- error1:
+error2:
     if (server)
         qemudCleanup(server);
+    if (pid_file)
+        unlink (pid_file);
+    close(sigwrite);
+
+error1:
+    if (godaemon)
+        closelog();
     return ret;
 }

-- 
|: Red Hat, Engineering, Boston   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]