From: Eric Blake <eblake@redhat.com>
Date: Thu, 20 Feb 2014 00:32:19 +0000 (-0700)
Subject: util: make it easier to grab only regular command exit
X-Git-Tag: v1.2.3-rc1~373
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b9dd878f;p=thirdparty%2Flibvirt.git

util: make it easier to grab only regular command exit

Auditing all callers of virCommandRun and virCommandWait that
passed a non-NULL pointer for exit status turned up some
interesting observations.  Many callers were merely passing
a pointer to avoid the overall command dying, but without
caring what the exit status was - but these callers would
be better off treating a child death by signal as an abnormal
exit.  Other callers were actually acting on the status, but
not all of them remembered to filter by WIFEXITED and convert
with WEXITSTATUS; depending on the platform, this can result
in a status being reported as 256 times too big.  And among
those that correctly parse the output, it gets rather verbose.
Finally, there were the callers that explicitly checked that
the status was 0, and gave their own message, but with fewer
details than what virCommand gives for free.

So the best idea is to move the complexity out of callers and
into virCommand - by default, we return the actual exit status
already cleaned through WEXITSTATUS and treat signals as a
failed command; but the few callers that care can ask for raw
status and act on it themselves.

* src/util/vircommand.h (virCommandRawStatus): New prototype.
* src/libvirt_private.syms (util/command.h): Export it.
* docs/internals/command.html.in: Document it.
* src/util/vircommand.c (virCommandRawStatus): New function.
(virCommandWait): Adjust semantics.
* tests/commandtest.c (test1): Test it.
* daemon/remote.c (remoteDispatchAuthPolkit): Adjust callers.
* src/access/viraccessdriverpolkit.c (virAccessDriverPolkitCheck):
Likewise.
* src/fdstream.c (virFDStreamCloseInt): Likewise.
* src/lxc/lxc_process.c (virLXCProcessStart): Likewise.
* src/qemu/qemu_command.c (qemuCreateInBridgePortWithHelper):
Likewise.
* src/xen/xen_driver.c (xenUnifiedXendProbe): Simplify.
* tests/reconnect.c (mymain): Likewise.
* tests/statstest.c (mymain): Likewise.
* src/bhyve/bhyve_process.c (virBhyveProcessStart)
(virBhyveProcessStop): Don't overwrite virCommand error.
* src/libvirt.c (virConnectAuthGainPolkit): Likewise.
* src/openvz/openvz_driver.c (openvzDomainGetBarrierLimit)
(openvzDomainSetBarrierLimit): Likewise.
* src/util/virebtables.c (virEbTablesOnceInit): Likewise.
* src/util/viriptables.c (virIpTablesOnceInit): Likewise.
* src/util/virnetdevveth.c (virNetDevVethCreate): Fix debug
message.
* src/qemu/qemu_capabilities.c (virQEMUCapsInitQMP): Add comment.
* src/storage/storage_backend_iscsi.c
(virStorageBackendISCSINodeUpdate): Likewise.

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

