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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
---
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

Reviewed-by: Martin Kletzander <mkletzan@xxxxxxxxxx>

    if (virStorageSourceGetActualType(a) != virStorageSourceGetActualType(b))
        return false;

--
2.31.1

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux