[PATCH 5/7] conf: Store SCSI bus length in virDomainDef

Michal Privoznik mprivozn at redhat.com
Tue Aug 3 12:32:29 UTC 2021


Libvirt assumes that a SCSI bus can fit up to 8 devices
(including controller itself), except for so called wide bus
which can accommodate up to 16 devices (again, including
controller). This plays important role when computing 'drive'
address in virDomainDiskDefAssignAddress(). So far, the only
driver that enables wide SCSI bus is VMX. But with never
releases, ESX is capable of "super wide" bus (64 devices).

We can blindly bump the limit in our code because then we would
compute address that's invalid for older ESX versions that we
still want to support.

Unfortunately, I haven't found a better place where to store this
than virDomainDef.

Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
---
 src/bhyve/bhyve_parse_command.c |  2 +-
 src/conf/domain_conf.c          | 33 ++++++++++++++-------------------
 src/conf/domain_conf.h          |  9 ++++++++-
 src/hyperv/hyperv_driver.c      |  4 ++--
 src/libxl/libxl_driver.c        |  2 +-
 src/libxl/xen_xl.c              |  2 +-
 src/libxl/xen_xm.c              |  2 +-
 src/lxc/lxc_native.c            |  2 +-
 src/openvz/openvz_conf.c        |  2 +-
 src/qemu/qemu_process.c         |  2 +-
 src/vbox/vbox_common.c          |  8 ++++----
 src/vmx/vmx.c                   |  2 +-
 src/vz/vz_sdk.c                 |  2 +-
 tests/genericxml2xmltest.c      |  2 +-
 tests/openvzutilstest.c         |  2 +-
 tests/qemublocktest.c           |  2 +-
 tests/qemumonitortestutils.c    |  2 +-
 tests/securityselinuxtest.c     |  2 +-
 18 files changed, 42 insertions(+), 40 deletions(-)

