From 5c254bb541f97f7a87458f4b7bf0e31802133047 Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Tue, 3 Aug 2021 11:00:48 +0200 Subject: [PATCH] conf: Store SCSI bus length in virDomainDef MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 newer 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 Reviewed-by: Ján Tomko --- 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 04fd88872c..76dcea6f21 100644 --- a/src/bhyve/bhyve_parse_command.c +++ b/src/bhyve/bhyve_parse_command.c @@ -948,7 +948,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 e8b0f00ad6..e9bdbbfd74 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3801,7 +3801,7 @@ virDomainObjNew(virDomainXMLOption *xmlopt) virDomainDef * -virDomainDefNew(void) +virDomainDefNew(virDomainXMLOption *xmlopt) { virDomainDef *ret; @@ -3814,6 +3814,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: @@ -5138,12 +5143,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}; @@ -5155,22 +5159,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 @@ -5185,7 +5180,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); @@ -7657,7 +7652,7 @@ virDomainDeviceFindSCSIController(const virDomainDef *def, } int -virDomainDiskDefAssignAddress(virDomainXMLOption *xmlopt, +virDomainDiskDefAssignAddress(virDomainXMLOption *xmlopt G_GNUC_UNUSED, virDomainDiskDef *def, const virDomainDef *vmdef) { @@ -7677,14 +7672,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) @@ -19528,7 +19523,7 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, g_autofree xmlNodePtr *nodes = NULL; g_autofree char *tmp = 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 984939cc69..375299f206 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 8996394291..a672901a81 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -2678,7 +2678,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); @@ -3048,7 +3048,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 5d4a273ff1..850786e5c9 100644 --- a/src/libxl/xen_xl.c +++ b/src/libxl/xen_xl.c @@ -1125,7 +1125,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 f978b94f93..ffcd4a7e8f 100644 --- a/src/libxl/xen_xm.c +++ b/src/libxl/xen_xm.c @@ -410,7 +410,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 347d5f4139..3840652912 100644 --- a/src/lxc/lxc_native.c +++ b/src/lxc/lxc_native.c @@ -1103,7 +1103,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 694b048e2b..4acbf0b1a8 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -484,7 +484,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 4264191a9a..fc016ee6fc 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -9110,7 +9110,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 6cb5ba8928..ecdcdbe88d 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -4004,7 +4004,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); @@ -4247,7 +4247,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; @@ -4366,7 +4366,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; @@ -6119,7 +6119,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 a9d0c6fece..46006b47d1 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 6d3a873158..7871b04005 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -1789,7 +1789,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 30d475ebe4..8618e6bbef 100644 --- a/tests/qemumonitortestutils.c +++ b/tests/qemumonitortestutils.c @@ -1038,7 +1038,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;