Add helper function virExecDaemonize

Wraps __virExec with the VIR_EXEC_DAEMON flag. Waits on the intermediate
process to ensure we don't end up with any zombies, and differentiates between
original process errors and intermediate process errors.

ChangeLog

@@ -1,3 +1,10 @@
+Mon May 11 09:29:52 EDT 2009 Cole Robinson <crobinso@redhat.com>
+
+	* src/libvirt_private.syms src/util.[ch]: Add a helper function
+	virExecDaemonize
+	* src/proxy_internal.c src/qemu_driver.c src/uml_driver.c
+	src/remote_driver.c: Use the new helper.
+
 Mon May 11 11:54:53 CEST 2009 Daniel Veillard <veillard@redhat.com>
 
 	* src/vbox/vbox_tmpl.c: "Host only" and "Internal" network support

src/libvirt_private.syms

@@ -320,7 +320,7 @@ virEnumToString;
 virEventAddHandle;
 virEventRemoveHandle;
 virExec;
-virExecWithHook;
+virExecDaemonize;
 virSetCloseExec;
 virSetNonBlock;
 virFormatMacAddr;

src/proxy_internal.c

@@ -143,7 +143,6 @@ static int
 virProxyForkServer(void)
 {
     const char *proxyPath = virProxyFindServerPath();
-    int ret, status;
     pid_t pid;
     const char *proxyarg[2];
 
@@ -157,20 +156,11 @@ virProxyForkServer(void)
     proxyarg[0] = proxyPath;
     proxyarg[1] = NULL;
 
-    if (virExec(NULL, proxyarg, NULL, NULL,
-                &pid, -1, NULL, NULL, VIR_EXEC_DAEMON) < 0)
+    if (virExecDaemonize(NULL, proxyarg, NULL, NULL,
+                         &pid, -1, NULL, NULL, 0,
+                         NULL, NULL) < 0)
         VIR_ERROR0("Failed to fork libvirt_proxy\n");
 
-    /*
-     * do a waitpid on the intermediate process to avoid zombies.
-     */
-retry_wait:
-    ret = waitpid(pid, &status, 0);
-    if (ret < 0) {
-        if (errno == EINTR)
-            goto retry_wait;
-    }
-
     return (0);
 }
 

src/qemu_driver.c

@@ -1434,23 +1434,20 @@ static int qemudStartVMDaemon(virConnectPtr conn,
     for (i = 0 ; i < ntapfds ; i++)
         FD_SET(tapfds[i], &keepfd);
 
-    ret = virExecWithHook(conn, argv, progenv, &keepfd, &child,
+    ret = virExecDaemonize(conn, argv, progenv, &keepfd, &child,
                            stdin_fd, &vm->logfile, &vm->logfile,
-                          VIR_EXEC_NONBLOCK | VIR_EXEC_DAEMON,
+                           VIR_EXEC_NONBLOCK,
                            qemudSecurityHook, &hookData);
 
     /* wait for qemu process to to show up */
     if (ret == 0) {
         int retries = 100;
-        int childstat;
 
-        while (waitpid(child, &childstat, 0) == -1 &&
-               errno == EINTR);
-
-        if (childstat == 0) {
         while (retries) {
-                if ((ret = virFileReadPid(driver->stateDir, vm->def->name, &vm->pid)) == 0)
+            if ((ret = virFileReadPid(driver->stateDir,
+                                      vm->def->name, &vm->pid)) == 0)
                 break;
+
             usleep(100*1000);
             retries--;
         }
@@ -1459,13 +1456,10 @@ static int qemudStartVMDaemon(virConnectPtr conn,
                              _("Domain %s didn't show up\n"), vm->def->name);
             ret = -1;
         }
-        } else {
-            qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
-                             "%s", _("Unable to daemonize QEMU process"));
+    } else
         ret = -1;
-        }
+
     vm->state = migrateFrom ? VIR_DOMAIN_PAUSED : VIR_DOMAIN_RUNNING;
-    }
 
     for (i = 0 ; argv[i] ; i++)
         VIR_FREE(argv[i]);

src/remote_internal.c

