[PATCH 4/4] vmx: Ensure unique disk targets when parsing

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

 



When parsing disks from a vmx file, the target name is generated
based on disk bus, controller the disk is attached to, and its
unit. But in case of SCSI and SATA attached disks this does not
guarantee the target name uniqueness. If there are two disks, one
attached to scsi.0 and the other to sata.0 both end up with the
same "sda" target name. And because of the way their drive
address is derived, they end up with the same address too.

Try harder to generate an unique disk target.

Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
---
 src/vmx/vmx.c                            | 189 +++++++++++++----------
 tests/vmx2xmldata/esx-in-the-wild-12.xml |   4 +-
 tests/vmx2xmldata/esx-in-the-wild-8.xml  |   4 +-
 3 files changed, 109 insertions(+), 88 deletions(-)

diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index 399f03b419..7c752c72f9 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -2142,105 +2142,126 @@ virXMXGenerateDiskTarget(virDomainDiskDef *def,
                          int controllerOrBus,
                          int unit)
 {
-    const char *prefix = NULL;
-    unsigned int idx = 0;
-
-    switch (def->bus) {
-    case VIR_DOMAIN_DISK_BUS_SCSI:
-        if (controllerOrBus < 0 || controllerOrBus > 3) {
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("SCSI controller index %1$d out of [0..3] range"),
-                           controllerOrBus);
-            return -1;
-        }
+    unsigned int tries = 0;
 
-        if (unit < 0 || unit > vmdef->scsiBusMaxUnit || unit == 7) {
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("SCSI unit index %1$d out of [0..6,8..%2$u] range"),
-                           unit, vmdef->scsiBusMaxUnit);
-            return -1;
-        }
+    for (tries = 0; tries < 10; tries++) {
+        g_autofree char *dst = NULL;
+        const char *prefix = NULL;
+        unsigned int idx = 0;
+        size_t i;
 
-        idx = controllerOrBus * 15 + (unit < 7 ? unit : unit - 1);
-        prefix = "sd";
-        break;
+        switch (def->bus) {
+        case VIR_DOMAIN_DISK_BUS_SCSI:
+            if (controllerOrBus < 0 || controllerOrBus > 3) {
+                virReportError(VIR_ERR_INTERNAL_ERROR,
+                               _("SCSI controller index %1$d out of [0..3] range"),
+                               controllerOrBus);
+                return -1;
+            }
 
-    case VIR_DOMAIN_DISK_BUS_SATA:
-        if (controllerOrBus < 0 || controllerOrBus > 3) {
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("SATA controller index %1$d out of [0..3] range"),
-                           controllerOrBus);
-            return -1;
-        }
+            if (unit < 0 || unit > vmdef->scsiBusMaxUnit || unit == 7) {
+                virReportError(VIR_ERR_INTERNAL_ERROR,
+                               _("SCSI unit index %1$d out of [0..6,8..%2$u] range"),
+                               unit, vmdef->scsiBusMaxUnit);
+                return -1;
+            }
 
-        if (unit < 0 || unit >= 30) {
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("SATA unit index %1$d out of [0..29] range"),
-                           unit);
-            return -1;
-        }
+            idx = controllerOrBus * 15 + (unit < 7 ? unit : unit - 1);
+            prefix = "sd";
+            break;
 
-        idx = controllerOrBus * 30 + unit;
-        prefix = "sd";
-        break;
+        case VIR_DOMAIN_DISK_BUS_SATA:
+            if (controllerOrBus < 0 || controllerOrBus > 3) {
+                virReportError(VIR_ERR_INTERNAL_ERROR,
+                               _("SATA controller index %1$d out of [0..3] range"),
+                               controllerOrBus);
+                return -1;
+            }
 
-    case VIR_DOMAIN_DISK_BUS_IDE:
-        if (controllerOrBus < 0 || controllerOrBus > 1) {
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("IDE bus index %1$d out of [0..1] range"),
-                           controllerOrBus);
-            return -1;
-        }
+            if (unit < 0 || unit >= 30) {
+                virReportError(VIR_ERR_INTERNAL_ERROR,
+                               _("SATA unit index %1$d out of [0..29] range"),
+                               unit);
+                return -1;
+            }
+
+            idx = controllerOrBus * 30 + unit;
+            prefix = "sd";
+            break;
 
