When block commit is not needed we can just simply unlink the
disk files.

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

diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index be2e5c8cc4..dbcdf56758 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -2414,6 +2414,7 @@ typedef struct _qemuSnapshotDeleteExternalData {
     virStorageSource *prevDiskSrc; /* source of disk for which @diskSrc is
                                       backing disk */
     qemuBlockJobData *job;
+    bool blockCommit;
 } qemuSnapshotDeleteExternalData;
 
 
@@ -2517,8 +2518,9 @@ qemuSnapshotDeleteExternalPrepareData(virDomainObj *vm,
 
         data = g_new0(qemuSnapshotDeleteExternalData, 1);
         data->snapDisk = snapDisk;
+        data->blockCommit = blockCommit;
 
-        if (blockCommit) {
+        if (data->blockCommit) {
             data->domDisk = qemuDomainDiskByName(vm->def, snapDisk->name);
             if (!data->domDisk)
                 return -1;
@@ -2949,31 +2951,41 @@ qemuSnapshotDiscardExternal(virDomainObj *vm,
         virTristateBool autofinalize = VIR_TRISTATE_BOOL_NO;
         unsigned int commitFlags = VIR_DOMAIN_BLOCK_COMMIT_DELETE;
 
-        if (data->domDisk->src == data->diskSrc) {
-            commitFlags |= VIR_DOMAIN_BLOCK_COMMIT_ACTIVE;
-            autofinalize = VIR_TRISTATE_BOOL_YES;
+        if (data->blockCommit) {
+            if (data->domDisk->src == data->diskSrc) {
+                commitFlags |= VIR_DOMAIN_BLOCK_COMMIT_ACTIVE;
+                autofinalize = VIR_TRISTATE_BOOL_YES;
+            }
+
+            if (qemuSnapshotSetInvalid(vm, data->parentSnap, data->snapDisk, true) < 0)
+                goto error;
+
+            data->job = qemuBlockCommit(vm,
+                                        data->domDisk,
+                                        data->parentDiskSrc,
+                                        data->diskSrc,
+                                        data->prevDiskSrc,
+                                        0,
+                                        VIR_ASYNC_JOB_SNAPSHOT,
+                                        autofinalize,
+                                        commitFlags);
+
+            if (!data->job)
+                goto error;
+        } else {
+            if (unlink(data->snapDisk->src->path) < 0) {
+                VIR_WARN("Failed to remove snapshot image '%s'",
+                         data->snapDisk->name);
+            }
         }
-
-        if (qemuSnapshotSetInvalid(vm, data->parentSnap, data->snapDisk, true) < 0)
-            goto error;
-
-        data->job = qemuBlockCommit(vm,
-                                    data->domDisk,
-                                    data->parentDiskSrc,
-                                    data->diskSrc,
-                                    data->prevDiskSrc,
-                                    0,
-                                    VIR_ASYNC_JOB_SNAPSHOT,
-                                    autofinalize,
-                                    commitFlags);
-
-        if (!data->job)
-            goto error;
     }
 
     for (cur = externalData; cur; cur = g_slist_next(cur)) {
         qemuSnapshotDeleteExternalData *data = cur->data;
 
+        if (!data->blockCommit)
+            continue;
+
         if (qemuSnapshotDeleteBlockJobRunning(vm, data->job) < 0)
             goto error;
 
@@ -2988,6 +3000,9 @@ qemuSnapshotDiscardExternal(virDomainObj *vm,
     for (cur = externalData; cur; cur = g_slist_next(cur)) {
         qemuSnapshotDeleteExternalData *data = cur->data;
 
+        if (!data->blockCommit)
+            continue;
+
         if (data->job->state == QEMU_BLOCKJOB_STATE_READY) {
             if (qemuBlockPivot(vm, data->job, VIR_ASYNC_JOB_SNAPSHOT, NULL) < 0)
                 goto error;
-- 
2.39.2

On Mon, Mar 13, 2023 at 16:42:15 +0100, Pavel Hrdina wrote:
> When block commit is not needed we can just simply unlink the
> disk files.
> 
> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> ---
>  src/qemu/qemu_snapshot.c | 55 +++++++++++++++++++++++++---------------
>  1 file changed, 35 insertions(+), 20 deletions(-)
> 
> diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
> index be2e5c8cc4..dbcdf56758 100644
> --- a/src/qemu/qemu_snapshot.c
> +++ b/src/qemu/qemu_snapshot.c

[...]

> @@ -2949,31 +2951,41 @@ qemuSnapshotDiscardExternal(virDomainObj *vm,
>          virTristateBool autofinalize = VIR_TRISTATE_BOOL_NO;
>          unsigned int commitFlags = VIR_DOMAIN_BLOCK_COMMIT_DELETE;
>  
> -        if (data->domDisk->src == data->diskSrc) {
> -            commitFlags |= VIR_DOMAIN_BLOCK_COMMIT_ACTIVE;
> -            autofinalize = VIR_TRISTATE_BOOL_YES;
> +        if (data->blockCommit) {
> +            if (data->domDisk->src == data->diskSrc) {
> +                commitFlags |= VIR_DOMAIN_BLOCK_COMMIT_ACTIVE;
> +                autofinalize = VIR_TRISTATE_BOOL_YES;
> +            }
> +
> +            if (qemuSnapshotSetInvalid(vm, data->parentSnap, data->snapDisk, true) < 0)
> +                goto error;
> +
> +            data->job = qemuBlockCommit(vm,
> +                                        data->domDisk,
> +                                        data->parentDiskSrc,
> +                                        data->diskSrc,
> +                                        data->prevDiskSrc,
> +                                        0,
> +                                        VIR_ASYNC_JOB_SNAPSHOT,
> +                                        autofinalize,
> +                                        commitFlags);
> +
> +            if (!data->job)
> +                goto error;
> +        } else {
> +            if (unlink(data->snapDisk->src->path) < 0) {

This can be done only when the 'src' object is "local storage"
(virStorageSourceIsLocalStorage)

> +                VIR_WARN("Failed to remove snapshot image '%s'",
> +                         data->snapDisk->name);
> +            }

Reviewed-by: Peter Krempa <pkrempa@redhat.com>