Extract the code deleting external snapshot metadata to separate
function.

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

diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 5418496c1e..4ea3f578e9 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -2281,47 +2281,15 @@ qemuSnapshotChildrenReparent(void *payload,
 }
 
 
-/* Discard one snapshot (or its metadata), without reparenting any children.  */
 static int
-qemuSnapshotDiscard(virQEMUDriver *driver,
-                    virDomainObj *vm,
-                    virDomainMomentObj *snap,
-                    bool update_parent,
-                    bool metadata_only)
+qemuSnapshotDiscardMetadata(virDomainObj *vm,
+                            virDomainMomentObj *snap,
+                            bool update_parent)
 {
+    qemuDomainObjPrivate *priv = vm->privateData;
+    virQEMUDriver *driver = priv->driver;
+    g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
     g_autofree char *snapFile = NULL;
-    qemuDomainObjPrivate *priv;
-    virDomainMomentObj *parentsnap = NULL;
-    g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
-
-    if (!metadata_only) {
-        if (!virDomainObjIsActive(vm)) {
-            size_t i;
-            /* Ignore any skipped disks */
-
-            /* Prefer action on the disks in use at the time the snapshot was
-             * created; but fall back to current definition if dealing with a
-             * snapshot created prior to libvirt 0.9.5.  */
-            virDomainDef *def = snap->def->dom;
-
-            if (!def)
-                def = vm->def;
-
-            for (i = 0; i < def->ndisks; i++) {
-                if (virDomainDiskTranslateSourcePool(def->disks[i]) < 0)
-                    return -1;
-            }
-
-            if (qemuDomainSnapshotForEachQcow2(driver, def, snap, "-d", true) < 0)
-                return -1;
-        } else {
-            priv = vm->privateData;
-            qemuDomainObjEnterMonitor(vm);
-            /* we continue on even in the face of error */
-            qemuMonitorDeleteSnapshot(priv->mon, snap->def->name);
-            qemuDomainObjExitMonitor(vm);
-        }
-    }
 
     if (update_parent) {
         if (snap->nchildren) {
@@ -2348,6 +2316,7 @@ qemuSnapshotDiscard(virQEMUDriver *driver,
     if (snap == virDomainSnapshotGetCurrent(vm->snapshots)) {
         virDomainSnapshotSetCurrent(vm->snapshots, NULL);
         if (update_parent && snap->def->parent_name) {
+            virDomainMomentObj *parentsnap = NULL;
             parentsnap = virDomainSnapshotFindByName(vm->snapshots,
                                                      snap->def->parent_name);
             if (!parentsnap) {
@@ -2376,6 +2345,52 @@ qemuSnapshotDiscard(virQEMUDriver *driver,
 }
 
 
+/* Discard one snapshot (or its metadata), without reparenting any children.  */
+static int
+qemuSnapshotDiscard(virQEMUDriver *driver,
+                    virDomainObj *vm,
+                    virDomainMomentObj *snap,
+                    bool update_parent,
+                    bool metadata_only)
+{
+    qemuDomainObjPrivate *priv;
+
+    if (!metadata_only) {
+        if (!virDomainObjIsActive(vm)) {
+            size_t i;
+            /* Ignore any skipped disks */
+
+            /* Prefer action on the disks in use at the time the snapshot was
+             * created; but fall back to current definition if dealing with a
+             * snapshot created prior to libvirt 0.9.5.  */
+            virDomainDef *def = snap->def->dom;
+
+            if (!def)
+                def = vm->def;
+
+            for (i = 0; i < def->ndisks; i++) {
+                if (virDomainDiskTranslateSourcePool(def->disks[i]) < 0)
+                    return -1;
+            }
+
+            if (qemuDomainSnapshotForEachQcow2(driver, def, snap, "-d", true) < 0)
+                return -1;
+        } else {
+            priv = vm->privateData;
+            qemuDomainObjEnterMonitor(vm);
+            /* we continue on even in the face of error */
+            qemuMonitorDeleteSnapshot(priv->mon, snap->def->name);
+            qemuDomainObjExitMonitor(vm);
+        }
+    }
+
+    if (qemuSnapshotDiscardMetadata(vm, snap, update_parent) < 0)
+        return -1;
+
+    return 0;
+}
+
+
 int
 qemuSnapshotDiscardAllMetadata(virQEMUDriver *driver,
                                virDomainObj *vm)
-- 
2.38.1

On Thu, Dec 08, 2022 at 14:30:54 +0100, Pavel Hrdina wrote:
> Extract the code deleting external snapshot metadata to separate
> function.
> 
> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> ---
>  src/qemu/qemu_snapshot.c | 91 +++++++++++++++++++++++-----------------
>  1 file changed, 53 insertions(+), 38 deletions(-)
> 
> diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
> index 5418496c1e..4ea3f578e9 100644
> --- a/src/qemu/qemu_snapshot.c
> +++ b/src/qemu/qemu_snapshot.c
> @@ -2281,47 +2281,15 @@ qemuSnapshotChildrenReparent(void *payload,
>  }
>  
>  
> -/* Discard one snapshot (or its metadata), without reparenting any children.  */
>  static int
> -qemuSnapshotDiscard(virQEMUDriver *driver,
> -                    virDomainObj *vm,
> -                    virDomainMomentObj *snap,
> -                    bool update_parent,
> -                    bool metadata_only)
> +qemuSnapshotDiscardMetadata(virDomainObj *vm,
> +                            virDomainMomentObj *snap,
> +                            bool update_parent)
>  {
> +    qemuDomainObjPrivate *priv = vm->privateData;
> +    virQEMUDriver *driver = priv->driver;
> +    g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
>      g_autofree char *snapFile = NULL;
> -    qemuDomainObjPrivate *priv;
> -    virDomainMomentObj *parentsnap = NULL;
> -    g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
> -
> -    if (!metadata_only) {
> -        if (!virDomainObjIsActive(vm)) {
> -            size_t i;
> -            /* Ignore any skipped disks */
> -
> -            /* Prefer action on the disks in use at the time the snapshot was
> -             * created; but fall back to current definition if dealing with a
> -             * snapshot created prior to libvirt 0.9.5.  */
> -            virDomainDef *def = snap->def->dom;
> -
> -            if (!def)
> -                def = vm->def;
> -
> -            for (i = 0; i < def->ndisks; i++) {
> -                if (virDomainDiskTranslateSourcePool(def->disks[i]) < 0)
> -                    return -1;
> -            }
> -
> -            if (qemuDomainSnapshotForEachQcow2(driver, def, snap, "-d", true) < 0)
> -                return -1;
> -        } else {
> -            priv = vm->privateData;
> -            qemuDomainObjEnterMonitor(vm);
> -            /* we continue on even in the face of error */
> -            qemuMonitorDeleteSnapshot(priv->mon, snap->def->name);
> -            qemuDomainObjExitMonitor(vm);
> -        }
> -    }
>  
>      if (update_parent) {
>          if (snap->nchildren) {
> @@ -2348,6 +2316,7 @@ qemuSnapshotDiscard(virQEMUDriver *driver,
>      if (snap == virDomainSnapshotGetCurrent(vm->snapshots)) {
>          virDomainSnapshotSetCurrent(vm->snapshots, NULL);
>          if (update_parent && snap->def->parent_name) {
> +            virDomainMomentObj *parentsnap = NULL;
>              parentsnap = virDomainSnapshotFindByName(vm->snapshots,
>                                                       snap->def->parent_name);
>              if (!parentsnap) {
> @@ -2376,6 +2345,52 @@ qemuSnapshotDiscard(virQEMUDriver *driver,
>  }
>  
>  
> +/* Discard one snapshot (or its metadata), without reparenting any children.  */
> +static int
> +qemuSnapshotDiscard(virQEMUDriver *driver,
> +                    virDomainObj *vm,
> +                    virDomainMomentObj *snap,
> +                    bool update_parent,
> +                    bool metadata_only)
> +{
> +    qemuDomainObjPrivate *priv;

Declare this variable ...

> +
> +    if (!metadata_only) {
> +        if (!virDomainObjIsActive(vm)) {
> +            size_t i;
> +            /* Ignore any skipped disks */
> +
> +            /* Prefer action on the disks in use at the time the snapshot was
> +             * created; but fall back to current definition if dealing with a
> +             * snapshot created prior to libvirt 0.9.5.  */
> +            virDomainDef *def = snap->def->dom;
> +
> +            if (!def)
> +                def = vm->def;
> +
> +            for (i = 0; i < def->ndisks; i++) {
> +                if (virDomainDiskTranslateSourcePool(def->disks[i]) < 0)
> +                    return -1;
> +            }
> +
> +            if (qemuDomainSnapshotForEachQcow2(driver, def, snap, "-d", true) < 0)
> +                return -1;
> +        } else {
> +            priv = vm->privateData;

... here.

> +            qemuDomainObjEnterMonitor(vm);
> +            /* we continue on even in the face of error */
> +            qemuMonitorDeleteSnapshot(priv->mon, snap->def->name);

but since it's only used for monitor you can also do
'qemuDomainGetMonitor(vm)' here instead.

> +            qemuDomainObjExitMonitor(vm);
> +        }

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