From 68b2596f83c044b67b9bae8cde6f3cbd72a776f2 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Wed, 17 Oct 2018 19:24:34 -0500 Subject: [PATCH] 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 ACKed-by: Michal Privoznik --- src/qemu/qemu_domain.c | 6 ++++-- src/qemu/qemu_driver.c | 1 - 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 045a7b4ac0..e25afdad6b 100644 --- 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 a52e2495d5..9f71641dfa 100644 --- 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); }