[PATCH 11/11] conf: Move and optimize disk target duplicity checking

[Peter Krempa <pkrempa@xxxxxxxxxx>]

Move the logic from virDomainDiskDefDstDuplicates into
virDomainDiskDefCheckDuplicateInfo so that we don't have to loop
multiple times through the array of disks. Since the original function
was called in qemuBuildDriveDevStr, it was actually called for every
single disk which was quite wasteful.

Additionally the target uniqueness check needed to be duplicated in
the disk hotplug case, since the disk was inserted into the domain
definition after the device string was formatted and thus
virDomainDiskDefDstDuplicates didn't do anything in that case.
---
 src/conf/domain_conf.c   | 44 +++++++++++++-------------------------------
 src/conf/domain_conf.h   |  1 -
 src/libvirt_private.syms |  1 -
 src/qemu/qemu_command.c  |  3 ---
 src/qemu/qemu_hotplug.c  |  6 ------
 5 files changed, 13 insertions(+), 42 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8a7a989..c1dffc4 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12988,31 +12988,6 @@ virDomainDiskControllerMatch(int controller_type, int disk_bus)
     return false;
 }

-/* Return true if there's a duplicate disk[]->dst name for the same bus */
-bool
-virDomainDiskDefDstDuplicates(virDomainDefPtr def)
-{
-    size_t i, j;
-
-    /* optimization */
-    if (def->ndisks <= 1)
-        return false;
-
-    for (i = 1; i < def->ndisks; i++) {
-        for (j = 0; j < i; j++) {
-            if (STREQ(def->disks[i]->dst, def->disks[j]->dst)) {
-                virReportError(VIR_ERR_XML_ERROR,
-                               _("target '%s' duplicated for disk sources "
-                                 "'%s' and '%s'"),
-                               def->disks[i]->dst,
-                               NULLSTR(virDomainDiskGetSource(def->disks[i])),
-                               NULLSTR(virDomainDiskGetSource(def->disks[j])));
-                return true;
-            }
-        }
-    }
-    return false;
-}

 int
 virDomainDiskIndexByAddress(virDomainDefPtr def,
@@ -23924,6 +23899,15 @@ int
 virDomainDiskDefCheckDuplicateInfo(virDomainDiskDefPtr a,
                                    virDomainDiskDefPtr b)
 {
+    if (STREQ(a->dst, b->dst)) {
+        virReportError(VIR_ERR_XML_ERROR,
+                       _("target '%s' duplicated for disk sources '%s' and '%s'"),
+                       a->dst,
+                       NULLSTR(virDomainDiskGetSource(a)),
+                       NULLSTR(virDomainDiskGetSource(b)));
+        return -1;
+    }
+
     if (a->wwn && b->wwn && STREQ(a->wwn, b->wwn)) {
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                        _("Disks '%s' and '%s' have identical WWN"),
@@ -23949,12 +23933,10 @@ virDomainDefCheckDuplicateDiskInfo(virDomainDefPtr def)
     size_t j;

     for (i = 0; i < def->ndisks; i++) {
-        if (def->disks[i]->wwn || def->disks[i]->serial) {
-            for (j = i + 1; j < def->ndisks; j++) {
-                if (virDomainDiskDefCheckDuplicateInfo(def->disks[i],
-                                                       def->disks[j]) < 0)
-                    return -1;
-            }
+        for (j = i + 1; j < def->ndisks; j++) {
+            if (virDomainDiskDefCheckDuplicateInfo(def->disks[i],
+                                                   def->disks[j]) < 0)
+                return -1;
         }
     }

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index cbf01bf..8a95b20 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2763,7 +2763,6 @@ int virDomainDefCompatibleDevice(virDomainDefPtr def,

 void virDomainRNGDefFree(virDomainRNGDefPtr def);

-bool virDomainDiskDefDstDuplicates(virDomainDefPtr def);
 int virDomainDiskIndexByAddress(virDomainDefPtr def,
                                 virDevicePCIAddressPtr pci_controller,
                                 unsigned int bus, unsigned int target,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index bf25473..11de1f8 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -257,7 +257,6 @@ virDomainDiskCacheTypeFromString;
 virDomainDiskCacheTypeToString;
 virDomainDiskDefAssignAddress;
 virDomainDiskDefCheckDuplicateInfo;
-virDomainDiskDefDstDuplicates;
 virDomainDiskDefForeachPath;
 virDomainDiskDefFree;
 virDomainDiskDefNew;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 8943270..d7f19f3 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4228,9 +4228,6 @@ qemuBuildDriveDevStr(virDomainDefPtr def,
     const char *contAlias;
     int controllerModel;

-    if (virDomainDiskDefDstDuplicates(def))
-        goto error;
-
     if (qemuCheckDiskConfig(disk) < 0)
         goto error;

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 18a5a12..8771780 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -794,12 +794,6 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
     case VIR_DOMAIN_DISK_DEVICE_DISK:
     case VIR_DOMAIN_DISK_DEVICE_LUN:
         for (i = 0; i < vm->def->ndisks; i++) {
-            if (STREQ(vm->def->disks[i]->dst, disk->dst)) {
-                virReportError(VIR_ERR_OPERATION_FAILED,
-                               _("target %s already exists"), disk->dst);
-                goto cleanup;
-            }
-
             if (virDomainDiskDefCheckDuplicateInfo(vm->def->disks[i], disk) < 0)
                 goto cleanup;
         }
-- 
2.6.2

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list