Re: [libvirt RFC 15/24] qemu_snapshot: call qemuSnapshotDiscardMetadata from qemuSnapshotDiscard
On Tue, Aug 23, 2022 at 18:32:18 +0200, Pavel Hrdina wrote:
This changes the snapshot delete call order. Previously we did
snapshot
XML reparenting before the actual snapshot deletion.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_snapshot.c | 64 +++++++++++++++++++++-------------------
1 file changed, 33 insertions(+), 31 deletions(-)
Okay, I now understand why you've needed to extrac the code, but it
doesn't make it less confusing for the future in any way. You need to
pick a better name and add comment for the function.
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index b94506c177..cbacb05c16 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -2281,6 +2281,34 @@ qemuSnapshotChildrenReparent(void *payload,
}
+static int
+qemuSnapshotDiscardMetadata(virDomainObj *vm,
+ virDomainMomentObj *snap,
+ virQEMUDriver *driver)
Since you need to move the function please put it in the correct place
at the time you've extracted it.
Additionally as mentioned @driver can be fetched from @vm so you can
make the header simpler.
+{
+ g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
+
+ if (snap->nchildren) {
+ virQEMUMomentReparent rep;
+
+ rep.dir = cfg->snapshotDir;
+ rep.parent = snap->parent;
+ rep.vm = vm;
+ rep.err = 0;
+ rep.xmlopt = driver->xmlopt;
+ rep.writeMetadata = qemuDomainSnapshotWriteMetadata;
+ virDomainMomentForEachChild(snap,
+ qemuSnapshotChildrenReparent,
+ &rep);
+ if (rep.err < 0)
+ return -1;
+ virDomainMomentMoveChildren(snap, snap->parent);
+ }
+
+ return 0;
+}
+
+
/* Discard one snapshot (or its metadata), without reparenting any children. */
static int
qemuSnapshotDiscard(virQEMUDriver *driver,
@@ -2323,6 +2351,11 @@ qemuSnapshotDiscard(virQEMUDriver *driver,
}
}
+ if (update_parent &&
+ qemuSnapshotDiscardMetadata(vm, snap, driver) < 0) {
Hmm, so you are planning on moving also ...
+ return -1;
+ }
+
snapFile = g_strdup_printf("%s/%s/%s.xml", cfg->snapshotDir,
vm->def->name,
snap->def->name);
... this code into that function?
That will make sense at the end, but I think you approached it from the
wrong end.
I'd start with extracting the metadata deletion first and then move the
reparenting of children into it later.