5:38 p.m.
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