[libvirt PATCH 13/20] qemu_snapshot: prepare data for non-active leaf external snapshot deletion

Pavel Hrdina posted 20 patches 1 year, 4 months ago
There is a newer version of this series
[libvirt PATCH 13/20] qemu_snapshot: prepare data for non-active leaf external snapshot deletion
Posted by Pavel Hrdina 1 year, 4 months ago
In this case there is no need to run block commit and using qemu process
at all.

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

diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 6ff18d7a9e..be2e5c8cc4 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -2597,34 +2597,40 @@ qemuSnapshotDeleteExternalPrepare(virDomainObj *vm,
         return 0;
     }
 
-    /* this also serves as validation whether the snapshot can be deleted */
-    if (qemuSnapshotDeleteExternalPrepareData(vm, snap, true, &tmpData) < 0)
-        return -1;
-
-    if (!virDomainObjIsActive(vm)) {
-        if (qemuProcessStart(NULL, driver, vm, NULL, VIR_ASYNC_JOB_SNAPSHOT,
-                             NULL, -1, NULL, NULL,
-                             VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
-                             VIR_QEMU_PROCESS_START_PAUSED) < 0) {
-            return -1;
-        }
-
-        *stop_qemu = true;
-
-        /* Call the prepare again as some data require that the VM is
-         * running to get everything we need. */
-        if (qemuSnapshotDeleteExternalPrepareData(vm, snap, true, externalData) < 0)
+    if (snap != virDomainSnapshotGetCurrent(vm->snapshots) &&
+        snap->nchildren == 0) {
+        if (qemuSnapshotDeleteExternalPrepareData(vm, snap, false, externalData) < 0)
             return -1;
     } else {
-        qemuDomainJobPrivate *jobPriv = vm->job->privateData;
+        /* this also serves as validation whether the snapshot can be deleted */
+        if (qemuSnapshotDeleteExternalPrepareData(vm, snap, true, &tmpData) < 0)
+            return -1;
 
-        *externalData = g_steal_pointer(&tmpData);
+        if (!virDomainObjIsActive(vm)) {
+            if (qemuProcessStart(NULL, driver, vm, NULL, VIR_ASYNC_JOB_SNAPSHOT,
+                                 NULL, -1, NULL, NULL,
+                                 VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
+                                 VIR_QEMU_PROCESS_START_PAUSED) < 0) {
+                return -1;
+            }
 
-        /* If the VM is running we need to indicate that the async snapshot
-         * job is snapshot delete job. */
-        jobPriv->snapshotDelete = true;
+            *stop_qemu = true;
 
-        qemuDomainSaveStatus(vm);
+            /* Call the prepare again as some data require that the VM is
+             * running to get everything we need. */
+            if (qemuSnapshotDeleteExternalPrepareData(vm, snap, true, externalData) < 0)
+                return -1;
+        } else {
+            qemuDomainJobPrivate *jobPriv = vm->job->privateData;
+
+            *externalData = g_steal_pointer(&tmpData);
+
+            /* If the VM is running we need to indicate that the async snapshot
+             * job is snapshot delete job. */
+            jobPriv->snapshotDelete = true;
+
+            qemuDomainSaveStatus(vm);
+        }
     }
 
     return 0;
-- 
2.39.2
Re: [libvirt PATCH 13/20] qemu_snapshot: prepare data for non-active leaf external snapshot deletion
Posted by Peter Krempa 1 year, 3 months ago
On Mon, Mar 13, 2023 at 16:42:14 +0100, Pavel Hrdina wrote:
> In this case there is no need to run block commit and using qemu process
> at all.
> 
> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> ---
>  src/qemu/qemu_snapshot.c | 52 ++++++++++++++++++++++------------------
>  1 file changed, 29 insertions(+), 23 deletions(-)
> 
> diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
> index 6ff18d7a9e..be2e5c8cc4 100644
> --- a/src/qemu/qemu_snapshot.c
> +++ b/src/qemu/qemu_snapshot.c
> @@ -2597,34 +2597,40 @@ qemuSnapshotDeleteExternalPrepare(virDomainObj *vm,

[...]

Please explain the logic and implications in a comment above this
condition.

> +    if (snap != virDomainSnapshotGetCurrent(vm->snapshots) &&
> +        snap->nchildren == 0) {
> +        if (qemuSnapshotDeleteExternalPrepareData(vm, snap, false, externalData) < 0)
>              return -1;
>      } else {

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