[libvirt PATCH 15/20] qemu_snapshot: delete: properly update parent snapshot with revert data

Pavel Hrdina posted 20 patches 1 year, 4 months ago
There is a newer version of this series
[libvirt PATCH 15/20] qemu_snapshot: delete: properly update parent snapshot with revert data
Posted by Pavel Hrdina 1 year, 4 months ago
When deleting external snapshot and parent snapshot is the currently
active snapshot as user reverted to it we need to properly update the
parent snapshot metadata.

After the delete is done the new overlay files will be the currently
used files created when snapshot revert was done, replacing the original
overlay files.

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

diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index dbcdf56758..513bcb5a86 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -3063,6 +3063,41 @@ qemuSnapshotDiscardExternal(virDomainObj *vm,
 }
 
 
+static int
+qemuSnapshotDeleteUpdateParent(virDomainObj *vm,
+                               virDomainMomentObj *parent)
+{
+    size_t i;
+    virQEMUDriver *driver = ((qemuDomainObjPrivate *) vm->privateData)->driver;
+    g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
+    virDomainSnapshotDef *parentDef = virDomainSnapshotObjGetDef(parent);
+
+    if (!parentDef)
+        return 0;
+
+    if (!parentDef->revertdisks)
+        return 0;
+
+    for (i = 0; i < parentDef->ndisks; i++) {
+        virDomainSnapshotDiskDefClear(&parentDef->disks[i]);
+    }
+    g_free(parentDef->disks);
+
+    parentDef->disks = g_steal_pointer(&parentDef->revertdisks);
+    parentDef->ndisks = parentDef->nrevertdisks;
+    parentDef->nrevertdisks = 0;
+
+    if (qemuDomainSnapshotWriteMetadata(vm,
+                                        parent,
+                                        driver->xmlopt,
+                                        cfg->snapshotDir) < 0) {
+        return -1;
+    }
+
+    return 0;
+}
+
+
 static int
 qemuSnapshotDiscardMetadata(virDomainObj *vm,
                             virDomainMomentObj *snap,
@@ -3102,6 +3137,11 @@ qemuSnapshotDiscardMetadata(virDomainObj *vm,
         virDomainMomentMoveChildren(snap, snap->parent);
     }
 
+    if (update_parent && snap->parent) {
+        if (qemuSnapshotDeleteUpdateParent(vm, snap->parent) < 0)
+            ret = -1;
+    }
+
     snapFile = g_strdup_printf("%s/%s/%s.xml", cfg->snapshotDir, vm->def->name,
                                snap->def->name);
 
-- 
2.39.2
Re: [libvirt PATCH 15/20] qemu_snapshot: delete: properly update parent snapshot with revert data
Posted by Peter Krempa 1 year, 3 months ago
On Mon, Mar 13, 2023 at 16:42:16 +0100, Pavel Hrdina wrote:
> When deleting external snapshot and parent snapshot is the currently
> active snapshot as user reverted to it we need to properly update the
> parent snapshot metadata.
> 
> After the delete is done the new overlay files will be the currently
> used files created when snapshot revert was done, replacing the original
> overlay files.
> 
> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> ---
>  src/qemu/qemu_snapshot.c | 40 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
> index dbcdf56758..513bcb5a86 100644
> --- a/src/qemu/qemu_snapshot.c
> +++ b/src/qemu/qemu_snapshot.c
> @@ -3063,6 +3063,41 @@ qemuSnapshotDiscardExternal(virDomainObj *vm,
>  }
>  
>  
> +static int
> +qemuSnapshotDeleteUpdateParent(virDomainObj *vm,
> +                               virDomainMomentObj *parent)
> +{
> +    size_t i;
> +    virQEMUDriver *driver = ((qemuDomainObjPrivate *) vm->privateData)->driver;

You can use QEMU_DOMAIN_PRIVATE(vm)->driver here to hide the typecast.

> +    g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
> +    virDomainSnapshotDef *parentDef = virDomainSnapshotObjGetDef(parent);
> +
> +    if (!parentDef)
> +        return 0;
> +
> +    if (!parentDef->revertdisks)
> +        return 0;

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