util: make it easier to grab only regular process exit

Right now, a caller waiting for a child process either requires
the child to have status 0, or must use WIFEXITED() and friends
itself.  But in many cases, we want the middle ground of treating
fatal signals as an error, and directly accessing the normal exit
value without having to use WEXITSTATUS(), in order to easily
detect an expected non-zero exit status.  This adds the middle
ground to the low-level virProcessWait; the next patch will add
it to virCommand.

* src/util/virprocess.h (virProcessWait): Alter signature.
* src/util/virprocess.c (virProcessWait): Add parameter.
(virProcessRunInMountNamespace): Adjust caller.
* src/util/vircommand.c (virCommandWait): Likewise.
* src/util/virfile.c (virFileAccessibleAs): Likewise.
* src/lxc/lxc_container.c (lxcContainerHasReboot)
(lxcContainerAvailable): Likewise.
* daemon/libvirtd.c (daemonForkIntoBackground): Likewise.
* tools/virt-login-shell.c (main): Likewise.
* tools/virsh-domain.c (cmdLxcEnterNamespace): Likewise.
* tests/testutils.c (virtTestCaptureProgramOutput): Likewise.
* tests/commandtest.c (test23): Likewise.

Signed-off-by: Eric Blake <eblake@redhat.com>

diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index b27c6fda6f7564668c2db87d0ee47c30c54caa54..72f0e81e67e2cbde9b81b957b89ec82efb097d92 100644 (file)
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -1,7 +1,7 @@
 /*
  * libvirtd.c: daemon start of day, guest process & i/o management
  *
- * Copyright (C) 2006-2012 Red Hat, Inc.
+ * Copyright (C) 2006-2014 Red Hat, Inc.
  * Copyright (C) 2006 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
@@ -206,7 +206,7 @@ static int daemonForkIntoBackground(const char *argv0)
             VIR_FORCE_CLOSE(statuspipe[1]);
 
             /* We wait to make sure the first child forked successfully */
-            if (virProcessWait(pid, NULL) < 0)
+            if (virProcessWait(pid, NULL, false) < 0)
                 goto error;
 
             /* If we get here, then the grandchild was spawned, so we

diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 71548043eb04f9ef454bacce52c8ed3c355cb185..f2440ff92bb79d5d2522ff65c0aff5e41b64fc5a 100644 (file)
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -173,11 +173,11 @@ int lxcContainerHasReboot(void)
         virReportSystemError(errno, "%s",
                              _("Unable to clone to check reboot support"));
         return -1;
-    } else if (virProcessWait(cpid, &status) < 0) {
+    } else if (virProcessWait(cpid, &status, false) < 0) {
         return -1;
     }
 
-    if (WEXITSTATUS(status) != 1) {
+    if (status != 1) {
         VIR_DEBUG("Containerized reboot support is missing "
                   "(kernel probably too old < 3.4)");
         return 0;
@@ -2075,7 +2075,7 @@ int lxcContainerAvailable(int features)
         VIR_DEBUG("clone call returned %s, container support is not enabled",
                   virStrerror(errno, ebuf, sizeof(ebuf)));
         return -1;
-    } else if (virProcessWait(cpid, NULL) < 0) {
+    } else if (virProcessWait(cpid, NULL, false) < 0) {
         return -1;
     }
 

diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index b9e5f3748a87d135a9574636a9e8986a8e8ec768..a4397b4c9096bf3dd0c77fd9296043febca56999 100644 (file)
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -2372,7 +2372,7 @@ virCommandWait(virCommandPtr cmd, int *exitstatus)
      * message is not as detailed as what we can provide.  So, we
      * guarantee that virProcessWait only fails due to failure to wait,
      * and repeat the exitstatus check code ourselves.  */
-    ret = virProcessWait(cmd->pid, exitstatus ? exitstatus : &status);
+    ret = virProcessWait(cmd->pid, exitstatus ? exitstatus : &status, true);
     if (cmd->flags & VIR_EXEC_ASYNC_IO) {
         cmd->flags &= ~VIR_EXEC_ASYNC_IO;
         virThreadJoin(cmd->asyncioThread);

diff --git a/src/util/virfile.c b/src/util/virfile.c
index 96f078dfd336e904e17c4c78b1b3079c3c383118..6fb7d6fcb6dd833ec9b5a076e32ff80c29c182c3 100644 (file)
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -1,7 +1,7 @@
 /*
  * virfile.c: safer file handling
  *
- * Copyright (C) 2010-2013 Red Hat, Inc.
+ * Copyright (C) 2010-2014 Red Hat, Inc.
  * Copyright (C) 2010 IBM Corporation
  * Copyright (C) 2010 Stefan Berger
  * Copyright (C) 2010 Eric Blake
@@ -1739,19 +1739,13 @@ virFileAccessibleAs(const char *path, int mode,
 
     if (pid) { /* parent */
         VIR_FREE(groups);
-        if (virProcessWait(pid, &status) < 0) {
-            /* virProcessWait() already
-             * reported error */
-            return -1;
-        }
-
-        if (!WIFEXITED(status)) {
-            errno = EINTR;
+        if (virProcessWait(pid, &status, false) < 0) {
+            /* virProcessWait() already reported error */
             return -1;
         }
 
         if (status) {
-            errno = WEXITSTATUS(status);
+            errno = status;
             return -1;
         }
 

diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index bd406db3065081125000b5dde283483c62a2aab2..72e995026dc2c81729c928135a6b0993833cef5f 100644 (file)
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -155,15 +155,21 @@ virProcessAbort(pid_t pid)
  * virProcessWait:
  * @pid: child to wait on
  * @exitstatus: optional status collection
+ * @raw: whether to pass non-normal status back to caller
  *
- * Wait for a child process to complete.
- * Return -1 on any error waiting for
- * completion. Returns 0 if the command
- * finished with the exit status set.  If @exitstatus is NULL, then the
- * child must exit with status 0 for this to succeed.
+ * Wait for a child process to complete.  If @exitstatus is NULL, then the
+ * child must exit normally with status 0.  Otherwise, if @raw is false,
+ * the child must exit normally, and @exitstatus will contain the final
+ * exit status (no need for the caller to use WEXITSTATUS()).  If @raw is
+ * true, then the result of wait() is returned in @exitstatus, and the
+ * caller must use WIFEXITED() and friends to decipher the child's status.
+ *
+ * Returns 0 on a successful wait.  Returns -1 on any error waiting for
+ * completion, or if the command completed with a status that cannot be
+ * reflected via the choice of @exitstatus and @raw.
  */
 int
-virProcessWait(pid_t pid, int *exitstatus)
+virProcessWait(pid_t pid, int *exitstatus, bool raw)
 {
     int ret;
     int status;
@@ -185,19 +191,27 @@ virProcessWait(pid_t pid, int *exitstatus)
     }
 
     if (exitstatus == NULL) {
-        if (status != 0) {
-            char *st = virProcessTranslateStatus(status);
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("Child process (%lld) unexpected %s"),
-                           (long long) pid, NULLSTR(st));
-            VIR_FREE(st);
-            return -1;
-        }
-    } else {
+        if (status != 0)
+            goto error;
+    } else if (raw) {
         *exitstatus = status;
+    } else if (WIFEXITED(status)) {
+        *exitstatus = WEXITSTATUS(status);
+    } else {
+        goto error;
     }
 
     return 0;
+
+error:
+    {
+        char *st = virProcessTranslateStatus(status);
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Child process (%lld) unexpected %s"),
+                       (long long) pid, NULLSTR(st));
+        VIR_FREE(st);
+    }
+    return -1;
 }
 
 
@@ -965,7 +979,7 @@ virProcessRunInMountNamespace(pid_t pid,
 
         VIR_FORCE_CLOSE(errfd[1]);
         ignore_value(virFileReadHeaderFD(errfd[0], 1024, &buf));
-        ret = virProcessWait(child, &status);
+        ret = virProcessWait(child, &status, false);
         if (!ret)
             ret = status == EXIT_CANCELED ? -1 : status;
         VIR_FREE(buf);

diff --git a/src/util/virprocess.h b/src/util/virprocess.h
index abe3635bc99c1ee9e264df747146ba2fbf8f4624..bcaede58a9797ae858045e2768e8d8dfc2665860 100644 (file)
--- a/src/util/virprocess.h
+++ b/src/util/virprocess.h
@@ -36,7 +36,7 @@ virProcessAbort(pid_t pid);
 void virProcessExitWithStatus(int status) ATTRIBUTE_NORETURN;
 
 int
-virProcessWait(pid_t pid, int *exitstatus)
+virProcessWait(pid_t pid, int *exitstatus, bool raw)
     ATTRIBUTE_RETURN_CHECK;
 
 int virProcessKill(pid_t pid, int sig);

diff --git a/tests/commandtest.c b/tests/commandtest.c
index fcda5e61401cc98c787791c6b0d95f6594630713..c0391a5e1e4e6c628ee163e0e8a57f5fbb163a6d 100644 (file)
--- a/tests/commandtest.c
+++ b/tests/commandtest.c
@@ -958,13 +958,13 @@ test23(const void *unused ATTRIBUTE_UNUSED)
             _exit(EXIT_FAILURE);
         if (pid == 0)
             _exit(42);
-        if (virProcessWait(pid, &status) < 0)
+        if (virProcessWait(pid, &status, true) < 0)
             _exit(EXIT_FAILURE);
         virProcessExitWithStatus(status);
         _exit(EXIT_FAILURE);
     }
 
