From 164aba8c4aef8094e151f593baf95421feb6dc5f Mon Sep 17 00:00:00 2001
From: Peter Krempa <pkrempa@redhat.com>
Date: Tue, 11 Jan 2022 10:28:45 +0100
Subject: [PATCH] qemuSnapshotCreate: Don't insert snapshot into list with
 VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA

Our approach to snapshots without metadata was to insert them to the
snapshot list and then later remove them from the list when the flag is
present.

This quirky logic was broken in a recent refactor of the snapshot code
causing that the snapshot stayed inserted in the snapshot list.

Recent refactor of the snapshot code didn't faithfully relocate this
logic to the new function.

Rather than attempting to restore the quirky logic of adding and then
removing the object, don't add the snapshot into the list at all when
the user doesn't want metadata.

We achieve this by creating a temporary 'virDomainMomentObj' wrapper
which is not inserted into the list and using that instead of calling
virDomainSnapshotAssignDef.

Fixes: 9bad0fb809b
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2039131
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/qemu/qemu_snapshot.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 9a5d3e60aa..5d383279b0 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -1756,7 +1756,7 @@ qemuSnapshotCreate(virDomainObj *vm,
                    virQEMUDriverConfig *cfg,
                    unsigned int flags)
 {
-
+    g_autoptr(virDomainMomentObj) tmpsnap = NULL;
     virDomainMomentObj *snap = NULL;
     virDomainMomentObj *current = NULL;
     virDomainSnapshotPtr ret = NULL;
@@ -1767,16 +1767,20 @@ qemuSnapshotCreate(virDomainObj *vm,
     if (qemuSnapshotPrepare(vm, def, &flags) < 0)
         return NULL;
 
-    if (!(snap = virDomainSnapshotAssignDef(vm->snapshots, def)))
-        return NULL;
+    if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA) {
+        snap = tmpsnap = virDomainMomentObjNew();
+        snap->def = &def->parent;
+    } else {
+        if (!(snap = virDomainSnapshotAssignDef(vm->snapshots, def)))
+            return NULL;
+
+        if ((current = virDomainSnapshotGetCurrent(vm->snapshots))) {
+            snap->def->parent_name = g_strdup(current->def->name);
+        }
+    }
 
     virObjectRef(def);
 
-    current = virDomainSnapshotGetCurrent(vm->snapshots);
-    if (current) {
-        snap->def->parent_name = g_strdup(current->def->name);
-    }
-
     /* actually do the snapshot */
     if (virDomainObjIsActive(vm)) {
         if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY ||
@@ -1804,7 +1808,7 @@ qemuSnapshotCreate(virDomainObj *vm,
         }
     }
 
-    if (!(flags & VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA)) {
+    if (!tmpsnap) {
         qemuSnapshotSetCurrent(vm, snap);
 
         if (qemuSnapshotCreateWriteMetadata(vm, snap, driver, cfg) < 0)
@@ -1818,7 +1822,8 @@ qemuSnapshotCreate(virDomainObj *vm,
     return ret;
 
  error:
-    virDomainSnapshotObjListRemove(vm->snapshots, snap);
+    if (!tmpsnap)
+        virDomainSnapshotObjListRemove(vm->snapshots, snap);
     return NULL;
 }