From 3df9626c404573d94580a2711e2d7a3e1aef7800 Mon Sep 17 00:00:00 2001 From: Peter Krempa Date: Wed, 25 Jul 2012 13:41:49 +0200 Subject: [PATCH] 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. --- tools/virsh-domain-monitor.c | 12 ++++-------- tools/virsh-domain.c | 36 ++++++++++++------------------------ tools/virsh-network.c | 3 +-- tools/virsh-pool.c | 3 +-- tools/virsh-snapshot.c | 15 +++++---------- tools/virsh.c | 13 +++++++++++-- 6 files changed, 34 insertions(+), 48 deletions(-) diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index 68d5983d0b..65a9808d16 100644 --- 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 cb77ca45d1..33b1727f94 100644 --- 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 69a766db17..49ec34f286 100644 --- 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 ccd9b79bac..af80427458 100644 --- 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 9182c9aa1e..92f0d6ce38 100644 --- 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 4f8ea941e9..953b75287f 100644 --- 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; -- 2.39.5