> -----Original Message-----
> From: Peter Krempa <pkrempa@xxxxxxxxxx>
> Sent: Monday, 20 February 2023 16:13
> To: Or Ozeri <ORO@xxxxxxxxxx>
> Cc: libvir-list@xxxxxxxxxx; Danny Harnik <DANNYH@xxxxxxxxxx>
> 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?
