Re: [libvirt PATCH 3/3] qemu: block: external snapshot-delete implementation for straightforward cases
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?
Thanks!
pvl