RE: [PATCH v2 5/6] qemu: Allow setting per-disk snapshot name for RBD disks
-----Original Message-----
From: Peter Krempa <pkrempa(a)redhat.com>
Sent: Monday, 20 February 2023 16:13
To: Or Ozeri <ORO(a)il.ibm.com>
Cc: libvir-list(a)redhat.com; Danny Harnik <DANNYH(a)il.ibm.com>
Subject: [EXTERNAL] Re: [PATCH v2 5/6] qemu: Allow setting per-disk
snapshot name for RBD disks
> diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index
> a10bdf7bf2..c72bdb4723 100644
> --- a/src/qemu/qemu_snapshot.c
> +++ b/src/qemu/qemu_snapshot.c
> @@ -664,7 +664,7 @@ qemuSnapshotPrepare(virDomainObj *vm,
> return -1;
> }
>
> - if (disk->snapshot_name) {
> + if (disk->snapshot_name && !is_raw_rbd) {
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> _("snapshot name setting for disk %s
unsupported "
> "for storage type %s"),
I don't see anything that updates the equivalent of dom_disk->src-
>snapshot after the snapshot:
The 'snapshot' property is filled as the equivalent property when formatting
the backend definition for the 'rbd' disk.
In case when the 'snapshot' field is meant to actually mean label the
'old'
state. You then must actually tweak the snapshot metadata to point to it.
That will allow proper reversion of the image.
We actually block reverting to this type of snapshot, see
" revert to a non-full internal snapshot not supported yet" in
qemuSnapshotRevertValidate.
Given this, is this change still required?
Actually I'm not sure I understand. Is setting dom_disk->src->snapshot enough?
Or can you please point me to another place in the code where a change is required?