[libvirt] [PATCH] Fix virsh snapshot-list error reporting
Noticed that the expected "not supported" error is dropped when invoking 'virsh snapshot-list dom' on a Xen installation running the libxl driver virsh snapshot-list test error: Invalid snapshot: virDomainSnapshotFree The error is overwritten by a call to virDomainSnapshotFree in cleanup code within cmdSnapshotList. Prevent overwritting the real error by not calling virDomainSnapshotFree with a NULL virDomainSnapshotPtr. --- This is one possible fix for the bug. The other would be to silently return in virDomainSnapshotFree on a NULL virDomainSnapshotPtr. tools/virsh-snapshot.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c index cfe8ee9..db9715b 100644 --- a/tools/virsh-snapshot.c +++ b/tools/virsh-snapshot.c @@ -1678,7 +1678,8 @@ cleanup: vshSnapshotListFree(snaplist); VIR_FREE(parent_snap); VIR_FREE(state); - virDomainSnapshotFree(start); + if (start) + virDomainSnapshotFree(start); xmlXPathFreeContext(ctxt); xmlFreeDoc(xml); VIR_FREE(doc); -- 1.8.1.4
On 07/25/2013 02:38 PM, Jim Fehlig wrote:
Noticed that the expected "not supported" error is dropped when invoking 'virsh snapshot-list dom' on a Xen installation running the libxl driver
virsh snapshot-list test error: Invalid snapshot: virDomainSnapshotFree
The error is overwritten by a call to virDomainSnapshotFree in cleanup code within cmdSnapshotList. Prevent overwritting the real error by not calling virDomainSnapshotFree with a NULL virDomainSnapshotPtr.
Indeed.
---
This is one possible fix for the bug. The other would be to silently return in virDomainSnapshotFree on a NULL virDomainSnapshotPtr.
No, that would be an incompatible behavior change - our API promises a failure on attempting to free a null object, and people have come to rely on that.
+++ b/tools/virsh-snapshot.c @@ -1678,7 +1678,8 @@ cleanup: vshSnapshotListFree(snaplist); VIR_FREE(parent_snap); VIR_FREE(state); - virDomainSnapshotFree(start); + if (start) + virDomainSnapshotFree(start);
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Eric Blake wrote:
On 07/25/2013 02:38 PM, Jim Fehlig wrote:
Noticed that the expected "not supported" error is dropped when invoking 'virsh snapshot-list dom' on a Xen installation running the libxl driver
virsh snapshot-list test error: Invalid snapshot: virDomainSnapshotFree
The error is overwritten by a call to virDomainSnapshotFree in cleanup code within cmdSnapshotList. Prevent overwritting the real error by not calling virDomainSnapshotFree with a NULL virDomainSnapshotPtr.
Indeed.
---
This is one possible fix for the bug. The other would be to silently return in virDomainSnapshotFree on a NULL virDomainSnapshotPtr.
No, that would be an incompatible behavior change - our API promises a failure on attempting to free a null object, and people have come to rely on that.
Ah, yes, good point.
+++ b/tools/virsh-snapshot.c @@ -1678,7 +1678,8 @@ cleanup: vshSnapshotListFree(snaplist); VIR_FREE(parent_snap); VIR_FREE(state); - virDomainSnapshotFree(start); + if (start) + virDomainSnapshotFree(start);
ACK.
Thanks, pushed now. Regards, Jim
participants (2)
-
Eric Blake -
Jim Fehlig