Re: [PATCH v2 6/6] qemu: Allow mixing active internal and external active disk snapshots
On Wed, Feb 15, 2023 at 05:28:22 -0600, Or Ozeri wrote:
Previous commit added support for active RBD disk snapshots,
using the same qmp_transaction used also for external snapshots.
The same transaction can be used to mix internal and external snapshots.
To allow this, this commit simply removes the check that disallows mixing
for active snapshots.
Note that previous commits added validation disallowing deletion
and reverting of RBD disks. The same validation applies to the mixed
type of snapshots introduced by this commit.
Signed-off-by: Or Ozeri <oro(a)il.ibm.com>
---
src/qemu/qemu_snapshot.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index c72bdb4723..81b3e94988 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -746,10 +746,9 @@ qemuSnapshotPrepare(virDomainObj *vm,
return -1;
}
- /* For now, we don't allow mixing internal and external disks.
- * XXX technically, we could mix internal and external disks for
+ /* For now, we don't allow mixing internal and external disks for
* offline snapshots */
The comment is not really correct now:
We allow after this patch:
- full internal snapshots with internal memory snapshot when all disks
are qcow2
- if the VM is online:
- a disk-only snapshot
- external only
- some or all RBD internal snapshots can be combined in
- external memory snapshot combined with the above (does not yet
work, see below)
- if ((found_internal && external) ||
+ if ((found_internal && external && !active) ||
(def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL && external)
||
(def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL &&
found_internal)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
What this also does not forbid is a combination of a full-internal
old-style snapshot while defining some snapshot names for random RBD
disks, which will not work.
Additionally when def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL
the code will not do what you want. It might even break. In such case we
still do want to forbid an internal snapshot even if it is on RBD.
Obviously unless you've tested and verified that it actually works.
The above also doesn't allow a external memory snapshot being combined
with internal RBD snapshots