-    if (virProcessWait(pid, &status) < 0)
+    if (virProcessWait(pid, &status, true) < 0)
         goto cleanup;
     if (!WIFEXITED(status) || WEXITSTATUS(status) != 42) {
         printf("Unexpected status %d\n", status);
@@ -982,13 +982,13 @@ test23(const void *unused ATTRIBUTE_UNUSED)
             raise(SIGKILL);
             _exit(EXIT_FAILURE);
         }
-        if (virProcessWait(pid, &status) < 0)
+        if (virProcessWait(pid, &status, true) < 0)
             _exit(EXIT_FAILURE);
         virProcessExitWithStatus(status);
         _exit(EXIT_FAILURE);
     }
 
-    if (virProcessWait(pid, &status) < 0)
+    if (virProcessWait(pid, &status, true) < 0)
         goto cleanup;
     if (!WIFSIGNALED(status) || WTERMSIG(status) != SIGKILL) {
         printf("Unexpected status %d\n", status);

diff --git a/tests/testutils.c b/tests/testutils.c
index 97d5b50cdc499c6f1dba2b4bea61b7ba8b24b5ec..ede623924e3ec670e1c322758258fc782a7d8ce1 100644 (file)
--- a/tests/testutils.c
+++ b/tests/testutils.c
@@ -1,7 +1,7 @@
 /*
  * testutils.c: basic test utils
  *
- * Copyright (C) 2005-2013 Red Hat, Inc.
+ * Copyright (C) 2005-2014 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -427,7 +427,7 @@ virtTestCaptureProgramOutput(const char *const argv[], char **buf, int maxlen)
         VIR_FORCE_CLOSE(pipefd[1]);
         len = virFileReadLimFD(pipefd[0], maxlen, buf);
         VIR_FORCE_CLOSE(pipefd[0]);
-        if (virProcessWait(pid, NULL) < 0)
+        if (virProcessWait(pid, NULL, false) < 0)
             return -1;
 
         return len;

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 6df18a33ea1ef66db83f1510b4d0bb0373f278ca..520c0eda5c8ba4d57d8028def68fd353558db363 100644 (file)
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -8206,7 +8206,7 @@ cmdLxcEnterNamespace(vshControl *ctl, const vshCmd *cmd)
             execv(cmdargv[0], cmdargv);
             _exit(255);
         } else {
-            if (virProcessWait(pid, NULL) < 0)
+            if (virProcessWait(pid, NULL, false) < 0)
                 _exit(255);
         }
         _exit(0);
@@ -8214,7 +8214,7 @@ cmdLxcEnterNamespace(vshControl *ctl, const vshCmd *cmd)
         for (i = 0; i < nfdlist; i++)
             VIR_FORCE_CLOSE(fdlist[i]);
         VIR_FREE(fdlist);
-        if (virProcessWait(pid, NULL) < 0)
+        if (virProcessWait(pid, NULL, false) < 0)
             goto cleanup;
     }
 

diff --git a/tools/virt-login-shell.c b/tools/virt-login-shell.c
index 5b85d153832596183f89d7062b7c58aea6ea7a0a..819cc5c752b334847805ce34a63415cf141469f9 100644 (file)
--- a/tools/virt-login-shell.c
+++ b/tools/virt-login-shell.c
@@ -369,9 +369,9 @@ main(int argc, char **argv)
                 return EXIT_FAILURE;
             }
         }
-        return virProcessWait(ccpid, &status2);
+        return virProcessWait(ccpid, &status2, true);
     }
-    ret = virProcessWait(cpid, &status);
+    ret = virProcessWait(cpid, &status, true);
 
 cleanup:
     virConfFree(conf);