Re: [PATCH 2/2] virStorageSourceIsSameLocation: Special-case storage sources of type 'volume'

On Fri, Nov 26, 2021 at 02:07:54PM +0100, Peter Krempa wrote: > The function is used also to compare virStorageSource which may not be > resolved to the image at that point in which case the 'path' is not yet > populated and the actual type is not yet set. This means that the > function fails to consider two identical volume-based disks as pointing > to the same thing. > > Add a special case for both images being type=volume in which case we > compare only the pool/volume names. > > Closes: https://gitlab.com/libvirt/libvirt/-/issues/240 > Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; > --- > src/conf/storage_source_conf.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/src/conf/storage_source_conf.c b/src/conf/storage_source_conf.c > index 44944e1dbd..c0acee189a 100644 > --- a/src/conf/storage_source_conf.c > +++ b/src/conf/storage_source_conf.c > @@ -920,6 +920,13 @@ virStorageSourceIsSameLocation(virStorageSource *a, > virStorageSourceIsEmpty(b)) > return true; > > + /* for disk type=volume we must check just pool/volume names as they might > + * not yet be resolved if e.g. we are comparing against the persistent def */ > + if (a->type == VIR_STORAGE_TYPE_VOLUME && b->type == VIR_STORAGE_TYPE_VOLUME) { > + return STREQ(a->srcpool->pool, b->srcpool->pool) && > + STREQ(a->srcpool->volume, b->srcpool->volume); > + } > + Does this hold true even for disk sources that differ in the `mode` attribute? I'm just asking for clarity. If that was taken into consideration and the difference does not matter here, then this is