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

[libvirt] [PATCH 06/10] 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>
---
 daemon/remote.c                     |  7 +++----
 docs/internals/command.html.in      | 17 +++++++++++++----
 src/access/viraccessdriverpolkit.c  |  9 ++++-----
 src/bhyve/bhyve_process.c           | 19 +++----------------
 src/fdstream.c                      |  3 ++-
 src/libvirt.c                       |  4 +---
 src/libvirt_private.syms            |  1 +
 src/lxc/lxc_process.c               | 11 +++++++++--
 src/openvz/openvz_driver.c          | 18 +++++-------------
 src/qemu/qemu_capabilities.c        |  1 +
 src/qemu/qemu_command.c             |  3 +--
 src/storage/storage_backend_iscsi.c |  7 ++-----
 src/util/vircommand.c               | 38 +++++++++++++++++++++++++++++++++----
 src/util/vircommand.h               |  2 ++
 src/util/virebtables.c              |  5 ++---
 src/util/viriptables.c              |  7 +++----
 src/util/virnetdevveth.c            |  4 ++--
 src/xen/xen_driver.c                |  9 ++++-----
 tests/commandtest.c                 | 23 ++++++++++++++++++++++
 tests/reconnect.c                   |  3 +--
 tests/statstest.c                   |  3 +--
 21 files changed, 117 insertions(+), 77 deletions(-)

diff --git a/daemon/remote.c b/daemon/remote.c
index 932f65f..b48d456 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 0336e65..f5c28af 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 c325739..d9ebc49 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 7717fec..ee85680 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 f7dae39..04d62b8 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 f38556a..f5d85db 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 d7a9ee7..9b27043 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1102,6 +1102,7 @@ virCommandNewArgList;
 virCommandNewArgs;
 virCommandNonblockingFDs;
 virCommandPassFD;
+virCommandRawStatus;
 virCommandRequireHandshake;
 virCommandRun;
 virCommandRunAsync;
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 8989245..ccd1efb 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -1199,12 +1199,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 b27ac4c..393f397 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 d618b3f..22a946d 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 9ee84a0..ff0b2d5 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 556c2cc..5479663 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 a4397b4..415b8c3 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 a743200..ec6185d 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 67f281c..ce5e813 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 9b78d86..9e03cc4 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 25eb282..18cc1eb 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
@@ -166,7 +166,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 7506e8c..ee05fb4 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 c0391a5..b329bfb 100644
--- a/tests/commandtest.c
+++ b/tests/commandtest.c
@@ -141,6 +141,12 @@ static int test1(const void *unused ATTRIBUTE_UNUSED)
     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)
         goto cleanup;
     ret = 0;
@@ -914,6 +920,17 @@ test22(const void *unused ATTRIBUTE_UNUSED)
         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);
+        goto cleanup;
+    }
     if (!WIFEXITED(status) || WEXITSTATUS(status) != 3) {
         printf("Unexpected status %d\n", status);
         goto cleanup;
@@ -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 09deb5d..f0779ad 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 441cedb..7af152a 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;
     }
-- 
1.8.5.3


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