diff --git a/src/bhyve/bhyve_parse_command.c b/src/bhyve/bhyve_parse_command.c
index 9b97c262ec..7b3a830b45 100644
--- a/src/bhyve/bhyve_parse_command.c
+++ b/src/bhyve/bhyve_parse_command.c
@@ -956,7 +956,7 @@ bhyveParseCommandLineString(const char* nativeConfig,
     int loader_argc = 0;
     char **loader_argv = NULL;
 
-    if (!(def = virDomainDefNew()))
+    if (!(def = virDomainDefNew(xmlopt)))
         goto cleanup;
 
     /* Initialize defaults. */
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 06c1fcf5e5..4ae78fa04d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3790,7 +3790,7 @@ virDomainObjNew(virDomainXMLOption *xmlopt)
 
 
 virDomainDef *
-virDomainDefNew(void)
+virDomainDefNew(virDomainXMLOption *xmlopt)
 {
     virDomainDef *ret;
 
@@ -3803,6 +3803,11 @@ virDomainDefNew(void)
     ret->mem.soft_limit = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED;
     ret->mem.swap_hard_limit = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED;
 
+    if (xmlopt && xmlopt->config.features & VIR_DOMAIN_DEF_FEATURE_WIDE_SCSI)
+        ret->scsiBusMaxUnit = SCSI_WIDE_BUS_MAX_CONT_UNIT;
+    else
+        ret->scsiBusMaxUnit = SCSI_NARROW_BUS_MAX_CONT_UNIT;
+
     return ret;
 
  error:
@@ -5130,12 +5135,11 @@ virDomainSCSIDriveAddressIsUsed(const virDomainDef *def,
 /* Find out the next usable "unit" of a specific controller */
 static int
 virDomainControllerSCSINextUnit(const virDomainDef *def,
-                                unsigned int max_unit,
                                 unsigned int controller)
 {
     size_t i;
 
-    for (i = 0; i < max_unit; i++) {
+    for (i = 0; i < def->scsiBusMaxUnit; i++) {
         /* Default to assigning addresses using bus = target = 0 */
         const virDomainDeviceDriveAddress addr = {controller, 0, 0, i};
 
@@ -5147,22 +5151,13 @@ virDomainControllerSCSINextUnit(const virDomainDef *def,
 }
 
 
-#define SCSI_WIDE_BUS_MAX_CONT_UNIT 16
-#define SCSI_NARROW_BUS_MAX_CONT_UNIT 7
-
 static void
-virDomainHostdevAssignAddress(virDomainXMLOption *xmlopt,
+virDomainHostdevAssignAddress(virDomainXMLOption *xmlopt G_GNUC_UNUSED,
                               const virDomainDef *def,
                               virDomainHostdevDef *hostdev)
 {
     int next_unit = 0;
     int controller = 0;
-    unsigned int max_unit;
-
-    if (xmlopt->config.features & VIR_DOMAIN_DEF_FEATURE_WIDE_SCSI)
-        max_unit = SCSI_WIDE_BUS_MAX_CONT_UNIT;
-    else
-        max_unit = SCSI_NARROW_BUS_MAX_CONT_UNIT;
 
     /* NB: Do not attempt calling virDomainDefMaybeAddController to
      * automagically add a "new" controller. Doing so will result in
@@ -5177,7 +5172,7 @@ virDomainHostdevAssignAddress(virDomainXMLOption *xmlopt,
      * hostdev being added to the as yet to be created controller.
      */
     do {
-        next_unit = virDomainControllerSCSINextUnit(def, max_unit, controller);
+        next_unit = virDomainControllerSCSINextUnit(def, controller);
         if (next_unit < 0)
             controller++;
     } while (next_unit < 0);
@@ -7647,7 +7642,7 @@ virDomainDeviceFindSCSIController(const virDomainDef *def,
 }
 
 int
-virDomainDiskDefAssignAddress(virDomainXMLOption *xmlopt,
+virDomainDiskDefAssignAddress(virDomainXMLOption *xmlopt G_GNUC_UNUSED,
                               virDomainDiskDef *def,
                               const virDomainDef *vmdef)
 {
@@ -7667,14 +7662,14 @@ virDomainDiskDefAssignAddress(virDomainXMLOption *xmlopt,
 
         def->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE;
 
-        if (xmlopt->config.features & VIR_DOMAIN_DEF_FEATURE_WIDE_SCSI) {
+        if (vmdef->scsiBusMaxUnit > SCSI_NARROW_BUS_MAX_CONT_UNIT) {
             /* For a wide SCSI bus we define the default mapping to be
              * 16 units per bus, 1 bus per controller, many controllers.
              * Unit 7 is the SCSI controller itself. Therefore unit 7
              * cannot be assigned to disks and is skipped.
              */
-            controller = idx / 15;
-            unit = idx % 15;
+            controller = idx / (vmdef->scsiBusMaxUnit - 1);
+            unit = idx % (vmdef->scsiBusMaxUnit - 1);
 
             /* Skip the SCSI controller at unit 7 */
             if (unit >= 7)
@@ -19549,7 +19544,7 @@ virDomainDefParseXML(xmlDocPtr xml,
             return NULL;
     }
 
-    if (!(def = virDomainDefNew()))
+    if (!(def = virDomainDefNew(xmlopt)))
         return NULL;
 
     if (virDomainDefParseIDs(def, ctxt, flags, &uuid_generated) < 0)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index ca21082624..80dd1b96a4 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2726,6 +2726,11 @@ struct _virDomainVirtioOptions {
     virTristateSwitch packed;
 };
 
+
+#define SCSI_WIDE_BUS_MAX_CONT_UNIT 16
+#define SCSI_NARROW_BUS_MAX_CONT_UNIT 7
+
+
 /*
  * Guest VM main configuration
  *
@@ -2904,6 +2909,8 @@ struct _virDomainDef {
                              callbacks failed for a non-critical reason
                              (was not able to fill in some data) and thus
                              should be re-run before starting */
+
+    unsigned int scsiBusMaxUnit;
 };
 
 
@@ -3351,7 +3358,7 @@ virDomainGraphicsDefNew(virDomainXMLOption *xmlopt);
 virDomainNetDef *
 virDomainNetDefNew(virDomainXMLOption *xmlopt);
 
-virDomainDef *virDomainDefNew(void);
+virDomainDef *virDomainDefNew(virDomainXMLOption *xmlopt);
 
 void virDomainObjAssignDef(virDomainObj *domain,
                            virDomainDef *def,
diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
index ff20d5548b..6e19cd6092 100644
--- a/src/hyperv/hyperv_driver.c
+++ b/src/hyperv/hyperv_driver.c
@@ -2688,7 +2688,7 @@ hypervDomainGetXMLDesc(virDomainPtr domain, unsigned int flags)
 
     virCheckFlags(VIR_DOMAIN_XML_COMMON_FLAGS, NULL);
 
-    if (!(def = virDomainDefNew()))
+    if (!(def = virDomainDefNew(priv->xmlopt)))
         return NULL;
 
     virUUIDFormat(domain->uuid, uuid_string);
@@ -3058,7 +3058,7 @@ hypervDomainAttachDeviceFlags(virDomainPtr domain, const char *xml, unsigned int
     virUUIDFormat(domain->uuid, uuid_string);
 
     /* get domain definition */
-    if (!(def = virDomainDefNew()))
+    if (!(def = virDomainDefNew(priv->xmlopt)))
         return -1;
 
     /* get domain device definition */
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 0c3c53c1d1..6a3938ead4 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -584,7 +584,7 @@ libxlAddDom0(libxlDriverPrivate *driver)
      * created.
      */
     if ((vm = virDomainObjListFindByID(driver->domains, 0)) == NULL) {
-        if (!(def = virDomainDefNew()))
+        if (!(def = virDomainDefNew(driver->xmlopt)))
             goto cleanup;
 
         def->id = 0;
diff --git a/src/libxl/xen_xl.c b/src/libxl/xen_xl.c
index c0905b08d8..066f39ffe3 100644
--- a/src/libxl/xen_xl.c
+++ b/src/libxl/xen_xl.c
@@ -1136,7 +1136,7 @@ xenParseXL(virConf *conf,
 {
     virDomainDef *def = NULL;
 
-    if (!(def = virDomainDefNew()))
+    if (!(def = virDomainDefNew(xmlopt)))
         return NULL;
 
     def->virtType = VIR_DOMAIN_VIRT_XEN;
diff --git a/src/libxl/xen_xm.c b/src/libxl/xen_xm.c
index feec25b9f4..1e0ea9e5b9 100644
--- a/src/libxl/xen_xm.c
+++ b/src/libxl/xen_xm.c
@@ -421,7 +421,7 @@ xenParseXM(virConf *conf,
 {
     virDomainDef *def = NULL;
 
-    if (!(def = virDomainDefNew()))
+    if (!(def = virDomainDefNew(xmlopt)))
         return NULL;
 
     def->virtType = VIR_DOMAIN_VIRT_XEN;
diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c
index 4bdd960e23..6ebd2e8e75 100644
--- a/src/lxc/lxc_native.c
+++ b/src/lxc/lxc_native.c
@@ -1109,7 +1109,7 @@ lxcParseConfigString(const char *config,
     if (!(properties = virConfReadString(config, VIR_CONF_FLAG_LXC_FORMAT)))
         return NULL;
 
-    if (!(vmdef = virDomainDefNew()))
+    if (!(vmdef = virDomainDefNew(xmlopt)))
         goto error;
 
     if (virUUIDGenerate(vmdef->uuid) < 0) {
diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c
index 2a794801ae..a0ee4f8fb4 100644
--- a/src/openvz/openvz_conf.c
+++ b/src/openvz/openvz_conf.c
@@ -486,7 +486,7 @@ int openvzLoadDomains(struct openvz_driver *driver)
         }
         *line++ = '\0';
 
-        if (!(def = virDomainDefNew()))
+        if (!(def = virDomainDefNew(driver->xmlopt)))
             goto cleanup;
 
         def->virtType = VIR_DOMAIN_VIRT_OPENVZ;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 521fda57da..9dee6954bd 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -9117,7 +9117,7 @@ qemuProcessQMPConnectMonitor(qemuProcessQMP *proc)
 
     if (!(xmlopt = virDomainXMLOptionNew(NULL, NULL, NULL, NULL, NULL)) ||
         !(proc->vm = virDomainObjNew(xmlopt)) ||
-        !(proc->vm->def = virDomainDefNew()))
+        !(proc->vm->def = virDomainDefNew(xmlopt)))
         goto cleanup;
 
     proc->vm->pid = proc->pid;
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 1ca521321c..a6c9877944 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -4009,7 +4009,7 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags)
     if (openSessionForMachine(data, dom->uuid, &iid, &machine) < 0)
         goto cleanup;
 
-    if (!(def = virDomainDefNew()))
+    if (!(def = virDomainDefNew(data->xmlopt)))
         goto cleanup;
 
     gVBoxAPI.UIMachine.GetAccessible(machine, &accessible);
@@ -4252,7 +4252,7 @@ static int vboxDomainAttachDeviceImpl(virDomainPtr dom,
         return ret;
 
     VBOX_IID_INITIALIZE(&iid);
-    if (!(def = virDomainDefNew()))
+    if (!(def = virDomainDefNew(data->xmlopt)))
         return ret;
 
     def->os.type = VIR_DOMAIN_OSTYPE_HVM;
@@ -4371,7 +4371,7 @@ static int vboxDomainDetachDevice(virDomainPtr dom, const char *xml)
         return ret;
 
     VBOX_IID_INITIALIZE(&iid);
-    if (!(def = virDomainDefNew()))
+    if (!(def = virDomainDefNew(data->xmlopt)))
         return ret;
 
     def->os.type = VIR_DOMAIN_OSTYPE_HVM;
@@ -6124,7 +6124,7 @@ static char *vboxDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot,
         goto cleanup;
 
     if (!(def = virDomainSnapshotDefNew()) ||
-        !(def->parent.dom = virDomainDefNew()))
+        !(def->parent.dom = virDomainDefNew(data->xmlopt)))
         goto cleanup;
     defdom = def->parent.dom;
     def->parent.name = g_strdup(snapshot->name);
diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index 11b6fcbdf8..192f1bd252 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -1445,7 +1445,7 @@ virVMXParseConfig(virVMXContext *ctx,
         goto cleanup;
 
     /* Allocate domain def */
-    if (!(def = virDomainDefNew()))
+    if (!(def = virDomainDefNew(xmlopt)))
         goto cleanup;
 
     def->virtType = VIR_DOMAIN_VIRT_VMWARE;
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index e09950812d..91102dca85 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -1809,7 +1809,7 @@ prlsdkLoadDomain(struct _vzDriver *driver,
     PRL_HANDLE job;
     char uuidstr[VIR_UUID_STRING_BRACED_BUFLEN];
 
-    if (!(def = virDomainDefNew()))
+    if (!(def = virDomainDefNew(driver->xmlopt)))
         goto error;
 
     if (!(def->name = prlsdkGetStringParamVar(PrlVmCfg_GetName, sdkdom)))
diff --git a/tests/genericxml2xmltest.c b/tests/genericxml2xmltest.c
index ef51ed91a6..54622ea831 100644
--- a/tests/genericxml2xmltest.c
+++ b/tests/genericxml2xmltest.c
@@ -100,7 +100,7 @@ testCompareBackupXML(const void *opaque)
     }
 
     /* create a fake definition and fill it with disks */
-    if (!(fakedef = virDomainDefNew()))
+    if (!(fakedef = virDomainDefNew(xmlopt)))
         return -1;
 
     fakedef->ndisks = backup->ndisks + 1;
diff --git a/tests/openvzutilstest.c b/tests/openvzutilstest.c
index 136c8011b8..ddb2fcbe6c 100644
--- a/tests/openvzutilstest.c
+++ b/tests/openvzutilstest.c
@@ -103,7 +103,7 @@ testReadNetworkConf(const void *data G_GNUC_UNUSED)
         .caps = openvzCapsInit(),
     };
 
-    if (!(def = virDomainDefNew()))
+    if (!(def = virDomainDefNew(driver.xmlopt)))
         goto cleanup;
 
     def->os.init = g_strdup("/sbin/init");
diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index 0513e5bfc0..308668f2b8 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -279,7 +279,7 @@ testQemuDiskXMLToProps(const void *opaque)
                                        VIR_DOMAIN_DEF_PARSE_STATUS)))
         return -1;
 
-    if (!(vmdef = virDomainDefNew()))
+    if (!(vmdef = virDomainDefNew(data->driver->xmlopt)))
         return -1;
 
     virDomainDiskInsert(vmdef, disk);
diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c
index 143dd954e6..059d0d3b3b 100644
--- a/tests/qemumonitortestutils.c
+++ b/tests/qemumonitortestutils.c
@@ -1047,7 +1047,7 @@ qemuMonitorCommonTestNew(virDomainXMLOption *xmlopt,
         test->vm = virDomainObjNew(xmlopt);
         if (!test->vm)
             goto error;
-        if (!(test->vm->def = virDomainDefNew()))
+        if (!(test->vm->def = virDomainDefNew(xmlopt)))
             goto error;
     }
 
diff --git a/tests/securityselinuxtest.c b/tests/securityselinuxtest.c
index 9b65eeb994..119ad6df34 100644
--- a/tests/securityselinuxtest.c
+++ b/tests/securityselinuxtest.c
@@ -67,7 +67,7 @@ testBuildDomainDef(bool dynamic,
     virDomainDef *def;
     virSecurityLabelDef *secdef = NULL;
 
-    if (!(def = virDomainDefNew()))
+    if (!(def = virDomainDefNew(NULL)))
         goto error;
 
     def->virtType = VIR_DOMAIN_VIRT_KVM;
-- 
2.31.1




More information about the libvir-list mailing list