On Fri, Apr 03, 2020 at 05:56:59PM +0200, Pavel Mores wrote: > On Tue, Mar 31, 2020 at 12:06:15PM +0200, Peter Krempa wrote: > > On Tue, Mar 31, 2020 at 11:18:29 +0200, Pavel Mores wrote: > > > + } > > > + > > > + if (flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN) > > > + topPath = disk->src->path; > > > + else > > > + topPath = snapdisk->src->path; > > > > You must not use paths for doing this lookup. Paths at best work for > > local files only and this would make the code not future proof. > > > > Also you want to verify that: > > - images you want to merge are actually in the backing chain > > - the backing chain looks as snapshots describe it (e.g you unplug vda > > and plug a different chain back) > > Let me check with you how exactly to perform this verification. > > To retrieve the names of the disks included in a snapshot, I can use its > virDomainSnapshotDef which contains an array of disks > (virDomainSnapshotDiskDef's) whose 'name' member can be used to identify disks. > > To get the state of the chain at moment the snapshot was created, I can use the > 'src' member and walk its 'backingStore' list to examine the whole chain. > > To get the currently running configuration, I assume I can use the names of the > relevant disks (obtained from the snapshot as mentioned above) to get a > virDomainDiskDefPtr for each of them, whose 'src' member points to a > virStorageSource that represents the topmost image in the disk's chain. From > there, I can walk the 'backingStore' list to get the other images in the chain > all the way to the base image. > > The verification succeeds if the disk names and their chains in the snapshot > and the running config correspond. Is the above correct? > > If so, I'm still not certain how to compare two virStorageSource's. How is > identity of two different virStorageSource instances is established? After having a closer look into this I'm unfortunately even less clear as to how to perform the checks mentioned in the review. In addition to the problem of establishing virStorageSource identity, a similar problem seems to come up with disks. They often seem to be identified and referred to by the target name, however what happens if a snapshot is taken for 'vda', then the disk's target is changed to 'vdb' and now the snapshot is to be deleted? Is there a way to find out that the disk is still there, only under a different name? And as far as comparing chains goes - I know can retrieve the chain for a running disk and I can retrieve what the chain looked like at the point a snapshot was made. However, how do I establish they are equivalent? How can I tell that snapshots in both chains compare identical in the first place? Is the snapshot name stable and persistent enough to be useful for this? Is it enough for chains to be equivalent that the chain at the point of snapshot creation be a superset of the currently running chain? Probably yes, provided snapshots can't be inserted in the middle of a chain - but could that ever happen? Thanks in advance for any help with this! pvl
