virsh: Refactor error clearing on graceful fallback paths

Virsh uses an error handler to save errors from libvirt. On some code
paths it's needed to clear libvirt errors and continue on fallback code
paths without reporting failure.

This patch adds function vshResetLibvirtError() that clears error
returned by libvirt and updates all places where the old two-line method
was used.

diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 68d5983d0b0bd77f6fb2060d0545e7906d4c9e8f..65a9808d16a13c768e038bcb45f2da4c938803e6 100644 (file)
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -849,8 +849,7 @@ cmdDomblkstat(vshControl *ctl, const vshCmd *cmd)
         if (last_error->code != VIR_ERR_NO_SUPPORT)
             goto cleanup;
 
-        virFreeError(last_error);
-        last_error = NULL;
+        vshResetLibvirtError();
 
         if (virDomainBlockStats(dom, device, &stats,
                                 sizeof(stats)) == -1) {
@@ -1166,8 +1165,7 @@ cmdDominfo(vshControl *ctl, const vshCmd *cmd)
             virDomainFree(dom);
             return false;
         } else {
-            virFreeError(last_error);
-            last_error = NULL;
+            vshResetLibvirtError();
         }
     } else {
         /* Only print something if a security model is active */
@@ -1340,8 +1338,7 @@ vshDomainListCollect(vshControl *ctl, unsigned int flags)
 
     /* check if the command is actually supported */
     if (last_error && last_error->code == VIR_ERR_NO_SUPPORT) {
-        virFreeError(last_error);
-        last_error = NULL;
+        vshResetLibvirtError();
         goto fallback;
     }
 
@@ -1350,8 +1347,7 @@ vshDomainListCollect(vshControl *ctl, unsigned int flags)
         unsigned int newflags = flags & (VIR_CONNECT_LIST_DOMAINS_ACTIVE |
                                          VIR_CONNECT_LIST_DOMAINS_INACTIVE);
 
-        virFreeError(last_error);
-        last_error = NULL;
+        vshResetLibvirtError();
         if ((ret = virConnectListAllDomains(ctl->conn, &list->domains,
                                             newflags)) >= 0) {
             list->ndomains = ret;

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index cb77ca45d171011004a9d00e67f2846cd96a5f90..33b1727f9486f68edfd6644c5b3625f4ef89d510 100644 (file)
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -2332,8 +2332,7 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd)
         if (has_managed_save < 0) {
             if (last_error->code != VIR_ERR_NO_SUPPORT)
                 goto error;
-            virFreeError(last_error);
-            last_error = NULL;
+            vshResetLibvirtError();
             has_managed_save = 0;
         }
 
@@ -2341,8 +2340,7 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd)
         if (has_snapshots < 0) {
             if (last_error->code != VIR_ERR_NO_SUPPORT)
                 goto error;
-            virFreeError(last_error);
-            last_error = NULL;
+            vshResetLibvirtError();
             has_snapshots = 0;
         }
         if (has_snapshots) {
@@ -2351,8 +2349,7 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd)
             if (has_snapshots_metadata < 0) {
                 /* The server did not know the new flag, assume that all
                    snapshots have metadata.  */
-                virFreeError(last_error);
-                last_error = NULL;
+                vshResetLibvirtError();
                 has_snapshots_metadata = has_snapshots;
             } else {
                 /* The server knew the new flag, all aspects of
@@ -2461,8 +2458,7 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd)
                 vshPrint(ctl,
                          _("Storage volume '%s'(%s) is not managed by libvirt. "
                            "Remove it manually.\n"), target, source);
-                virFreeError(last_error);
-                last_error = NULL;
+                vshResetLibvirtError();
                 continue;
             }
             vlist[nvols].source = source;
@@ -2497,8 +2493,7 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd)
         if (rc == 0 || (last_error->code != VIR_ERR_NO_SUPPORT &&
                         last_error->code != VIR_ERR_INVALID_ARG))
             goto out;
-        virFreeError(last_error);
-        last_error = NULL;
+        vshResetLibvirtError();
     }
 
     /* The new API is unsupported or unsafe; fall back to doing things
@@ -2657,13 +2652,11 @@ cmdStart(vshControl *ctl, const vshCmd *cmd)
             virshReportError(ctl);
             goto cleanup;
         }
-        virFreeError(last_error);
-        last_error = NULL;
+        vshResetLibvirtError();
         rc = virDomainHasManagedSaveImage(dom, 0);
         if (rc < 0) {
             /* No managed save image to remove */
-            virFreeError(last_error);
-            last_error = NULL;
+            vshResetLibvirtError();
         } else if (rc > 0) {
             if (virDomainManagedSaveRemove(dom, 0) < 0) {
                 virshReportError(ctl);
@@ -4173,8 +4166,7 @@ cmdVcpucount(vshControl *ctl, const vshCmd *cmd)
                 if (!tmp || virStrToLong_i(tmp + 1, &tmp, 10, &count) < 0)
                     count = -1;
             }
-            virFreeError(last_error);
-            last_error = NULL;
+            vshResetLibvirtError();
             VIR_FREE(xml);
         }
 
@@ -4187,8 +4179,7 @@ cmdVcpucount(vshControl *ctl, const vshCmd *cmd)
         } else {
             vshPrint(ctl, "%d\n", count);
         }
-        virFreeError(last_error);
-        last_error = NULL;
+        vshResetLibvirtError();
     }
 
     if (all || (maximum && live)) {
@@ -4208,8 +4199,7 @@ cmdVcpucount(vshControl *ctl, const vshCmd *cmd)
         } else {
             vshPrint(ctl, "%d\n", count);
         }
-        virFreeError(last_error);
-        last_error = NULL;
+        vshResetLibvirtError();
     }
 
     if (all || (active && config)) {
@@ -4245,8 +4235,7 @@ cmdVcpucount(vshControl *ctl, const vshCmd *cmd)
         } else {
             vshPrint(ctl, "%d\n", count);
         }
-        virFreeError(last_error);
-        last_error = NULL;
+        vshResetLibvirtError();
     }
 
     if (all || (active && live)) {
@@ -4267,8 +4256,7 @@ cmdVcpucount(vshControl *ctl, const vshCmd *cmd)
         } else {
             vshPrint(ctl, "%d\n", count);
         }
-        virFreeError(last_error);
-        last_error = NULL;
+        vshResetLibvirtError();
     }
 
     virDomainFree(dom);

diff --git a/tools/virsh-network.c b/tools/virsh-network.c
index 69a766db17fb4cf59c73f0c9568a4bc58d7ea40f..49ec34f286da02b92631616b3aee7f378b42bae9 100644 (file)
--- a/tools/virsh-network.c
+++ b/tools/virsh-network.c
@@ -639,8 +639,7 @@ static char *vshNetworkGetXMLDesc(virNetworkPtr network)
         /* The server side libvirt doesn't support
          * VIR_NETWORK_XML_INACTIVE, so retry without it.
          */
-        virFreeError(last_error);
-        last_error = NULL;
+        vshResetLibvirtError();
         flags &= ~VIR_NETWORK_XML_INACTIVE;
         doc = virNetworkGetXMLDesc(network, flags);
     }

diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c
index ccd9b79bac87130ee07b0f40250e8744a3f43ee4..af80427458de9372638668a3012257d34ffe44e0 100644 (file)
--- a/tools/virsh-pool.c
+++ b/tools/virsh-pool.c
@@ -1379,8 +1379,7 @@ cmdPoolEdit(vshControl *ctl, const vshCmd *cmd)
     if (!(tmp_desc = virStoragePoolGetXMLDesc(pool, flags))) {
         if (last_error->code == VIR_ERR_INVALID_ARG) {
             flags &= ~VIR_STORAGE_XML_INACTIVE;
-            virFreeError(last_error);
-            last_error = NULL;
+            vshResetLibvirtError();
         } else {
             goto cleanup;
         }

diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
index 9182c9aa1e6ebd505396b650b2fe39cdd0ba72a3..92f0d6ce380ba59c0f086dc2460e909b8d098db0 100644 (file)
--- a/tools/virsh-snapshot.c
+++ b/tools/virsh-snapshot.c
@@ -43,8 +43,7 @@ vshSnapshotCreate(vshControl *ctl, virDomainPtr dom, const char *buffer,
         (flags & VIR_DOMAIN_SNAPSHOT_CREATE_HALT)) {
         int persistent;
 
-        virFreeError(last_error);
-        last_error = NULL;
+        vshResetLibvirtError();
         persistent = virDomainIsPersistent(dom);
         if (persistent < 0) {
             virshReportError(ctl);
@@ -644,8 +643,7 @@ cleanup:
         virshReportError(ctl);
         vshError(ctl, "%s", _("unable to determine if snapshot has parent"));
     } else {
-        virFreeError(last_error);
-        last_error = NULL;
+        vshResetLibvirtError();
     }
     if (parent)
         virDomainSnapshotFree(parent);
@@ -932,8 +930,7 @@ vshSnapshotListCollect(vshControl *ctl, virDomainPtr dom,
              last_error->code == VIR_ERR_NO_SUPPORT)) {
             /* We can emulate --from.  */
             /* XXX can we also emulate --leaves? */
-            virFreeError(last_error);
-            last_error = NULL;
+            vshResetLibvirtError();
             ctl->useSnapshotOld = true;
             flags &= ~VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS;
             goto global;
@@ -950,8 +947,7 @@ vshSnapshotListCollect(vshControl *ctl, virDomainPtr dom,
         /* XXX can we also emulate --leaves? */
         if (!from && count < 0 && last_error->code == VIR_ERR_INVALID_ARG &&
             (flags & VIR_DOMAIN_SNAPSHOT_LIST_ROOTS)) {
-            virFreeError(last_error);
-            last_error = NULL;
+            vshResetLibvirtError();
             roots = true;
             flags &= ~VIR_DOMAIN_SNAPSHOT_LIST_ROOTS;
             count = virDomainSnapshotNum(dom, flags);
@@ -1514,8 +1510,7 @@ cmdDomainSnapshotRevert(vshControl *ctl, const vshCmd *cmd)
     if (result < 0 && force &&
         last_error->code == VIR_ERR_SNAPSHOT_REVERT_RISKY) {
         flags |= VIR_DOMAIN_SNAPSHOT_REVERT_FORCE;
-        virFreeError(last_error);
-        last_error = NULL;
+        vshResetLibvirtError();
         result = virDomainRevertToSnapshot(snapshot, flags);
     }
     if (result < 0)

diff --git a/tools/virsh.c b/tools/virsh.c
index 4f8ea941e98c4b320d4212ab22c3228237ba2e5c..953b75287fe91770149637e37cb8fe2d754ee1e4 100644 (file)
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -491,6 +491,16 @@ virshErrorHandler(void *unused ATTRIBUTE_UNUSED, virErrorPtr error)
         virDefaultErrorFunc(error);
 }
 
+/*
+ * Reset libvirt error on graceful fallback paths
+ */
+static void
+vshResetLibvirtError(void)
+{
+    virFreeError(last_error);
+    last_error = NULL;
+}
+
 /*
  * Report an error when a command finishes.  This is better than before
  * (when correct operation would report errors), but it has some
@@ -521,8 +531,7 @@ virshReportError(vshControl *ctl)
     vshError(ctl, "%s", last_error->message);
 
 out:
-    virFreeError(last_error);
-    last_error = NULL;
+    vshResetLibvirtError();
 }
 
 static volatile sig_atomic_t intCaught = 0;