When user creates a new snapshot after reverting to non-leaf snapshot we
no longer need to store the temporary overlays as they will be part of
the VM XMLs stored in the newly created snapshot.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
---
 src/qemu/qemu_snapshot.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index a206f015c4..2950ad7d77 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -1652,6 +1652,23 @@ qemuSnapshotCreateWriteMetadata(virDomainObj *vm,
 }
 
 
+static void
+qemuSnapshotClearRevertdisks(virDomainMomentObj *current)
+{
+    virDomainSnapshotDef *curdef = NULL;
+
+    if (!current)
+        return;
+
+    curdef = virDomainSnapshotObjGetDef(current);
+
+    if (curdef->revertdisks) {
+        g_clear_pointer(&curdef->revertdisks, g_free);
+        curdef->nrevertdisks = 0;
+    }
+}
+
+
 static virDomainSnapshotPtr
 qemuSnapshotRedefine(virDomainObj *vm,
                      virDomainPtr domain,
@@ -1661,6 +1678,7 @@ qemuSnapshotRedefine(virDomainObj *vm,
                      unsigned int flags)
 {
     virDomainMomentObj *snap = NULL;
+    virDomainMomentObj *current = virDomainSnapshotGetCurrent(vm->snapshots);
     virDomainSnapshotPtr ret = NULL;
     g_autoptr(virDomainSnapshotDef) snapdef = virObjectRef(snapdeftmp);
 
@@ -1678,8 +1696,10 @@ qemuSnapshotRedefine(virDomainObj *vm,
      * makes sense, such as checking that qemu-img recognizes the
      * snapshot name in at least one of the domain's disks?  */
 
-    if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT)
+    if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT) {
+        qemuSnapshotClearRevertdisks(current);
         qemuSnapshotSetCurrent(vm, snap);
+    }
 
     if (qemuSnapshotCreateWriteMetadata(vm, snap, driver, cfg) < 0)
         goto error;
@@ -1758,6 +1778,7 @@ qemuSnapshotCreate(virDomainObj *vm,
     }
 
     if (!tmpsnap) {
+        qemuSnapshotClearRevertdisks(current);
         qemuSnapshotSetCurrent(vm, snap);
 
         if (qemuSnapshotCreateWriteMetadata(vm, snap, driver, cfg) < 0)
-- 
2.41.0

On Tue, Jun 27, 2023 at 17:07:22 +0200, Pavel Hrdina wrote:
> When user creates a new snapshot after reverting to non-leaf snapshot we
> no longer need to store the temporary overlays as they will be part of
> the VM XMLs stored in the newly created snapshot.
> 
> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> ---
>  src/qemu/qemu_snapshot.c | 23 ++++++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
> index a206f015c4..2950ad7d77 100644
> --- a/src/qemu/qemu_snapshot.c
> +++ b/src/qemu/qemu_snapshot.c
> @@ -1652,6 +1652,23 @@ qemuSnapshotCreateWriteMetadata(virDomainObj *vm,
>  }
>  
>  
> +static void
> +qemuSnapshotClearRevertdisks(virDomainMomentObj *current)
> +{
> +    virDomainSnapshotDef *curdef = NULL;
> +
> +    if (!current)
> +        return;
> +
> +    curdef = virDomainSnapshotObjGetDef(current);
> +
> +    if (curdef->revertdisks) {
> +        g_clear_pointer(&curdef->revertdisks, g_free);

At least in one instance the structure's 'name' and 'src' fields are
filled with allocated pointers, so this looks like it will leak memory.

I also didn't find code which frees the 'reverdisks' field when the
snapshot definition is being freed.