-        if (unit < 0 || unit > 1) {
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("IDE unit index %1$d out of [0..1] range"), unit);
+        case VIR_DOMAIN_DISK_BUS_IDE:
+            if (controllerOrBus < 0 || controllerOrBus > 1) {
+                virReportError(VIR_ERR_INTERNAL_ERROR,
+                               _("IDE bus index %1$d out of [0..1] range"),
+                               controllerOrBus);
+                return -1;
+            }
+
+            if (unit < 0 || unit > 1) {
+                virReportError(VIR_ERR_INTERNAL_ERROR,
+                               _("IDE unit index %1$d out of [0..1] range"), unit);
+                return -1;
+            }
+            idx = controllerOrBus * 2 + unit;
+            prefix = "hd";
+            break;
+
+        case VIR_DOMAIN_DISK_BUS_FDC:
+            if (controllerOrBus != 0) {
+                virReportError(VIR_ERR_INTERNAL_ERROR,
+                               _("FDC controller index %1$d out of [0] range"),
+                               controllerOrBus);
+                return -1;
+            }
+
+            if (unit < 0 || unit > 1) {
+                virReportError(VIR_ERR_INTERNAL_ERROR,
+                               _("FDC unit index %1$d out of [0..1] range"),
+                               unit);
+                return -1;
+            }
+
+            idx = unit;
+            prefix = "fd";
+            break;
+
+        case VIR_DOMAIN_DISK_BUS_VIRTIO:
+        case VIR_DOMAIN_DISK_BUS_XEN:
+        case VIR_DOMAIN_DISK_BUS_USB:
+        case VIR_DOMAIN_DISK_BUS_UML:
+        case VIR_DOMAIN_DISK_BUS_SD:
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("Unsupported bus type '%1$s' for device type '%2$s'"),
+                           virDomainDiskBusTypeToString(def->bus),
+                           virDomainDiskDeviceTypeToString(def->device));
             return -1;
-        }
-        idx = controllerOrBus * 2 + unit;
-        prefix = "hd";
-        break;
-
-    case VIR_DOMAIN_DISK_BUS_FDC:
-        if (controllerOrBus != 0) {
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("FDC controller index %1$d out of [0] range"),
-                           controllerOrBus);
+            break;
+
+        case VIR_DOMAIN_DISK_BUS_NONE:
+        case VIR_DOMAIN_DISK_BUS_LAST:
+            virReportEnumRangeError(virDomainDiskBus, def->bus);
             return -1;
+            break;
         }
 
-        if (unit < 0 || unit > 1) {
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("FDC unit index %1$d out of [0..1] range"),
-                           unit);
-            return -1;
+        /* Now generate target candidate and check for its uniqueness. */
+
+        dst = virIndexToDiskName(idx + tries, prefix);
+
+        for (i = 0; i < vmdef->ndisks; i++) {
+            if (STREQ(vmdef->disks[i]->dst, dst))
+                break;
         }
 
-        idx = unit;
-        prefix = "fd";
-        break;
-
-    case VIR_DOMAIN_DISK_BUS_VIRTIO:
-    case VIR_DOMAIN_DISK_BUS_XEN:
-    case VIR_DOMAIN_DISK_BUS_USB:
-    case VIR_DOMAIN_DISK_BUS_UML:
-    case VIR_DOMAIN_DISK_BUS_SD:
-        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                       _("Unsupported bus type '%1$s' for device type '%2$s'"),
-                       virDomainDiskBusTypeToString(def->bus),
-                       virDomainDiskDeviceTypeToString(def->device));
-        return -1;
-        break;
-
-    case VIR_DOMAIN_DISK_BUS_NONE:
-    case VIR_DOMAIN_DISK_BUS_LAST:
-        virReportEnumRangeError(virDomainDiskBus, def->bus);
-        return -1;
-        break;
+        if (i == vmdef->ndisks) {
+            def->dst = g_steal_pointer(&dst);
+            return 0;
+        }
     }
 
-    def->dst = virIndexToDiskName(idx, prefix);
-    return 0;
+    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                   _("Unable to generate disk target name"));
+    return -1;
 }
 
 
diff --git a/tests/vmx2xmldata/esx-in-the-wild-12.xml b/tests/vmx2xmldata/esx-in-the-wild-12.xml
index 42184501d0..a7730845ee 100644
--- a/tests/vmx2xmldata/esx-in-the-wild-12.xml
+++ b/tests/vmx2xmldata/esx-in-the-wild-12.xml
@@ -21,9 +21,9 @@
       <address type='drive' controller='0' bus='0' target='0' unit='0'/>
     </disk>
     <disk type='file' device='cdrom'>
-      <target dev='sda' bus='sata'/>
+      <target dev='sdb' bus='sata'/>
       <readonly/>
-      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+      <address type='drive' controller='0' bus='0' target='0' unit='1'/>
     </disk>
     <controller type='scsi' index='0' model='vmpvscsi'/>
     <controller type='sata' index='0'/>
diff --git a/tests/vmx2xmldata/esx-in-the-wild-8.xml b/tests/vmx2xmldata/esx-in-the-wild-8.xml
index 0eea610709..4e3f986e38 100644
--- a/tests/vmx2xmldata/esx-in-the-wild-8.xml
+++ b/tests/vmx2xmldata/esx-in-the-wild-8.xml
@@ -36,9 +36,9 @@
     </disk>
     <disk type='file' device='cdrom'>
       <source file='[692eb778-2d4937fe] CentOS-4.7.ServerCD-x86_64.iso'/>
-      <target dev='sda' bus='sata'/>
+      <target dev='sdd' bus='sata'/>
       <readonly/>
-      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+      <address type='drive' controller='0' bus='0' target='0' unit='3'/>
     </disk>
     <controller type='scsi' index='0' model='vmpvscsi'/>
     <controller type='sata' index='0'/>
-- 
2.41.0
_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx




[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