@@ -284,7 +284,6 @@ remoteForkDaemon(virConnectPtr conn)
 {
     const char *daemonPath = remoteFindDaemonPath();
     const char *const daemonargs[] = { daemonPath, "--timeout=30", NULL };
-    int ret, status;
     pid_t pid;
 
     if (!daemonPath) {
@@ -292,18 +291,10 @@ remoteForkDaemon(virConnectPtr conn)
         return -1;
     }
 
-    if (virExec(NULL, daemonargs, NULL, NULL,
-                &pid, -1, NULL, NULL, VIR_EXEC_DAEMON) < 0)
+    if (virExecDaemonize(NULL, daemonargs, NULL, NULL,
+                         &pid, -1, NULL, NULL, 0,
+                         NULL, NULL) < 0)
         return -1;
-    /*
-     * do a waitpid on the intermediate process to avoid zombies.
-     */
- retry_wait:
-    ret = waitpid(pid, &status, 0);
-    if (ret < 0) {
-        if (errno == EINTR)
-            goto retry_wait;
-    }
 
     return 0;
 }

src/uml_driver.c

@@ -821,16 +821,11 @@ static int umlStartVMDaemon(virConnectPtr conn,
     for (i = 0 ; i < ntapfds ; i++)
         FD_SET(tapfds[i], &keepfd);
 
-    ret = virExec(conn, argv, progenv, &keepfd, &pid,
+    ret = virExecDaemonize(conn, argv, progenv, &keepfd, &pid,
                            -1, &logfd, &logfd,
-                  VIR_EXEC_DAEMON);
+                           0, NULL, NULL);
     close(logfd);
 
-    /* Cleanup intermediate proces */
-    if (waitpid(pid, NULL, 0) != pid)
-        umlLog(VIR_LOG_WARN, _("failed to wait on process: %d: %s\n"),
-               pid, virStrerror(errno, ebuf, sizeof ebuf));
-
     for (i = 0 ; argv[i] ; i++)
         VIR_FREE(argv[i]);
     VIR_FREE(argv);

src/util.c

@@ -591,6 +591,55 @@ virExec(virConnectPtr conn,
                            flags, NULL, NULL);
 }
 
+/*
+ * See __virExec for explanation of the arguments.
+ *
+ * This function will wait for the intermediate process (between the caller
+ * and the daemon) to exit. retpid will be the pid of the daemon, which can
+ * be checked for example to see if the daemon crashed immediately.
+ *
+ * Returns 0 on success
+ *         -1 if initial fork failed (will have a reported error)
+ *         -2 if intermediate process failed
+ *         (won't have a reported error. pending on where the failure
+ *          occured and when in the process occured, the error output
+ *          could have gone to stderr or the passed errfd).
+ */
+int virExecDaemonize(virConnectPtr conn,
+                     const char *const*argv,
+                     const char *const*envp,
+                     const fd_set *keepfd,
+                     pid_t *retpid,
+                     int infd, int *outfd, int *errfd,
+                     int flags,
+                     virExecHook hook,
+                     void *data) {
+    int ret;
+    int childstat = 0;
+
+    ret = virExecWithHook(conn, argv, envp, keepfd, retpid,
+                          infd, outfd, errfd,
+                          flags |= VIR_EXEC_DAEMON,
+                          hook, data);
+
+    /* __virExec should have set an error */
+    if (ret != 0)
+        return -1;
+
+    /* Wait for intermediate process to exit */
+    while (waitpid(*retpid, &childstat, 0) == -1 &&
+                   errno == EINTR);
+
+    if (childstat != 0) {
+        ReportError(conn, VIR_ERR_INTERNAL_ERROR,
+                    _("Intermediate daemon process exited with status %d."),
+                    WEXITSTATUS(childstat));
+        ret = -2;
+    }
+
+    return ret;
+}
+
 static int
 virPipeReadUntilEOF(virConnectPtr conn, int outfd, int errfd,
                     char **outbuf, char **errbuf) {

src/util.h

@@ -46,6 +46,15 @@ int virSetCloseExec(int fd);
  * after fork() but before execve() */
 typedef int (*virExecHook)(void *data);
 
+int virExecDaemonize(virConnectPtr conn,
+                     const char *const*argv,
+                     const char *const*envp,
+                     const fd_set *keepfd,
+                     pid_t *retpid,
+                     int infd, int *outfd, int *errfd,
+                     int flags,
+                     virExecHook hook,
+                     void *data);
 int virExecWithHook(virConnectPtr conn,
                     const char *const*argv,
                     const char *const*envp,