[libvirt PATCH v2 1/6] qemu_snapshot: fix reverting external snapshot when not all disks are included

[Pavel Hrdina]

On Fri, Sep 01, 2023 at 11:10:43AM +0200, Peter Krempa wrote:
> On Fri, Sep 01, 2023 at 10:32:12 +0200, Pavel Hrdina wrote:
> > We need to skip all disks that have snapshot type other than 'external'.
> 
> Since the commit message is vague on the specific problem details ...
> 
> > Signed-off-by: Pavel Hrdina <phrdina at 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 d943281e35..ff85d56572 100644
> > --- a/src/qemu/qemu_snapshot.c
> > +++ b/src/qemu/qemu_snapshot.c
> > @@ -2058,6 +2058,9 @@ qemuSnapshotRevertExternalPrepare(virDomainObj *vm,
> >          virDomainSnapshotDiskDef *snapdisk = &tmpsnapdef->disks[i];
> >          virDomainDiskDef *domdisk = domdef->disks[i];
> >  
> > +        if (snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
> > +            continue;
> 
> ... I remember you talking about the need to create overlays also for
> images which might have been excluded in given snapshot (the one you are
> reverting to) , but externally snapshotted in a later (still existing)
> one.
> 
> In such case not creating the overlay here would still invalidate the
> overlay of the next snapshot. Is that right?
> 
> I also remember that you mentioned that the actual problem is with empty
> cdroms, thus, did you rather want to use 'virStorageSourceIsEmpty' here?

That is done in virDomainSnapshotAlignDisks() called right before the
for loop in qemuSnapshotRevertExternalPrepare(), there we fill in the
temporary snapshot definition with new overlays. From that moment the
temporary snapshot definition contains all disks based on the VM
definition.

Looking at the code, when reverting to a snapshot we will always create
new overlays for all disks including readonly disks, not sure if that is
what we should do or no.

Pavel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20230901/22d94877/attachment.sig>