conf: add global check for duplicate drive addresses

Add a global check for duplicate drive addresses. This will fix the
problem of duplicate disk and hostdev drive addresses.

Example for duplicate drive addresses:
<disk>
  ...
  <target name='sda'/>
</disk>
<disk>
  ...
  <target name='sdb'/>
  <address type='drive' controller=0 bus=0 target=0 unit=0/>
</disk>

Another example:
<hostdev mode='subsystem' type='scsi' managed='no'>
  <source>
  ...
  </source>
  <address type='drive' controller='0' bus='0' target='0' unit='0'/>
</hostdev>
<hostdev mode='subsystem' type='scsi' managed='no'>
  <source>
  ...
  </source>
  <address type='drive' controller='0' bus='0' target='0' unit='0'/>
</hostdev>

Unfortunately the fixes (1b08cc170a84077afd4d15f4639a9a2cf398e9a2,
8d46386bfe01b84982e25e915ad9cfbae5cf4cb1) weren't enough to catch these
cases and it isn't possible to add additional checks in
virDomainDeviceDefPostParseInternal() for SCSI hostdevs or
virDomainDiskDefAssignAddress() for SCSI/IDE/FDC/SATA disks without
adding another parse flag (virDomainDefParseFlags) to disable this
validation while updating or detaching a disk or hostdev.

Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 25d7186aca31d146f523ac0b5bc65e2cc76e8dd8..0e879e145ea9034891e6cab222e3823a489e0221 100644 (file)
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4842,6 +4842,111 @@ virDomainDefCheckDuplicateDiskInfo(const virDomainDef *def)
     return 0;
 }
 
+/**
+ * virDomainDefCheckDuplicateDriveAddresses:
+ * @def: domain definition to check against
+ *
+ * This function checks @def for duplicate drive addresses. Drive
+ * addresses are only in use for disks and hostdevs at the moment.
+ *
+ * Returns 0 in case of there are no duplicate drive addresses, -1
+ * otherwise.
+ */
+static int
+virDomainDefCheckDuplicateDriveAddresses(const virDomainDef *def)
+{
+    size_t i;
+    size_t j;
+
+    for (i = 0; i < def->ndisks; i++) {
+        virDomainDiskDefPtr disk_i = def->disks[i];
+        virDomainDeviceInfoPtr disk_info_i = &disk_i->info;
+
+        if (disk_info_i->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE)
+            continue;
+
+        for (j = i + 1; j < def->ndisks; j++) {
+            virDomainDiskDefPtr disk_j = def->disks[j];
+            virDomainDeviceInfoPtr disk_info_j = &disk_j->info;
+
+            if (disk_i->bus != disk_j->bus)
+                continue;
+
+            if (disk_info_j->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE)
+                continue;
+
+            if (virDomainDeviceInfoAddressIsEqual(disk_info_i, disk_info_j)) {
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                               _("Found duplicate drive address for disk with "
+                                 "target name '%s' controller='%u' bus='%u' "
+                                 "target='%u' unit='%u'"),
+                               disk_i->dst,
+                               disk_info_i->addr.drive.controller,
+                               disk_info_i->addr.drive.bus,
+                               disk_info_i->addr.drive.target,
+                               disk_info_i->addr.drive.unit);
+                return -1;
+            }
+        }
+
+        /* Note: There is no need to check for conflicts with SCSI
+         * hostdevs above, because conflicts with hostdevs are checked
+         * in the next loop.
+         */
+    }
+
+    for (i = 0; i < def->nhostdevs; i++) {
+        virDomainHostdevDefPtr hdev_i = def->hostdevs[i];
+        virDomainDeviceInfoPtr hdev_info_i = hdev_i->info;
+        virDomainDeviceDriveAddressPtr hdev_addr_i;
+
+        if (!virHostdevIsSCSIDevice(hdev_i))
+            continue;
+
+        if (hdev_i->info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE)
+            continue;
+
+        hdev_addr_i = &hdev_info_i->addr.drive;
+        for (j = i + 1; j < def->nhostdevs; j++) {
+            virDomainHostdevDefPtr hdev_j = def->hostdevs[j];
+            virDomainDeviceInfoPtr hdev_info_j = hdev_j->info;
+
+            if (!virHostdevIsSCSIDevice(hdev_j))
+                continue;
+
+            /* Address type check for hdev_j will be done implicitly
+             * in virDomainDeviceInfoAddressIsEqual() */
+
+            if (virDomainDeviceInfoAddressIsEqual(hdev_info_i, hdev_info_j)) {
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                               _("SCSI host address controller='%u' "
+                                 "bus='%u' target='%u' unit='%u' in "
+                                 "use by another SCSI host device"),
+                               hdev_addr_i->bus,
+                               hdev_addr_i->controller,
+                               hdev_addr_i->target,
+                               hdev_addr_i->unit);
+                return -1;
+            }
+        }
+
+        if (virDomainDriveAddressIsUsedByDisk(def, VIR_DOMAIN_DISK_BUS_SCSI,
+                                              hdev_addr_i)) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("SCSI host address controller='%u' "
+                             "bus='%u' target='%u' unit='%u' in "
+                             "use by another SCSI disk"),
+                           hdev_addr_i->bus,
+                           hdev_addr_i->controller,
+                           hdev_addr_i->target,
+                           hdev_addr_i->unit);
+            return -1;
+        }
+    }
+
+    return 0;
+}
+
 
 static int
 virDomainDefValidateInternal(const virDomainDef *def)
@@ -4849,6 +4954,9 @@ virDomainDefValidateInternal(const virDomainDef *def)
     if (virDomainDefCheckDuplicateDiskInfo(def) < 0)
         return -1;
 
+    if (virDomainDefCheckDuplicateDriveAddresses(def) < 0)
+        return -1;
+
     if (virDomainDefGetVcpusTopology(def, NULL) < 0)
         return -1;