Deleting a snapshot and all its descendants had problems with tracking the current snapshot. The deletion does not necessarily proceed in depth-first order, so a parent could be deleted before a child, wreaking havoc on passing the notion of the current snapshot to the parent. Furthermore, even if traversal were depth-first, doing multiple file writes to pass current up the chain one snapshot at a time is wasteful, comparing to a single update to the current snapshot at the end of the algorithm. * src/qemu/qemu_driver.c (snap_remove): Add field. (qemuDomainSnapshotDiscard): Add parameter. (qemuDomainSnapshotDiscardDescendant): Adjust accordingly. (qemuDomainSnapshotDelete): Properly reset current. --- This patch should be applied immediately after 8/43 to fix one other bug with snapshot deletion that I noticed in the process of fixing the nested hash iteration. It will cause a few patches later in the series to have a merge conflict if you apply them via 'git am', but my git repository (see 0/43) has been rebased appropriately. After adding 7.5/43, 7.9/43, and 8.5/43 in the right places, I was successfully able to do: virsh> snapshot-create-as dom s1 virsh> snapshot-create-as dom s2 virsh> snapshot-create-as dom s3 virsh> snapshot-create-as dom s4 virsh> snapshot-delete --children dom s2 virsh> snapshot-current --name dom s1 virsh> snapshot-list --parent dom correct listing with just s1 left A note of caution: I tested with snapshots named 's1' instead of '1'; because I just discovered another libvirt bug that I just filed: https://bugzilla.redhat.com/show_bug.cgi?id=733143 Namely, qemu-img prefers deleting snapshot ids over snapshot tags, so if your tags are simple integers but do not line up with ids, then you risk libvirt deleting the wrong snapshot data. src/qemu/qemu_driver.c | 20 ++++++++++++++------ 1 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1405b71..c959e58 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8940,9 +8940,11 @@ cleanup: return ret; } -static int qemuDomainSnapshotDiscard(struct qemud_driver *driver, - virDomainObjPtr vm, - virDomainSnapshotObjPtr snap) +static int +qemuDomainSnapshotDiscard(struct qemud_driver *driver, + virDomainObjPtr vm, + virDomainSnapshotObjPtr snap, + bool update_current) { const char *qemuimgarg[] = { NULL, "snapshot", "-d", NULL, NULL, NULL }; char *snapFile = NULL; @@ -8995,7 +8997,7 @@ static int qemuDomainSnapshotDiscard(struct qemud_driver *driver, } if (snap == vm->current_snapshot) { - if (snap->def->parent) { + if (update_current && snap->def->parent) { parentsnap = virDomainSnapshotFindByName(&vm->snapshots, snap->def->parent); if (!parentsnap) { @@ -9032,6 +9034,7 @@ struct snap_remove { struct qemud_driver *driver; virDomainObjPtr vm; int err; + bool current; }; static void @@ -9043,7 +9046,9 @@ qemuDomainSnapshotDiscardDescendant(void *payload, struct snap_remove *curr = data; int err; - err = qemuDomainSnapshotDiscard(curr->driver, curr->vm, snap); + if (snap->def->current) + curr->current = true; + err = qemuDomainSnapshotDiscard(curr->driver, curr->vm, snap, false); if (err && !curr->err) curr->err = err; } @@ -9122,12 +9127,15 @@ static int qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot, rem.driver = driver; rem.vm = vm; rem.err = 0; + rem.current = false; virDomainSnapshotForEachDescendant(&vm->snapshots, snap, qemuDomainSnapshotDiscardDescendant, &rem); if (rem.err < 0) goto endjob; + if (rem.current) + vm->current_snapshot = snap; } else { rep.driver = driver; rep.snap = snap; @@ -9139,7 +9147,7 @@ static int qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot, goto endjob; } - ret = qemuDomainSnapshotDiscard(driver, vm, snap); + ret = qemuDomainSnapshotDiscard(driver, vm, snap, true); endjob: if (qemuDomainObjEndJob(driver, vm) == 0) -- 1.7.4.4 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list