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@xxxxxxxxxx> > > --- > > 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
Attachment:
signature.asc
Description: PGP signature