diff --git a/daemon/remote.c b/daemon/remote.c
index 932f65fc1c..b48d456d9e 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -3129,10 +3129,9 @@ remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED,
 
     authdismissed = (pkout && strstr(pkout, "dismissed=true"));
     if (status != 0) {
-        char *tmp = virProcessTranslateStatus(status);
-        VIR_ERROR(_("Policy kit denied action %s from pid %lld, uid %d: %s"),
-                  action, (long long) callerPid, callerUid, NULLSTR(tmp));
-        VIR_FREE(tmp);
+        VIR_ERROR(_("Policy kit denied action %s from pid %lld, uid %d "
+                    "with status %d"),
+                  action, (long long) callerPid, callerUid, status);
         goto authdeny;
     }
     PROBE(RPC_SERVER_CLIENT_AUTH_ALLOW,
diff --git a/docs/internals/command.html.in b/docs/internals/command.html.in
index 0336e65c33..f5c28afd9a 100644
--- a/docs/internals/command.html.in
+++ b/docs/internals/command.html.in
@@ -430,7 +430,7 @@
   if (string)
       VIR_DEBUG("about to run %s", string);
   VIR_FREE(string);
-  if (virCommandRun(cmd) &lt; 0)
+  if (virCommandRun(cmd, NULL) &lt; 0)
       return -1;
 </pre>
 
@@ -458,15 +458,24 @@
       non-zero exit status can represent a success condition,
       it is possible to request the exit status and perform
       that check manually instead of letting <code>virCommandRun</code>
-      raise the error
+      raise the error.  By default, the captured status is only
+      for a normal exit (death from a signal is treated as an error),
+      but a caller can use <code>virCommandRawStatus</code> to get
+      encoded status that includes any terminating signals.
     </p>
 
 <pre>
   int status;
   if (virCommandRun(cmd, &amp;status) &lt; 0)
-     return -1;
+      return -1;
+  if (status == 1) {
+    ...do stuff...
+  }
 
-  if (WEXITSTATUS(status) ...) {
+  virCommandRawStatus(cmd2);
+  if (virCommandRun(cmd2, &amp;status) &lt; 0)
+      return -1;
+  if (WIFEXITED(status) &amp;&amp; WEXITSTATUS(status) == 1) {
     ...do stuff...
   }
 </pre>
diff --git a/src/access/viraccessdriverpolkit.c b/src/access/viraccessdriverpolkit.c
index c32573900c..d9ebc49ee6 100644
--- a/src/access/viraccessdriverpolkit.c
+++ b/src/access/viraccessdriverpolkit.c
@@ -1,7 +1,7 @@
 /*
  * viraccessdriverpolkit.c: polkited access control driver
  *
- * Copyright (C) 2012 Red Hat, Inc.
+ * Copyright (C) 2012, 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
@@ -170,11 +170,10 @@ virAccessDriverPolkitCheck(virAccessManagerPtr manager ATTRIBUTE_UNUSED,
             ret = 0; /* Denied */
         } else {
             ret = -1; /* Error */
-            char *tmp = virProcessTranslateStatus(status);
             virAccessError(VIR_ERR_ACCESS_DENIED,
-                           _("Policy kit denied action %s from %s: %s"),
-                           actionid, process, NULLSTR(tmp));
-            VIR_FREE(tmp);
+                           _("Policy kit denied action %s from %s: "
+                             "exit status %d"),
+                           actionid, process, status);
         }
         goto cleanup;
     }
diff --git a/src/bhyve/bhyve_process.c b/src/bhyve/bhyve_process.c
index 7717fec684..ee856800e1 100644
--- a/src/bhyve/bhyve_process.c
+++ b/src/bhyve/bhyve_process.c
@@ -57,7 +57,7 @@ virBhyveProcessStart(virConnectPtr conn,
     virCommandPtr cmd = NULL;
     virCommandPtr load_cmd = NULL;
     bhyveConnPtr privconn = conn->privateData;
-    int ret = -1, status;
+    int ret = -1;
 
     if (virAsprintf(&logfile, "%s/%s.log",
                     BHYVE_LOG_DIR, vm->def->name) < 0)
@@ -114,15 +114,9 @@ virBhyveProcessStart(virConnectPtr conn,
                  virStrerror(errno, ebuf, sizeof(ebuf)));
 
     VIR_DEBUG("Loading domain '%s'", vm->def->name);
-    if (virCommandRun(load_cmd, &status) < 0)
+    if (virCommandRun(load_cmd, NULL) < 0)
         goto cleanup;
 
-    if (status != 0) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("Guest failed to load: %d"), status);
-        goto cleanup;
-    }
-
     /* Now we can start the domain */
     VIR_DEBUG("Starting domain '%s'", vm->def->name);
     ret = virCommandRun(cmd, NULL);
