[libvirt PATCH 16/20] qemu_snapshot: don't allow creating external snapshot after reverting

Pavel Hrdina posted 20 patches 1 year, 4 months ago
There is a newer version of this series
[libvirt PATCH 16/20] qemu_snapshot: don't allow creating external snapshot after reverting
Posted by Pavel Hrdina 1 year, 4 months ago
This would allow creating new branch in the external snapshot history
which we currently don't support.

The issue with branching the external snapshot history is that deleting
some snapshot would require calling block-stream instead of
block-commit which is currently not implemented.

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

diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 513bcb5a86..95297cfe6a 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -665,6 +665,8 @@ qemuSnapshotPrepare(virDomainObj *vm,
     bool found_internal = false;
     bool forbid_internal = false;
     int external = 0;
+    virDomainMomentObj *curSnap = NULL;
+    virDomainSnapshotDef *curDef = NULL;
 
     for (i = 0; i < def->ndisks; i++) {
         virDomainSnapshotDiskDef *disk = &def->disks[i];
@@ -801,6 +803,16 @@ qemuSnapshotPrepare(virDomainObj *vm,
         return -1;
     }
 
+    curSnap = virDomainSnapshotGetCurrent(vm->snapshots);
+    if (curSnap)
+        curDef = virDomainSnapshotObjGetDef(curSnap);
+
+    if (curDef && curDef->revertdisks) {
+        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+                       _("creating external snapshot after reverting to not last snapshot is not supported"));
+        return -1;
+    }
+
     /* Alter flags to let later users know what we learned.  */
     if (external && !active)
         *flags |= VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY;
-- 
2.39.2
Re: [libvirt PATCH 16/20] qemu_snapshot: don't allow creating external snapshot after reverting
Posted by Peter Krempa 1 year, 3 months ago
On Mon, Mar 13, 2023 at 16:42:17 +0100, Pavel Hrdina wrote:
> This would allow creating new branch in the external snapshot history
> which we currently don't support.
> 
> The issue with branching the external snapshot history is that deleting
> some snapshot would require calling block-stream instead of
> block-commit which is currently not implemented.

If the problem is with deletion of the snapshot afterwards I strongly
suggest detecting and limiting the deletion itself.

Creation doesn't seem to be the problem and it feels weird to restrict
it.