snapshot: Don't hose list on deletion failure

If qemuDomainSnapshotDiscard() fails for any reason (rare,
but possible with an ill-timed ENOMEM or if
qemuDomainSnapshotForEachQcow2() has problems talking to the
qemu guest monitor), then an attempt to retry the snapshot
deletion API will crash because we didn't undo the effects
of virDomainSnapshotDropParent() temporarily rearranging the
internal list structures, and the second attempt to drop
parents will dereference NULL.  Fix it by instead noting that
there are only two callers to qemuDomainSnapshotDiscard(),
and only one of the two callers wants the parent to be updated;
thus we can move the call to virDomainSnapshotDropParent()
into a code path that only gets executed on success.

Signed-off-by: Eric Blake <eblake@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 045a7b4ac0a1374d6ff7a950827be186cd005bda..e25afdad6bb27526e488c8e986d4bc8ca5485faf 100644 (file)
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -8249,7 +8249,7 @@ int
 qemuDomainSnapshotDiscard(virQEMUDriverPtr driver,
                           virDomainObjPtr vm,
                           virDomainSnapshotObjPtr snap,
-                          bool update_current,
+                          bool update_parent,
                           bool metadata_only)
 {
     char *snapFile = NULL;
@@ -8278,7 +8278,7 @@ qemuDomainSnapshotDiscard(virQEMUDriverPtr driver,
         goto cleanup;
 
     if (snap == vm->current_snapshot) {
-        if (update_current && snap->def->parent) {
+        if (update_parent && snap->def->parent) {
             parentsnap = virDomainSnapshotFindByName(vm->snapshots,
                                                      snap->def->parent);
             if (!parentsnap) {
@@ -8301,6 +8301,8 @@ qemuDomainSnapshotDiscard(virQEMUDriverPtr driver,
 
     if (unlink(snapFile) < 0)
         VIR_WARN("Failed to unlink %s", snapFile);
+    if (update_parent)
+        virDomainSnapshotDropParent(snap);
     virDomainSnapshotObjListRemove(vm->snapshots, snap);
 
     ret = 0;

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index a52e2495d51da73a3ff0678432b1982c2703a3d3..9f71641dfa133df339157b10958cf5c18c0baa3b 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16526,7 +16526,6 @@ qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot,
         snap->first_child = NULL;
         ret = 0;
     } else {
-        virDomainSnapshotDropParent(snap);
         ret = qemuDomainSnapshotDiscard(driver, vm, snap, true, metadata_only);
     }