@@ -165,7 +159,6 @@ virBhyveProcessStop(bhyveConnPtr driver,
 {
     size_t i;
     int ret = -1;
-    int status;
     virCommandPtr cmd = NULL;
 
     if (!virDomainObjIsActive(vm)) {
@@ -203,15 +196,9 @@ virBhyveProcessStop(bhyveConnPtr driver,
     if (!(cmd = virBhyveProcessBuildDestroyCmd(driver, vm)))
         goto cleanup;
 
-    if (virCommandRun(cmd, &status) < 0)
+    if (virCommandRun(cmd, NULL) < 0)
         goto cleanup;
 
-    if (status != 0) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("Guest failed to stop: %d"), status);
-        goto cleanup;
-    }
-
     ret = 0;
 
     virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, reason);
diff --git a/src/fdstream.c b/src/fdstream.c
index f7dae390d0..04d62b8e3c 100644
--- a/src/fdstream.c
+++ b/src/fdstream.c
@@ -1,7 +1,7 @@
 /*
  * fdstream.c: generic streams impl for file descriptors
  *
- * Copyright (C) 2009-2012 Red Hat, Inc.
+ * Copyright (C) 2009-2012, 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
@@ -302,6 +302,7 @@ virFDStreamCloseInt(virStreamPtr st, bool streamAbort)
         else
             buf[len] = '\0';
 
+        virCommandRawStatus(fdst->cmd);
         if (virCommandWait(fdst->cmd, &status) < 0) {
             ret = -1;
         } else if (status != 0) {
diff --git a/src/libvirt.c b/src/libvirt.c
index dcf6b53f0e..a385935379 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -145,15 +145,13 @@ static int
 virConnectAuthGainPolkit(const char *privilege)
 {
     virCommandPtr cmd;
-    int status;
     int ret = -1;
 
     if (geteuid() == 0)
         return 0;
 
     cmd = virCommandNewArgList(POLKIT_AUTH, "--obtain", privilege, NULL);
-    if (virCommandRun(cmd, &status) < 0 ||
-        status > 0)
+    if (virCommandRun(cmd, NULL) < 0)
         goto cleanup;
 
     ret = 0;
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 97447f64bb..37f3fac1c1 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1108,6 +1108,7 @@ virCommandNewArgList;
 virCommandNewArgs;
 virCommandNonblockingFDs;
 virCommandPassFD;
+virCommandRawStatus;
 virCommandRequireHandshake;
 virCommandRun;
 virCommandRunAsync;
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 1ebad9c22e..fcdcb2d734 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -1224,12 +1224,19 @@ int virLXCProcessStart(virConnectPtr conn,
         VIR_WARN("Unable to seek to end of logfile: %s",
                  virStrerror(errno, ebuf, sizeof(ebuf)));
 
+    virCommandRawStatus(cmd);
     if (virCommandRun(cmd, &status) < 0)
         goto cleanup;
 
     if (status != 0) {
-        if (virLXCProcessReadLogOutput(vm, logfile, pos, ebuf, sizeof(ebuf)) <= 0)
-            snprintf(ebuf, sizeof(ebuf), "unexpected exit status %d", status);
+        if (virLXCProcessReadLogOutput(vm, logfile, pos, ebuf,
+                                       sizeof(ebuf)) <= 0) {
+            if (WIFEXITED(status))
+                snprintf(ebuf, sizeof(ebuf), _("unexpected exit status %d"),
+                         WEXITSTATUS(status));
+            else
+                snprintf(ebuf, sizeof(ebuf), "%s", _("terminated abnormally"));
+        }
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("guest failed to start: %s"), ebuf);
         goto cleanup;
diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
index b27ac4c7c5..393f397568 100644
--- a/src/openvz/openvz_driver.c
+++ b/src/openvz/openvz_driver.c
@@ -1,7 +1,7 @@
 /*
  * openvz_driver.c: core driver methods for managing OpenVZ VEs
  *
- * Copyright (C) 2010-2013 Red Hat, Inc.
+ * Copyright (C) 2010-2014 Red Hat, Inc.
  * Copyright (C) 2006, 2007 Binary Karma
  * Copyright (C) 2006 Shuveb Hussain
  * Copyright (C) 2007 Anoop Joe Cyriac
@@ -1710,7 +1710,7 @@ openvzDomainGetBarrierLimit(virDomainPtr domain,
                             unsigned long long *barrier,
                             unsigned long long *limit)
 {
-    int status, ret = -1;
+    int ret = -1;
     char *endp, *output = NULL;
     const char *tmp;
     virCommandPtr cmd = virCommandNewArgList(VZLIST, "--no-header", NULL);
@@ -1718,12 +1718,8 @@ openvzDomainGetBarrierLimit(virDomainPtr domain,
     virCommandSetOutputBuffer(cmd, &output);
     virCommandAddArgFormat(cmd, "-o%s.b,%s.l", param, param);
     virCommandAddArg(cmd, domain->name);
-    if (virCommandRun(cmd, &status) < 0 || status != 0) {
-        virReportError(VIR_ERR_OPERATION_FAILED,
-                       _("Failed to get %s for %s: %d"), param, domain->name,
-                       status);
+    if (virCommandRun(cmd, NULL) < 0)
         goto cleanup;
-    }
 
     tmp = output;
     virSkipSpaces(&tmp);
@@ -1754,7 +1750,7 @@ openvzDomainSetBarrierLimit(virDomainPtr domain,
                             unsigned long long barrier,
                             unsigned long long limit)
 {
-    int status, ret = -1;
+    int ret = -1;
     virCommandPtr cmd = virCommandNewArgList(VZCTL, "--quiet", "set", NULL);
 
     /* LONG_MAX indicates unlimited so reject larger values */
@@ -1769,12 +1765,8 @@ openvzDomainSetBarrierLimit(virDomainPtr domain,
     virCommandAddArgFormat(cmd, "--%s", param);
     virCommandAddArgFormat(cmd, "%llu:%llu", barrier, limit);
     virCommandAddArg(cmd, "--save");
-    if (virCommandRun(cmd, &status) < 0 || status != 0) {
-        virReportError(VIR_ERR_OPERATION_FAILED,
-                       _("Failed to set %s for %s: %d"), param, domain->name,
-                       status);
+    if (virCommandRun(cmd, NULL) < 0)
         goto cleanup;
-    }
 
     ret = 0;
 cleanup:
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 76a8c7359c..cae25e0e1f 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -2693,6 +2693,7 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
     virCommandSetGID(cmd, runGid);
     virCommandSetUID(cmd, runUid);
 
+    /* Log, but otherwise ignore, non-zero status.  */
     if (virCommandRun(cmd, &status) < 0)
         goto cleanup;
 
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 9ee84a0efb..ff0b2d5ec0 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -232,7 +232,6 @@ static int qemuCreateInBridgePortWithHelper(virQEMUDriverConfigPtr cfg,
                                             unsigned int flags)
 {
     virCommandPtr cmd;
-    int status;
     int pair[2] = { -1, -1 };
 
     if ((flags & ~VIR_NETDEV_TAP_CREATE_VNET_HDR) != VIR_NETDEV_TAP_CREATE_IFUP)
@@ -269,7 +268,7 @@ static int qemuCreateInBridgePortWithHelper(virQEMUDriverConfigPtr cfg,
     }
 
     if (virNetDevTapGetName(*tapfd, ifname) < 0 ||
-        virCommandWait(cmd, &status) < 0) {
+        virCommandWait(cmd, NULL) < 0) {
         VIR_FORCE_CLOSE(*tapfd);
         *tapfd = -1;
     }
diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c
index 1149e432a9..3d75215c3f 100644
--- a/src/storage/storage_backend_iscsi.c
+++ b/src/storage/storage_backend_iscsi.c
@@ -1,7 +1,7 @@
 /*
  * storage_backend_iscsi.c: storage backend for iSCSI handling
  *
- * Copyright (C) 2007-2008, 2010-2012 Red Hat, Inc.
+ * Copyright (C) 2007-2014 Red Hat, Inc.
  * Copyright (C) 2007-2008 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
@@ -111,10 +111,6 @@ virStorageBackendISCSISession(virStoragePoolObjPtr pool,
 
     virCommandPtr cmd = virCommandNewArgList(ISCSIADM, "--mode", "session", NULL);
 
-    /* Note that we ignore the exitstatus.  Older versions of iscsiadm tools
-     * returned an exit status of > 0, even if they succeeded.  We will just
-     * rely on whether session got filled in properly.
-     */
     if (virStorageBackendRunProgRegex(pool,
                                       cmd,
                                       1,
@@ -681,6 +677,7 @@ virStorageBackendISCSINodeUpdate(const char *portal,
                                 "--value", value,
                                 NULL);
 
+    /* Ignore non-zero status.  */
     if (virCommandRun(cmd, &status) < 0) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("Failed to update '%s' of node mode for target '%s'"),
diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index a4397b4c90..415b8c3f46 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -113,6 +113,7 @@ struct _virCommand {
     pid_t pid;
     char *pidfile;
     bool reap;
+    bool rawStatus;
 
     unsigned long long maxMemLock;
     unsigned int maxProcesses;
@@ -1120,6 +1121,25 @@ virCommandNonblockingFDs(virCommandPtr cmd)
     cmd->flags |= VIR_EXEC_NONBLOCK;
 }
 
+/**
+ * virCommandRawStatus:
+ * @cmd: the command to modify
+ *
+ * Mark this command as returning raw exit status via virCommandRun() or
+ * virCommandWait() (caller must use WIFEXITED() and friends, and can
+ * detect death from signals) instead of the default of only allowing
+ * normal exit status (caller must not use WEXITSTATUS(), and death from
+ * signals returns -1).
+ */
+void
+virCommandRawStatus(virCommandPtr cmd)
+{
+    if (!cmd || cmd->has_error)
+        return;
+
+    cmd->rawStatus = true;
+}
+
 /* Add an environment variable to the cmd->env list.  'env' is a
  * string like "name=value".  If the named environment variable is
  * already set, then it is replaced in the list.
@@ -2045,7 +2065,11 @@ int virCommandExec(virCommandPtr cmd ATTRIBUTE_UNUSED)
  * Returns -1 on any error executing the
  * command. Returns 0 if the command executed,
  * with the exit status set.  If @exitstatus is NULL, then the
- * child must exit with status 0 for this to succeed.
+ * child must exit with status 0 for this to succeed.  By default,
+ * a non-NULL @exitstatus contains the normal exit status of the child
+ * (death from a signal is treated as execution error); but if
+ * virCommandRawStatus() was used, it instead contains the raw exit
+ * status that the caller must then decipher using WIFEXITED() and friends.
  */
 int
 virCommandRun(virCommandPtr cmd, int *exitstatus)
@@ -2335,7 +2359,11 @@ cleanup:
  * 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.
+ * child must exit with status 0 for this to succeed.  By default,
+ * a non-NULL @exitstatus contains the normal exit status of the child
+ * (death from a signal is treated as execution error); but if
+ * virCommandRawStatus() was used, it instead contains the raw exit
+ * status that the caller must then decipher using WIFEXITED() and friends.
  */
 int
 virCommandWait(virCommandPtr cmd, int *exitstatus)
@@ -2372,7 +2400,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, true);
+    ret = virProcessWait(cmd->pid, &status, true);
     if (cmd->flags & VIR_EXEC_ASYNC_IO) {
         cmd->flags &= ~VIR_EXEC_ASYNC_IO;
         virThreadJoin(cmd->asyncioThread);
@@ -2390,7 +2418,9 @@ virCommandWait(virCommandPtr cmd, int *exitstatus)
     if (ret == 0) {
         cmd->pid = -1;
         cmd->reap = false;
-        if (status) {
+        if (exitstatus && (cmd->rawStatus || WIFEXITED(status))) {
+            *exitstatus = cmd->rawStatus ? status : WEXITSTATUS(status);
+        } else if (status) {
             char *str = virCommandToString(cmd);
             char *st = virProcessTranslateStatus(status);
             bool haveErrMsg = cmd->errbuf && *cmd->errbuf && (*cmd->errbuf)[0];
diff --git a/src/util/vircommand.h b/src/util/vircommand.h
index a7432004f1..ec6185d896 100644
--- a/src/util/vircommand.h
+++ b/src/util/vircommand.h
@@ -86,6 +86,8 @@ void virCommandDaemonize(virCommandPtr cmd);
 
 void virCommandNonblockingFDs(virCommandPtr cmd);
 
+void virCommandRawStatus(virCommandPtr cmd);
+
 void virCommandAddEnvFormat(virCommandPtr cmd, const char *format, ...)
     ATTRIBUTE_NONNULL(2) ATTRIBUTE_FMT_PRINTF(2, 3);
 
diff --git a/src/util/virebtables.c b/src/util/virebtables.c
index 67f281c2e8..ce5e81382f 100644
--- a/src/util/virebtables.c
+++ b/src/util/virebtables.c
@@ -1,7 +1,7 @@
 /*
  * virebtables.c: Helper APIs for managing ebtables
  *
- * Copyright (C) 2007-2013 Red Hat, Inc.
+ * Copyright (C) 2007-2014 Red Hat, Inc.
  * Copyright (C) 2009 IBM Corp.
  *
  * This library is free software; you can redistribute it and/or
@@ -66,10 +66,9 @@ virEbTablesOnceInit(void)
                  "firewalld support disabled for ebtables.");
     } else {
         virCommandPtr cmd = virCommandNew(firewall_cmd_path);
-        int status;
 
         virCommandAddArgList(cmd, "--state", NULL);
-        if (virCommandRun(cmd, &status) < 0 || status != 0) {
+        if (virCommandRun(cmd, NULL) < 0) {
             VIR_INFO("firewall-cmd found but disabled for ebtables");
             VIR_FREE(firewall_cmd_path);
             firewall_cmd_path = NULL;
diff --git a/src/util/viriptables.c b/src/util/viriptables.c
index 9b78d86b6b..9e03cc4014 100644
--- a/src/util/viriptables.c
+++ b/src/util/viriptables.c
@@ -1,7 +1,7 @@
 /*
  * viriptables.c: helper APIs for managing iptables
  *
- * Copyright (C) 2007-2013 Red Hat, Inc.
+ * Copyright (C) 2007-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
@@ -60,7 +60,6 @@ static int
 virIpTablesOnceInit(void)
 {
     virCommandPtr cmd;
-    int status;
 
 #if HAVE_FIREWALLD
     firewall_cmd_path = virFindFileInPath("firewall-cmd");
@@ -71,7 +70,7 @@ virIpTablesOnceInit(void)
         cmd = virCommandNew(firewall_cmd_path);
 
         virCommandAddArgList(cmd, "--state", NULL);
-        if (virCommandRun(cmd, &status) < 0 || status != 0) {
+        if (virCommandRun(cmd, NULL) < 0) {
             VIR_INFO("firewall-cmd found but disabled for iptables");
             VIR_FREE(firewall_cmd_path);
             firewall_cmd_path = NULL;
@@ -88,7 +87,7 @@ virIpTablesOnceInit(void)
 
     cmd = virCommandNew(IPTABLES_PATH);
     virCommandAddArgList(cmd, "-w", "-L", "-n", NULL);
-    if (virCommandRun(cmd, &status) < 0 || status != 0) {
+    if (virCommandRun(cmd, NULL) < 0) {
         VIR_INFO("xtables locking not supported by your iptables");
     } else {
         VIR_INFO("using xtables locking for iptables");
diff --git a/src/util/virnetdevveth.c b/src/util/virnetdevveth.c
index e698ce2c83..62d4774fa2 100644
--- a/src/util/virnetdevveth.c
+++ b/src/util/virnetdevveth.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2010-2013 Red Hat, Inc.
+ * Copyright (C) 2010-2014 Red Hat, Inc.
  * Copyright IBM Corp. 2008
  *
  * This library is free software; you can redistribute it and/or
@@ -183,7 +183,7 @@ int virNetDevVethCreate(char** veth1, char** veth2)
 
         VIR_DEBUG("Failed to create veth host: %s guest: %s: %d",
                   *veth1 ? *veth1 : veth1auto,
-                  *veth1 ? *veth1 : veth1auto,
+                  *veth2 ? *veth2 : veth2auto,
                   status);
         VIR_FREE(veth1auto);
         VIR_FREE(veth2auto);
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index 7506e8cc04..ee05fb41e2 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -308,16 +308,15 @@ xenUnifiedProbe(void)
 }
 
 #ifdef WITH_LIBXL
-static int
+static bool
 xenUnifiedXendProbe(void)
 {
     virCommandPtr cmd;
-    int status;
-    int ret = 0;
+    bool ret = false;
 
     cmd = virCommandNewArgList("/usr/sbin/xend", "status", NULL);
-    if (virCommandRun(cmd, &status) == 0 && status == 0)
-        ret = 1;
+    if (virCommandRun(cmd, NULL) == 0)
+        ret = true;
     virCommandFree(cmd);
 
     return ret;
diff --git a/tests/commandtest.c b/tests/commandtest.c
index c0391a5e1e..b329bfb90c 100644
--- a/tests/commandtest.c
+++ b/tests/commandtest.c
@@ -139,6 +139,12 @@ static int test1(const void *unused ATTRIBUTE_UNUSED)
     int status;
 
     cmd = virCommandNew(abs_builddir "/commandhelper-doesnotexist");
+    if (virCommandRun(cmd, &status) < 0)
+        goto cleanup;
+    if (status != EXIT_ENOENT)
+        goto cleanup;
+
+    virCommandRawStatus(cmd);
     if (virCommandRun(cmd, &status) < 0)
         goto cleanup;
     if (!WIFEXITED(status) || WEXITSTATUS(status) != EXIT_ENOENT)
@@ -909,6 +915,17 @@ test22(const void *unused ATTRIBUTE_UNUSED)
 
     cmd = virCommandNewArgList("/bin/sh", "-c", "exit 3", NULL);
 
+    if (virCommandRun(cmd, &status) < 0) {
+        virErrorPtr err = virGetLastError();
+        printf("Cannot run child %s\n", err->message);
+        goto cleanup;
+    }
+    if (status != 3) {
+        printf("Unexpected status %d\n", status);
+        goto cleanup;
+    }
+
+    virCommandRawStatus(cmd);
     if (virCommandRun(cmd, &status) < 0) {
         virErrorPtr err = virGetLastError();
         printf("Cannot run child %s\n", err->message);
@@ -922,6 +939,12 @@ test22(const void *unused ATTRIBUTE_UNUSED)
     virCommandFree(cmd);
     cmd = virCommandNewArgList("/bin/sh", "-c", "kill -9 $$", NULL);
 
+    if (virCommandRun(cmd, &status) == 0) {
+        printf("Death by signal not detected, status %d\n", status);
+        goto cleanup;
+    }
+
+    virCommandRawStatus(cmd);
     if (virCommandRun(cmd, &status) < 0) {
         virErrorPtr err = virGetLastError();
         printf("Cannot run child %s\n", err->message);
diff --git a/tests/reconnect.c b/tests/reconnect.c
index 09deb5d106..f0779adbba 100644
--- a/tests/reconnect.c
+++ b/tests/reconnect.c
@@ -15,7 +15,6 @@ mymain(void)
     bool ro = false;
     virConnectPtr conn;
     virDomainPtr dom;
-    int status;
     virCommandPtr cmd;
     struct utsname ut;
 
@@ -26,7 +25,7 @@ mymain(void)
     if (strstr(ut.release, "xen") == NULL)
         return EXIT_AM_SKIP;
     cmd = virCommandNewArgList("/usr/sbin/xend", "status", NULL);
-    if (virCommandRun(cmd, &status) != 0 || status != 0) {
+    if (virCommandRun(cmd, NULL) < 0) {
         virCommandFree(cmd);
         return EXIT_AM_SKIP;
     }
diff --git a/tests/statstest.c b/tests/statstest.c
index 441cedb0c6..7af152a49f 100644
--- a/tests/statstest.c
+++ b/tests/statstest.c
@@ -40,7 +40,6 @@ static int
 mymain(void)
 {
     int ret = 0;
-    int status;
     virCommandPtr cmd;
     struct utsname ut;
 
@@ -51,7 +50,7 @@ mymain(void)
     if (strstr(ut.release, "xen") == NULL)
         return EXIT_AM_SKIP;
     cmd = virCommandNewArgList("/usr/sbin/xend", "status", NULL);
-    if (virCommandRun(cmd, &status) != 0 || status != 0) {
+    if (virCommandRun(cmd, NULL) < 0) {
         virCommandFree(cmd);
         return EXIT_AM_SKIP;
     }