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.

diff --git a/ChangeLog b/ChangeLog
index c4e9739ce774e14728b0a3075b8994da6dd94566..07681e2fc2d36e00ccf7cc139dd7e8006c015b86 100644 (file)
--- a/ChangeLog
+++ b/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

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 599ec0b19675434ffce10ae6db54eb2ab6aba27f..ce6386968e93cc07654b7b32f1b6f8e438d41148 100644 (file)
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -320,7 +320,7 @@ virEnumToString;
 virEventAddHandle;
 virEventRemoveHandle;
 virExec;
-virExecWithHook;
+virExecDaemonize;
 virSetCloseExec;
 virSetNonBlock;
 virFormatMacAddr;

diff --git a/src/proxy_internal.c b/src/proxy_internal.c
index 56e8db89fa9cc32cf033285f63dbce1f6273ba2c..9e7ab56f705eb8e8c64f5d99968b03eca84ea099 100644 (file)
--- a/src/proxy_internal.c
+++ b/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);
 }
 

diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index 7a46b1c06b7661e78b6f888bd3c671db46098623..bafdd73f78aedd34dbdddb32e426268156e0904a 100644 (file)
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -1434,38 +1434,32 @@ static int qemudStartVMDaemon(virConnectPtr conn,
     for (i = 0 ; i < ntapfds ; i++)
         FD_SET(tapfds[i], &keepfd);
 
-    ret = virExecWithHook(conn, argv, progenv, &keepfd, &child,
-                          stdin_fd, &vm->logfile, &vm->logfile,
-                          VIR_EXEC_NONBLOCK | VIR_EXEC_DAEMON,
-                          qemudSecurityHook, &hookData);
+    ret = virExecDaemonize(conn, argv, progenv, &keepfd, &child,
+                           stdin_fd, &vm->logfile, &vm->logfile,
+                           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);
+        while (retries) {
+            if ((ret = virFileReadPid(driver->stateDir,
+                                      vm->def->name, &vm->pid)) == 0)
+                break;
 
-        if (childstat == 0) {
-            while (retries) {
-                if ((ret = virFileReadPid(driver->stateDir, vm->def->name, &vm->pid)) == 0)
-                    break;
-                usleep(100*1000);
-                retries--;
-            }
-            if (ret) {
-                qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
-                                 _("Domain %s didn't show up\n"), vm->def->name);
-                ret = -1;
-            }
-        } else {
+            usleep(100*1000);
+            retries--;
+        }
+        if (ret) {
             qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
-                             "%s", _("Unable to daemonize QEMU process"));
+                             _("Domain %s didn't show up\n"), vm->def->name);
             ret = -1;
         }
-        vm->state = migrateFrom ? VIR_DOMAIN_PAUSED : VIR_DOMAIN_RUNNING;
-    }
+    } else
+        ret = -1;
+
+    vm->state = migrateFrom ? VIR_DOMAIN_PAUSED : VIR_DOMAIN_RUNNING;
 
     for (i = 0 ; argv[i] ; i++)
         VIR_FREE(argv[i]);

diff --git a/src/remote_internal.c b/src/remote_internal.c
index 24226e502341a602283a7c73a4ff80cc8e093840..4f69876dad1c2fda81a0bef75cd502d3ded8558a 100644 (file)
--- a/src/remote_internal.c
+++ b/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;
 }

diff --git a/src/uml_driver.c b/src/uml_driver.c
index 0457f13c865b8f11352fc828dec032e554d1f042..b72c540831dab084fcc847e2dc9caa3a13bbe293 100644 (file)
--- a/src/uml_driver.c
+++ b/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,
-                  -1, &logfd, &logfd,
-                  VIR_EXEC_DAEMON);
+    ret = virExecDaemonize(conn, argv, progenv, &keepfd, &pid,
+                           -1, &logfd, &logfd,
+                           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);

diff --git a/src/util.c b/src/util.c
index c5edc9d96ef346b289dcf24fd11930f30ec4df65..829e1783ba6d788e3fd9a53666479251c9bcbdb6 100644 (file)
--- a/src/util.c
+++ b/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) {

diff --git a/src/util.h b/src/util.h
index 4bed0778d88dd0f26422e23aab84bf30a83b8b2f..bc29b169f211062b542daeeb70a657e7578e236a 100644 (file)
--- a/src/util.h
+++ b/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,