[PATCH] snapshot: one less point of failure in qemu

[Eric Blake <eblake@xxxxxxxxxx>]

https://bugzilla.redhat.com/show_bug.cgi?id=727709
mentions that if qemu fails to create the snapshot (such as what
happens on Fedora 15 qemu, which has qmp but where savevm is only
in hmp, and where libvirt is old enough to not try the hmp fallback),
then 'virsh snapshot-list dom' will show a garbage snapshot entry,
and the libvirt internal directory for storing snapshot metadata.

This fixes the fallout bug of polluting the snapshot-list with
garbage on failure (the root cause of the F15 bug of not having
fallback to hmp has already been fixed in newer libvirt releases).

* src/qemu/qemu_driver.c (qemuDomainSnapshotCreateXML): Allocate
memory before making snapshot, and cleanup on failure.
---
 src/qemu/qemu_driver.c |   23 ++++++++++++++---------
 1 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7802e08..da2703e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8665,7 +8665,7 @@ static virDomainSnapshotPtr qemuDomainSnapshotCreateXML(virDomainPtr domain,
     virDomainSnapshotObjPtr snap = NULL;
     virDomainSnapshotPtr snapshot = NULL;
     char uuidstr[VIR_UUID_STRING_BUFLEN];
-    virDomainSnapshotDefPtr def;
+    virDomainSnapshotDefPtr def = NULL;

     virCheckFlags(0, NULL);

@@ -8695,6 +8695,13 @@ static virDomainSnapshotPtr qemuDomainSnapshotCreateXML(virDomainPtr domain,
         goto cleanup;

     snap->def->state = virDomainObjGetState(vm, NULL);
+    if (vm->current_snapshot) {
+        def->parent = strdup(vm->current_snapshot->def->name);
+        if (def->parent == NULL) {
+            virReportOOMError();
+            goto cleanup;
+        }
+    }

     /* actually do the snapshot */
     if (!virDomainObjIsActive(vm)) {
@@ -8711,14 +8718,6 @@ static virDomainSnapshotPtr qemuDomainSnapshotCreateXML(virDomainPtr domain,
      * on it, so we have to go forward the best we can
      */

-    if (vm->current_snapshot) {
-        def->parent = strdup(vm->current_snapshot->def->name);
-        if (def->parent == NULL) {
-            virReportOOMError();
-            goto cleanup;
-        }
-    }
-
     /* Now we set the new current_snapshot for the domain */
     vm->current_snapshot = snap;

@@ -8732,6 +8731,12 @@ static virDomainSnapshotPtr qemuDomainSnapshotCreateXML(virDomainPtr domain,
 cleanup:
     if (vm)
         virDomainObjUnlock(vm);
+    if (!snapshot) {
+        if (snap)
+            virDomainSnapshotObjListRemove(&vm->snapshots, snap);
+        else
+            virDomainSnapshotDefFree(def);
+    }
     qemuDriverUnlock(driver);
     return snapshot;
 }
-- 
1.7.4.4

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list