7:48 a.m.
This series fixes places where errors were not reset correctly and adds a
function for resetting libvirt errors in virsh that simplifies this operation.
v2 is a rebase of the previous version as it conflicted with the rewrite of
cmdUndefine.
Peter Krempa (2):
virsh: Refactor error clearing on graceful fallback paths
virsh: Fix error resetting on fallback paths
tools/virsh-domain-monitor.c | 16 ++++++----------
tools/virsh-domain.c | 36 ++++++++++++------------------------
tools/virsh-network.c | 3 +--
tools/virsh-pool.c | 3 +--
tools/virsh-snapshot.c | 19 +++++++------------
tools/virsh.c | 13 +++++++++++--
6 files changed, 38 insertions(+), 52 deletions(-)
--
1.7.8.6
7:48 a.m.
New subject: [libvirt] [PATCHv2 1/2] 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 68d5983..65a9808 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 cb77ca4..33b1727 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 69a766d..49ec34f 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 ccd9b79..af80427 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 9182c9a..92f0d6c 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 5658796..15a0dc7 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -492,6 +492,16 @@ virshErrorHandler(void *unused ATTRIBUTE_UNUSED, virErrorPtr 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
* problems: we lose the smarter formatting of virDefaultErrorFunc(),
@@ -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;
--
1.7.8.6
3:19 a.m.
New subject: [libvirt] [PATCHv2 1/2] virsh: Refactor error clearing on graceful fallback paths
On 07/26/2012 02:48 PM, Peter Krempa wrote:
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.
ACK,
Martin
7:48 a.m.
New subject: [libvirt] [PATCHv2 2/2] virsh: Fix error resetting on fallback paths
On some fallback paths in virsh, error reported by the previously failed
API is cleared by virResetLastError() that doesn'd free error stored by
virsh. This patch changes this to clear it using vshResetLibvirtError().
---
tools/virsh-domain-monitor.c | 4 ++--
tools/virsh-snapshot.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 65a9808..151a8d0 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -66,7 +66,7 @@ vshGetDomainDescription(vshControl *ctl, virDomainPtr dom, bool title,
if (err && err->code == VIR_ERR_NO_DOMAIN_METADATA) {
desc = vshStrdup(ctl, "");
- virResetLastError();
+ vshResetLibvirtError();
return desc;
}
@@ -1362,7 +1362,7 @@ vshDomainListCollect(vshControl *ctl, unsigned int flags)
fallback:
/* fall back to old method (0.9.12 and older) */
- virResetLastError();
+ vshResetLibvirtError();
/* list active domains, if necessary */
if (!MATCH(VIR_CONNECT_LIST_FILTERS_ACTIVE) ||
diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
index 92f0d6c..d6de3da 100644
--- a/tools/virsh-snapshot.c
+++ b/tools/virsh-snapshot.c
@@ -704,7 +704,7 @@ cmdSnapshotInfo(vshControl *ctl, const vshCmd *cmd)
if (current < 0) {
virDomainSnapshotPtr other = virDomainSnapshotCurrent(dom, 0);
- virResetLastError();
+ vshResetLibvirtError();
current = 0;
if (other) {
if (STREQ(name, virDomainSnapshotGetName(other)))
@@ -759,7 +759,7 @@ cmdSnapshotInfo(vshControl *ctl, const vshCmd *cmd)
if (metadata < 0) {
metadata = virDomainSnapshotNum(dom,
VIR_DOMAIN_SNAPSHOT_LIST_METADATA);
- virResetLastError();
+ vshResetLibvirtError();
}
if (metadata >= 0)
vshPrint(ctl, "%-15s %s\n", _("Metadata:"),
--
1.7.8.6
3:21 a.m.
New subject: [libvirt] [PATCHv2 2/2] virsh: Fix error resetting on fallback paths
On 07/26/2012 02:48 PM, Peter Krempa wrote:
On some fallback paths in virsh, error reported by the previously
failed
API is cleared by virResetLastError() that doesn'd free error stored by
s/doesn'd/doesn't/
virsh. This patch changes this to clear it using
vshResetLibvirtError().
ACK,
Martin
3:42 a.m.
New subject: [libvirt] [PATCHv2 2/2] virsh: Fix error resetting on fallback paths
On 07/27/12 10:21, Martin Kletzander wrote:
On 07/26/2012 02:48 PM, Peter Krempa wrote:
> On some fallback paths in virsh, error reported by the previously failed
> API is cleared by virResetLastError() that doesn'd free error stored by
s/doesn'd/doesn't/
> virsh. This patch changes this to clear it using vshResetLibvirtError().
ACK,
Martin
I fixed the commit message and pushed the series. Thanks!
Peter
4501
days inactive
4502
days old
5 comments
2 participants
participants (2)
-
Martin Kletzander -
Peter Krempa