[libvirt] [RFC PATCH 00/12] Support multiple PHBs on pSeries guests
Note: if you want to try this out, you'll need to make sure your QEMU binary includes this commit[1]; moreover, that commit is missing a way for libvirt to detect whether the new naming scheme is in place, so this will have to remain an RFC until the QEMU side has been sorted out. Patches 1-3 are just setting up the stage. Patch 4 starts actually introducing the feature, by relaxing some checks that can no longer be as strict. Patches 5-10 puts all the boring bits (XML parsing and formatting, QEMU capabilities) in place. Patch 11 enables the feature at last. Patch 12 introduces a single test, a bunch more will be added before posting this for real (not as RFC). [1] https://github.com/dgibson/qemu/commit/0a6a9ba2adc48a9a5ea7406d1a5fb3c36f007... Andrea Bolognani (12): qemu: Allow qemuBuildControllerDevStr() to return NULL qemu: Tweak index number checking conf: Move index number checking to drivers qemu: Relax pci-root index requirement for pSeries guests schema: Allow <target index='...'/> schema: Add 'spapr-pci-host-bridge' controller model conf: Parse and format <target index='...'/> conf: Add 'spapr-pci-host-bridge' controller model qemu: Automatically pick index and model for pci-root controllers qemu: Introduce QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE qemu: Use multiple PHBs for pSeries guests tests: Add tests for pSeries guests with multiple PHBs docs/schemas/domaincommon.rng | 7 ++ src/bhyve/bhyve_domain.c | 15 +++ src/conf/domain_conf.c | 34 ++++-- src/conf/domain_conf.h | 2 + src/libxl/libxl_domain.c | 14 +++ src/lxc/lxc_domain.c | 14 +++ src/openvz/openvz_driver.c | 14 +++ src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 126 +++++++++++++++++---- src/qemu/qemu_command.h | 9 +- src/qemu/qemu_domain.c | 13 +++ src/qemu/qemu_domain_address.c | 47 +++++++- src/qemu/qemu_hotplug.c | 5 +- src/uml/uml_driver.c | 14 +++ src/vz/vz_driver.c | 14 +++ src/xen/xen_driver.c | 14 +++ .../qemuargv2xmldata/qemuargv2xml-pseries-disk.xml | 5 +- .../qemuargv2xml-pseries-nvram.xml | 5 +- tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml | 1 + .../qemuxml2argv-pseries-phb-simple.args | 26 +++++ .../qemuxml2argv-pseries-phb-simple.xml | 20 ++++ tests/qemuxml2argvtest.c | 5 + .../qemuxml2xmlout-panic-pseries.xml | 5 +- .../qemuxml2xmlout-ppc64-usb-controller-legacy.xml | 5 +- .../qemuxml2xmlout-ppc64-usb-controller.xml | 5 +- .../qemuxml2xmlout-pseries-nvram.xml | 5 +- .../qemuxml2xmlout-pseries-panic-missing.xml | 5 +- .../qemuxml2xmlout-pseries-panic-no-address.xml | 5 +- ...g.xml => qemuxml2xmlout-pseries-phb-simple.xml} | 13 ++- tests/qemuxml2xmltest.c | 4 + 31 files changed, 407 insertions(+), 47 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-phb-simple.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-phb-simple.xml copy tests/qemuxml2xmloutdata/{qemuxml2xmlout-pseries-panic-missing.xml => qemuxml2xmlout-pseries-phb-simple.xml} (69%) -- 2.7.4
We will soon need to be able to return a NULL pointer without the caller considering that an error: to make it possible, change the return type to int and use an out parameter for the string instead. Add some documentation for the function as well. --- src/qemu/qemu_command.c | 53 ++++++++++++++++++++++++++++++++++++------------- src/qemu/qemu_command.h | 9 +++++---- src/qemu/qemu_hotplug.c | 5 ++++- 3 files changed, 48 insertions(+), 19 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 41eecfd..73767fb 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2520,10 +2520,31 @@ qemuBuildUSBControllerDevStr(virDomainControllerDefPtr def, } -char * +/** + * qemuBuildControllerDevStr: + * @domainDef: domain definition + * @def: controller definition + * @qemuCaps: QEMU binary capabilities + * @devstr: device string + * @nusbcontroller: number of USB controllers + * + * Turn @def into a description of the controller that QEMU will understand, + * to be used either on the command line or through the monitor. + * + * The description will be returned in @devstr and can be NULL, eg. when + * passing in one of the built-in controllers. The returned string must be + * freed by the caller. + * + * The number pointed to by @nusbcontroller will be increased by one every + * time the description for a USB controller has been generated successfully. + * + * Returns: 0 on success, <0 on failure + */ +int qemuBuildControllerDevStr(const virDomainDef *domainDef, virDomainControllerDefPtr def, virQEMUCapsPtr qemuCaps, + char **devstr, int *nusbcontroller) { virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -2532,11 +2553,11 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, if (!qemuCheckCCWS390AddressSupport(domainDef, def->info, qemuCaps, "controller")) - return NULL; + return -1; if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) { if ((qemuDomainSetSCSIControllerModel(domainDef, qemuCaps, &model)) < 0) - return NULL; + return -1; } if (!(def->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI && @@ -2544,22 +2565,22 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, if (def->queues) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("'queues' is only supported by virtio-scsi controller")); - return NULL; + return -1; } if (def->cmd_per_lun) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("'cmd_per_lun' is only supported by virtio-scsi controller")); - return NULL; + return -1; } if (def->max_sectors) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("'max_sectors' is only supported by virtio-scsi controller")); - return NULL; + return -1; } if (def->ioeventfd) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("'ioeventfd' is only supported by virtio-scsi controller")); - return NULL; + return -1; } } @@ -2954,11 +2975,12 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, if (virBufferCheckError(&buf) < 0) goto error; - return virBufferContentAndReset(&buf); + *devstr = virBufferContentAndReset(&buf); + return 0; error: virBufferFreeAndReset(&buf); - return NULL; + return -1; } @@ -3053,12 +3075,15 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd, continue; } - virCommandAddArg(cmd, "-device"); - if (!(devstr = qemuBuildControllerDevStr(def, cont, qemuCaps, - &usbcontroller))) + if (qemuBuildControllerDevStr(def, cont, qemuCaps, + &devstr, &usbcontroller) < 0) return -1; - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); + + if (devstr) { + virCommandAddArg(cmd, "-device"); + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); + } } } diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 69fe846..d8478f8 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -119,10 +119,11 @@ char *qemuBuildDriveDevStr(const virDomainDef *def, virQEMUCapsPtr qemuCaps); /* Current, best practice */ -char *qemuBuildControllerDevStr(const virDomainDef *domainDef, - virDomainControllerDefPtr def, - virQEMUCapsPtr qemuCaps, - int *nusbcontroller); +int qemuBuildControllerDevStr(const virDomainDef *domainDef, + virDomainControllerDefPtr def, + virQEMUCapsPtr qemuCaps, + char **devstr, + int *nusbcontroller); int qemuBuildMemoryBackendStr(unsigned long long size, unsigned long long pagesize, diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 97fb272..8a23238 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -523,7 +523,10 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver, if (qemuAssignDeviceControllerAlias(vm->def, priv->qemuCaps, controller) < 0) goto cleanup; - if (!(devstr = qemuBuildControllerDevStr(vm->def, controller, priv->qemuCaps, NULL))) + if (qemuBuildControllerDevStr(vm->def, controller, priv->qemuCaps, &devstr, NULL) < 0) + goto cleanup; + + if (!devstr) goto cleanup; if (VIR_REALLOC_N(vm->def->controllers, vm->def->ncontrollers+1) < 0) -- 2.7.4
Moving the check and rewriting it this way doesn't alter the current behavior, but will allow us to special-case pci-root down the line. --- src/qemu/qemu_command.c | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 73767fb..462a598 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2680,6 +2680,26 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, case VIR_DOMAIN_CONTROLLER_TYPE_PCI: switch ((virDomainControllerModelPCI) def->model) { case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE: + case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE: + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT: + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT: + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT: + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS: + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS: + if (def->idx == 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("index for pci controllers of model '%s' must be > 0"), + virDomainControllerModelPCITypeToString(def->model)); + goto error; + } + break; + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST: + break; + } + switch ((virDomainControllerModelPCI) def->model) { + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE: if (def->opts.pciopts.modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE || def->opts.pciopts.chassisNr == -1) { @@ -2926,12 +2946,6 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, _("wrong function called")); goto error; } - if (def->idx == 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("index for pci controllers of model '%s' must be > 0"), - virDomainControllerModelPCITypeToString(def->model)); - goto error; - } break; case VIR_DOMAIN_CONTROLLER_TYPE_IDE: -- 2.7.4
pSeries guests will soon be allowed to have multiple PHBs (pci-root controllers), which of course means that all but one of them will have a non-zero index; hence, we'll need to relax the current check. However, right now the check is performed in the conf module, which is generic rather than tied to the QEMU driver, and where we don't have information such as the guest machine type available. To make this change of behavior possible down the line, we need to move the check from the XML parser to the driver. Unfortunately, this means duplicating code :( --- src/bhyve/bhyve_domain.c | 15 +++++++++++++++ src/conf/domain_conf.c | 6 ------ src/libxl/libxl_domain.c | 14 ++++++++++++++ src/lxc/lxc_domain.c | 14 ++++++++++++++ src/openvz/openvz_driver.c | 14 ++++++++++++++ src/qemu/qemu_domain.c | 10 ++++++++++ src/uml/uml_driver.c | 14 ++++++++++++++ src/vz/vz_driver.c | 14 ++++++++++++++ src/xen/xen_driver.c | 14 ++++++++++++++ 9 files changed, 109 insertions(+), 6 deletions(-) diff --git a/src/bhyve/bhyve_domain.c b/src/bhyve/bhyve_domain.c index 76b4fac..05c1508 100644 --- a/src/bhyve/bhyve_domain.c +++ b/src/bhyve/bhyve_domain.c @@ -122,6 +122,21 @@ bhyveDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, bhyveDomainDiskDefAssignAddress(driver, disk, def) < 0) return -1; } + + if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER) { + virDomainControllerDefPtr cont = dev->data.controller; + + if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI && + (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT || + cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) && + cont->idx != 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("pci-root and pcie-root controllers " + "should have index 0")); + return -1; + } + } + return 0; } diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 97d42fe..fb33e9c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8844,12 +8844,6 @@ virDomainControllerDefParseXML(xmlNodePtr node, "have an address")); goto error; } - if (def->idx > 0) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("pci-root and pcie-root controllers " - "should have index 0")); - goto error; - } if ((rc = virDomainParseScaledValue("./pcihole64", NULL, ctxt, &bytes, 1024, 1024ULL * ULONG_MAX, false)) < 0) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index ea28c93..844295f 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -379,6 +379,20 @@ libxlDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, virDomainDiskSetFormat(disk, VIR_STORAGE_FILE_RAW); } + if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER) { + virDomainControllerDefPtr cont = dev->data.controller; + + if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI && + (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT || + cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) && + cont->idx != 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("pci-root and pcie-root controllers " + "should have index 0")); + return -1; + } + } + return 0; } diff --git a/src/lxc/lxc_domain.c b/src/lxc/lxc_domain.c index 3a7404f..a50f6fe 100644 --- a/src/lxc/lxc_domain.c +++ b/src/lxc/lxc_domain.c @@ -389,6 +389,20 @@ virLXCDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, dev->data.chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE) dev->data.chr->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LXC; + if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER) { + virDomainControllerDefPtr cont = dev->data.controller; + + if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI && + (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT || + cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) && + cont->idx != 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("pci-root and pcie-root controllers " + "should have index 0")); + return -1; + } + } + return 0; } diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index eaa9ef6..62732f3 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -128,6 +128,20 @@ openvzDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, return -1; } + if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER) { + virDomainControllerDefPtr cont = dev->data.controller; + + if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI && + (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT || + cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) && + cont->idx != 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("pci-root and pcie-root controllers " + "should have index 0")); + return -1; + } + } + return 0; } diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index c187214..3cc827e 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3207,6 +3207,16 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, virDomainNumaGetNodeCount(def->numa)); goto cleanup; } + + if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI && + (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT || + cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) && + cont->idx != 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("pci-root and pcie-root controllers " + "should have index 0")); + goto cleanup; + } } else if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB && cont->model == -1) { /* Pick a suitable default model for the USB controller if none diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index f0c0ad3..0d4b937 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -427,6 +427,20 @@ umlDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, return -1; } + if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER) { + virDomainControllerDefPtr cont = dev->data.controller; + + if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI && + (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT || + cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) && + cont->idx != 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("pci-root and pcie-root controllers " + "should have index 0")); + return -1; + } + } + return 0; } diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 1ca9fd7..35c63b4 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -295,6 +295,20 @@ vzDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, VIR_STRDUP(dev->data.net->model, "e1000") < 0) return -1; + if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER) { + virDomainControllerDefPtr cont = dev->data.controller; + + if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI && + (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT || + cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) && + cont->idx != 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("pci-root and pcie-root controllers " + "should have index 0")); + return -1; + } + } + return 0; } diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 3f9bfa7..10d8f5a 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -363,6 +363,20 @@ xenDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, } } + if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER) { + virDomainControllerDefPtr cont = dev->data.controller; + + if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI && + (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT || + cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) && + cont->idx != 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("pci-root and pcie-root controllers " + "should have index 0")); + return -1; + } + } + return 0; } -- 2.7.4
pSeries guests will soon be allowed to have multiple PHBs (pci-root controllers), meaning the current check on the controller index no longer applies to them. --- src/qemu/qemu_domain.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 3cc827e..77d1107 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3208,7 +3208,10 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, goto cleanup; } - if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI && + /* pSeries guests can have multiple pci-root controllers, + * but other machine types only support a single one */ + if (!qemuDomainMachineIsPSeries(def) && + cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI && (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT || cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) && cont->idx != 0) { -- 2.7.4
--- docs/schemas/domaincommon.rng | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index c64544a..f2f5c3a 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1931,6 +1931,11 @@ </attribute> </optional> <optional> + <attribute name='index'> + <ref name='uint8'/> + </attribute> + </optional> + <optional> <element name='node'> <ref name='unsignedInt'/> </element> -- 2.7.4
--- docs/schemas/domaincommon.rng | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index f2f5c3a..38b4b80 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1889,6 +1889,8 @@ <element name="model"> <attribute name="name"> <choice> + <!-- implementations of 'pci-root' --> + <value>spapr-pci-host-bridge</value> <!-- implementations of 'pci-bridge' --> <value>pci-bridge</value> <!-- implementations of 'dmi-to-pci-bridge' --> -- 2.7.4
--- src/conf/domain_conf.c | 24 ++++++++++++++++++++++++ src/conf/domain_conf.h | 1 + 2 files changed, 25 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index fb33e9c..93e706c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1774,6 +1774,7 @@ virDomainControllerDefNew(virDomainControllerType type) def->opts.pciopts.chassis = -1; def->opts.pciopts.port = -1; def->opts.pciopts.busNr = -1; + def->opts.pciopts.idx = -1; def->opts.pciopts.numaNode = -1; break; case VIR_DOMAIN_CONTROLLER_TYPE_IDE: @@ -8694,6 +8695,7 @@ virDomainControllerDefParseXML(xmlNodePtr node, goto error; } def->idx = idxVal; + VIR_FREE(idx); } cur = node->children; @@ -8725,6 +8727,7 @@ virDomainControllerDefParseXML(xmlNodePtr node, chassis = virXMLPropString(cur, "chassis"); port = virXMLPropString(cur, "port"); busNr = virXMLPropString(cur, "busNr"); + idx = virXMLPropString(cur, "index"); processedTarget = true; } } @@ -8940,6 +8943,23 @@ virDomainControllerDefParseXML(xmlNodePtr node, goto error; } } + if (idx) { + if (virStrToLong_i(idx, NULL, 0, + &def->opts.pciopts.idx) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid index '%s' in PCI controller"), + idx); + goto error; + } + if (def->opts.pciopts.idx < 0 || + def->opts.pciopts.idx > 31) { + virReportError(VIR_ERR_XML_ERROR, + _("PCI controller index '%s' out of range " + "- must be 0-31"), + idx); + goto error; + } + } if (numaNode >= 0) def->opts.pciopts.numaNode = numaNode; break; @@ -20967,6 +20987,7 @@ virDomainControllerDefFormat(virBufferPtr buf, def->opts.pciopts.chassis != -1 || def->opts.pciopts.port != -1 || def->opts.pciopts.busNr != -1 || + def->opts.pciopts.idx != -1 || def->opts.pciopts.numaNode != -1) pciTarget = true; break; @@ -21007,6 +21028,9 @@ virDomainControllerDefFormat(virBufferPtr buf, if (def->opts.pciopts.busNr != -1) virBufferAsprintf(buf, " busNr='%d'", def->opts.pciopts.busNr); + if (def->opts.pciopts.idx != -1) + virBufferAsprintf(buf, " index='%d'", + def->opts.pciopts.idx); if (def->opts.pciopts.numaNode == -1) { virBufferAddLit(buf, "/>\n"); } else { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 1e53cc3..0cf9c58 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -768,6 +768,7 @@ struct _virDomainPCIControllerOpts { int chassis; int port; int busNr; /* used by pci-expander-bus, -1 == unspecified */ + int idx; /* used by spapr-pci-host-bridge, -1 == unspecified */ /* numaNode is a *subelement* of target (to match existing * item in memory target config) -1 == unspecified */ -- 2.7.4
Adding it to the virDomainControllerPCIModelName enumeration is enough for existing code to handle it, so parsing and formatting will work without further tweaking. --- src/conf/domain_conf.c | 4 +++- src/conf/domain_conf.h | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 93e706c..14afd0d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -338,7 +338,9 @@ VIR_ENUM_IMPL(virDomainControllerPCIModelName, "x3130-upstream", "xio3130-downstream", "pxb", - "pxb-pcie") + "pxb-pcie", + "spapr-pci-host-bridge", +); VIR_ENUM_IMPL(virDomainControllerModelSCSI, VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST, "auto", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 0cf9c58..948aaa2 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -694,6 +694,7 @@ typedef enum { VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_XIO3130_DOWNSTREAM, VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PXB, VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PXB_PCIE, + VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_SPAPR_PCI_HOST_BRIDGE, VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_LAST } virDomainControllerPCIModelName; -- 2.7.4
pSeries guests will soon need the new information; luckily, we can figure it out automatically most of the time, so users won't have to bother with it. --- src/qemu/qemu_domain_address.c | 47 ++++++++++++++++++++-- .../qemuargv2xmldata/qemuargv2xml-pseries-disk.xml | 5 ++- .../qemuargv2xml-pseries-nvram.xml | 5 ++- .../qemuxml2xmlout-panic-pseries.xml | 5 ++- .../qemuxml2xmlout-ppc64-usb-controller-legacy.xml | 5 ++- .../qemuxml2xmlout-ppc64-usb-controller.xml | 5 ++- .../qemuxml2xmlout-pseries-nvram.xml | 5 ++- .../qemuxml2xmlout-pseries-panic-missing.xml | 5 ++- .../qemuxml2xmlout-pseries-panic-no-address.xml | 5 ++- 9 files changed, 76 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 20f4ba2..6f7f4ef 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -1845,13 +1845,15 @@ qemuDomainSupportsPCI(virDomainDefPtr def, static void -qemuDomainPCIControllerSetDefaultModelName(virDomainControllerDefPtr cont) +qemuDomainPCIControllerSetDefaultModelName(virDomainControllerDefPtr cont, + virDomainDefPtr def) { int *modelName = &cont->opts.pciopts.modelName; /* make sure it's not already set */ if (*modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) return; + switch ((virDomainControllerModelPCI)cont->model) { case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE: *modelName = VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCI_BRIDGE; @@ -1875,6 +1877,9 @@ qemuDomainPCIControllerSetDefaultModelName(virDomainControllerDefPtr cont) *modelName = VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PXB_PCIE; break; case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: + if (qemuDomainMachineIsPSeries(def)) + *modelName = VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_SPAPR_PCI_HOST_BRIDGE; + break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST: break; @@ -1883,6 +1888,29 @@ qemuDomainPCIControllerSetDefaultModelName(virDomainControllerDefPtr cont) static int +qemuDomainAddressFindNewIndex(virDomainDefPtr def) +{ + int ret = -1; + size_t i; + + for (i = 0; i < def->ncontrollers; i++) { + virDomainControllerDefPtr cont = def->controllers[i]; + + if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) { + if (cont->opts.pciopts.idx > ret) + ret = cont->opts.pciopts.idx; + } + } + + ret++; + if (ret < 0 || ret > 31) + ret = -1; + + return ret; +} + + +static int qemuDomainAddressFindNewBusNr(virDomainDefPtr def) { /* Try to find a nice default for busNr for a new pci-expander-bus. @@ -2142,7 +2170,7 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, * device in qemu) for any controller that doesn't yet * have it set. */ - qemuDomainPCIControllerSetDefaultModelName(cont); + qemuDomainPCIControllerSetDefaultModelName(cont, def); /* set defaults for any other auto-generated config * options for this controller that haven't been @@ -2179,9 +2207,22 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, goto cleanup; } break; + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: + if (!qemuDomainMachineIsPSeries(def)) + break; + if (options->idx == -1) + options->idx = qemuDomainAddressFindNewIndex(def); + if (options->idx == -1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("No valid index is available to " + "auto-assign to bus %d. Must be " + "manually assigned"), + addr->bus); + goto cleanup; + } + break; case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE: case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT: - case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST: break; diff --git a/tests/qemuargv2xmldata/qemuargv2xml-pseries-disk.xml b/tests/qemuargv2xmldata/qemuargv2xml-pseries-disk.xml index 1bad8ee..601d0f7 100644 --- a/tests/qemuargv2xmldata/qemuargv2xml-pseries-disk.xml +++ b/tests/qemuargv2xmldata/qemuargv2xml-pseries-disk.xml @@ -30,7 +30,10 @@ <controller type='usb' index='0'> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> </controller> - <controller type='pci' index='0' model='pci-root'/> + <controller type='pci' index='0' model='pci-root'> + <model name='spapr-pci-host-bridge'/> + <target index='0'/> + </controller> <controller type='scsi' index='0'> <address type='spapr-vio' reg='0x2000'/> </controller> diff --git a/tests/qemuargv2xmldata/qemuargv2xml-pseries-nvram.xml b/tests/qemuargv2xmldata/qemuargv2xml-pseries-nvram.xml index 7e9f864..7787847 100644 --- a/tests/qemuargv2xmldata/qemuargv2xml-pseries-nvram.xml +++ b/tests/qemuargv2xmldata/qemuargv2xml-pseries-nvram.xml @@ -17,7 +17,10 @@ <controller type='usb' index='0'> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> </controller> - <controller type='pci' index='0' model='pci-root'/> + <controller type='pci' index='0' model='pci-root'> + <model name='spapr-pci-host-bridge'/> + <target index='0'/> + </controller> <memballoon model='none'/> <nvram> <address type='spapr-vio' reg='0x4000'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-panic-pseries.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-panic-pseries.xml index 1ed11ce..7fb49fe 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-panic-pseries.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-panic-pseries.xml @@ -17,7 +17,10 @@ <controller type='usb' index='0'> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> </controller> - <controller type='pci' index='0' model='pci-root'/> + <controller type='pci' index='0' model='pci-root'> + <model name='spapr-pci-host-bridge'/> + <target index='0'/> + </controller> <serial type='pty'> <target port='0'/> <address type='spapr-vio' reg='0x30000000'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-ppc64-usb-controller-legacy.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-ppc64-usb-controller-legacy.xml index 33e7810..6d3d51e 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-ppc64-usb-controller-legacy.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-ppc64-usb-controller-legacy.xml @@ -22,7 +22,10 @@ <controller type='usb' index='0'> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> </controller> - <controller type='pci' index='0' model='pci-root'/> + <controller type='pci' index='0' model='pci-root'> + <model name='spapr-pci-host-bridge'/> + <target index='0'/> + </controller> <memballoon model='virtio'> <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> </memballoon> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-ppc64-usb-controller.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-ppc64-usb-controller.xml index 30932e5..659cabe 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-ppc64-usb-controller.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-ppc64-usb-controller.xml @@ -22,7 +22,10 @@ <controller type='usb' index='0' model='pci-ohci'> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> </controller> - <controller type='pci' index='0' model='pci-root'/> + <controller type='pci' index='0' model='pci-root'> + <model name='spapr-pci-host-bridge'/> + <target index='0'/> + </controller> <memballoon model='virtio'> <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> </memballoon> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-nvram.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-nvram.xml index 713f31c..f89b23b 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-nvram.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-nvram.xml @@ -17,7 +17,10 @@ <controller type='usb' index='0'> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> </controller> - <controller type='pci' index='0' model='pci-root'/> + <controller type='pci' index='0' model='pci-root'> + <model name='spapr-pci-host-bridge'/> + <target index='0'/> + </controller> <memballoon model='none'/> <nvram> <address type='spapr-vio' reg='0x4000'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-missing.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-missing.xml index 1ed11ce..7fb49fe 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-missing.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-missing.xml @@ -17,7 +17,10 @@ <controller type='usb' index='0'> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> </controller> - <controller type='pci' index='0' model='pci-root'/> + <controller type='pci' index='0' model='pci-root'> + <model name='spapr-pci-host-bridge'/> + <target index='0'/> + </controller> <serial type='pty'> <target port='0'/> <address type='spapr-vio' reg='0x30000000'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-no-address.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-no-address.xml index 1ed11ce..7fb49fe 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-no-address.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-no-address.xml @@ -17,7 +17,10 @@ <controller type='usb' index='0'> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> </controller> - <controller type='pci' index='0' model='pci-root'/> + <controller type='pci' index='0' model='pci-root'> + <model name='spapr-pci-host-bridge'/> + <target index='0'/> + </controller> <serial type='pty'> <target port='0'/> <address type='spapr-vio' reg='0x30000000'/> -- 2.7.4
This new capability can be used to detect whether a QEMU binary supports the spapr-pci-host-bridge controller. --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml | 1 + 3 files changed, 4 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 6f60a00..21fc089 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -358,6 +358,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "query-cpu-model-expansion", /* 245 */ "virtio-net.host_mtu", "spice-rendernode", + "spapr-pci-host-bridge", ); @@ -1625,6 +1626,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "ivshmem-plain", QEMU_CAPS_DEVICE_IVSHMEM_PLAIN }, { "ivshmem-doorbell", QEMU_CAPS_DEVICE_IVSHMEM_DOORBELL }, { "vhost-scsi", QEMU_CAPS_DEVICE_VHOST_SCSI }, + { "spapr-pci-host-bridge", QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE }, }; static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBalloon[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 0f998c4..ee51088 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -394,6 +394,7 @@ typedef enum { QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION, /* qmp query-cpu-model-expansion */ QEMU_CAPS_VIRTIO_NET_HOST_MTU, /* virtio-net-*.host_mtu */ QEMU_CAPS_SPICE_RENDERNODE, /* -spice rendernode */ + QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE, /* -device spapr-pci-host-bridge */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml b/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml index 92e2781..ae174d2 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml @@ -159,6 +159,7 @@ <flag name='query-qmp-schema'/> <flag name='vhost-scsi'/> <flag name='drive-iotune-group'/> + <flag name='spapr-pci-host-bridge'/> <version>2006000</version> <kvmVersion>0</kvmVersion> <package></package> -- 2.7.4
Additional PHBs (pci-root controllers) will be created for the guest using the spapr-pci-host-bridge QEMU device, if available; the implicit default PHB, while present in the guest configuration, will be skipped. --- src/qemu/qemu_command.c | 47 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 44 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 462a598..412a918 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2940,6 +2940,40 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, def->opts.pciopts.numaNode); break; case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: + if (def->opts.pciopts.modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE || + def->opts.pciopts.idx == -1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("autogenerated pci-root options not set")); + goto error; + } + + /* Skip the implicit one */ + if (def->opts.pciopts.idx == 0) + goto done; + + modelName = virDomainControllerPCIModelNameTypeToString(def->opts.pciopts.modelName); + if (!modelName) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown pci-root model name value %d"), + def->opts.pciopts.modelName); + goto error; + } + if (def->opts.pciopts.modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_SPAPR_PCI_HOST_BRIDGE) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("PCI controller model name '%s' is not valid for a pci-root"), + modelName); + goto error; + } + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("the spapr-pci-host-bridge controller " + "is not supported in this QEMU binary")); + goto error; + } + virBufferAsprintf(&buf, "%s,index=%d,id=%s", + modelName, def->opts.pciopts.idx, + def->info.alias); + break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -2989,6 +3023,7 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, if (virBufferCheckError(&buf) < 0) goto error; + done: *devstr = virBufferContentAndReset(&buf); return 0; @@ -3046,10 +3081,16 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd, continue; } - /* skip pci-root/pcie-root */ + /* skip pcie-root */ if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI && - (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT || - cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT)) + cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) + continue; + + /* Skip pci-root, except for pSeries guests (which actually + * support more than one PCI Host Bridge per guest) */ + if (!qemuDomainMachineIsPSeries(def) && + cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI && + cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) continue; /* first SATA controller on Q35 machines is implicit */ -- 2.7.4
--- .../qemuxml2argv-pseries-phb-simple.args | 26 +++++++++++++ .../qemuxml2argv-pseries-phb-simple.xml | 20 ++++++++++ tests/qemuxml2argvtest.c | 5 +++ .../qemuxml2xmlout-pseries-phb-simple.xml | 43 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 4 ++ 5 files changed, 98 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-phb-simple.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-phb-simple.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-phb-simple.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-phb-simple.args b/tests/qemuxml2argvdata/qemuxml2argv-pseries-phb-simple.args new file mode 100644 index 0000000..fa7d1ee --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-phb-simple.args @@ -0,0 +1,26 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-ppc64 \ +-name QEMUGuest1 \ +-S \ +-M pseries \ +-m 512 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid 1ccfd97d-5eb4-478a-bbe6-88d254c16db7 \ +-nographic \ +-nodefconfig \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-no-acpi \ +-boot c \ +-device spapr-pci-host-bridge,index=1,id=pci \ +-device spapr-pci-host-bridge,index=2,id=pci \ +-usb \ +-chardev pty,id=charserial0 \ +-device spapr-vty,chardev=charserial0,reg=0x30000000 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-phb-simple.xml b/tests/qemuxml2argvdata/qemuxml2argv-pseries-phb-simple.xml new file mode 100644 index 0000000..c25ec0e --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-phb-simple.xml @@ -0,0 +1,20 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid> + <memory unit='KiB'>524288</memory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='ppc64' machine='pseries'>hvm</type> + </os> + <clock offset='utc'/> + <devices> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + <controller type='pci' model='pci-root'/> + <controller type='pci' model='pci-root'/> + <controller type='pci' model='pci-root'/> + <console type='pty'> + <address type="spapr-vio"/> + </console> + <memballoon model="none"/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index ad9ce8e..ae79bcc 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1666,6 +1666,11 @@ mymain(void) QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG); DO_TEST_FAILURE("pseries-panic-address", QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG); + + DO_TEST("pseries-phb-simple", + QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG, + QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE); + DO_TEST("disk-ide-drive-split", QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_IDE_CD); diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-phb-simple.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-phb-simple.xml new file mode 100644 index 0000000..6604151 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-phb-simple.xml @@ -0,0 +1,43 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid> + <memory unit='KiB'>524288</memory> + <currentMemory unit='KiB'>524288</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='ppc64' machine='pseries'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + <controller type='pci' index='0' model='pci-root'> + <model name='spapr-pci-host-bridge'/> + <target index='0'/> + </controller> + <controller type='pci' index='1' model='pci-root'> + <model name='spapr-pci-host-bridge'/> + <target index='1'/> + </controller> + <controller type='pci' index='2' model='pci-root'> + <model name='spapr-pci-host-bridge'/> + <target index='2'/> + </controller> + <controller type='usb' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </controller> + <serial type='pty'> + <target port='0'/> + <address type='spapr-vio' reg='0x30000000'/> + </serial> + <console type='pty'> + <target type='serial' port='0'/> + <address type='spapr-vio' reg='0x30000000'/> + </console> + <memballoon model='none'/> + <panic model='pseries'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 4353ad2..93fa476 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -656,6 +656,10 @@ mymain(void) DO_TEST("pseries-panic-missing", NONE); DO_TEST("pseries-panic-no-address", NONE); + DO_TEST("pseries-phb-simple", + QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG, + QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE); + DO_TEST("balloon-device-auto", NONE); DO_TEST("balloon-device-period", NONE); DO_TEST("channel-virtio-auto", NONE); -- 2.7.4
On Tue, Feb 28, 2017 at 05:12:32PM +0100, Andrea Bolognani wrote:
Note: if you want to try this out, you'll need to make sure your QEMU binary includes this commit[1]; moreover, that commit is missing a way for libvirt to detect whether the new naming scheme is in place, so this will have to remain an RFC until the QEMU side has been sorted out.
[snip]
[1] https://github.com/dgibson/qemu/commit/0a6a9ba2adc48a9a5ea7406d1a5fb3c36f007...
IMHO changing QEMU naming just for sake of making PPC look slightly more like non-PPC architecture is not enough justification for making an incompatible change like this. As you show above, regardless of whether libvirt currently uses this feature or not, it hurts libvirt because when we do add support we need to be able to cope with new and old QEMU. I'd suggest that commit is simply dropped unless there is a clear functional reason for why the naming must be changed, not merely a style reason. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|
On Wed, 2017-03-01 at 11:47 +0000, Daniel P. Berrange wrote:
Note: if you want to try this out, you'll need to make sure your QEMU binary includes this commit[1]; moreover, that commit is missing a way for libvirt to detect whether the new naming scheme is in place, so this will have to remain an RFC until the QEMU side has been sorted out. [snip] [1] https://github.com/dgibson/qemu/commit/0a6a9ba2adc48a9a5ea7406d1a5fb3c36f007... IMHO changing QEMU naming just for sake of making PPC look slightly more like non-PPC architecture is not enough justification for making an incompatible change like this. As you show above, regardless of whether libvirt currently uses this feature or not, it hurts libvirt because when we do add support we need to be able to cope with new and old QEMU. I'd suggest that commit is simply dropped unless there is a clear functional reason for why the naming must be changed, not merely a style reason.
Hm, you have a point. I'll try to see how difficult it would be to teach PCI devices about the quirkiness in bus naming when it comes to PHBs. -- Andrea Bolognani / Red Hat / Virtualization
On Wed, 01 Mar 2017 13:41:10 +0100 Andrea Bolognani <abologna@redhat.com> wrote:
On Wed, 2017-03-01 at 11:47 +0000, Daniel P. Berrange wrote: [...] [...] [...] [...]
Hm, you have a point. I'll try to see how difficult it would be to teach PCI devices about the quirkiness in bus naming when it comes to PHBs.
An update: I'm intending to squeeze the device tree fix to advertise extended config space into qemu-2.9. I'm not going to include the naming change - I'll see how your investigation here turns out before proceeding with that. -- David Gibson <dgibson@redhat.com> Senior Software Engineer, Virtualization, Red Hat
On Fri, 2017-03-03 at 12:37 +1100, David Gibson wrote:
Hm, you have a point. I'll try to see how difficult it would be to teach PCI devices about the quirkiness in bus naming when it comes to PHBs. An update: I'm intending to squeeze the device tree fix to advertise extended config space into qemu-2.9. I'm not going to include the naming change - I'll see how your investigation here turns out before proceeding with that.
Sorry for not keeping you up to date, here's the situation: I got the code working with the current naming scheme and it's not even too big a kludge, so dropping that patch is definitely the right thing to do. Unfortunately I've uncovered a few other issues while writing more test cases, that's why I haven't yet posted a respin. -- Andrea Bolognani / Red Hat / Virtualization
On Fri, 03 Mar 2017 09:07:53 +0100 Andrea Bolognani <abologna@redhat.com> wrote:
On Fri, 2017-03-03 at 12:37 +1100, David Gibson wrote: [...] [...]
Sorry for not keeping you up to date, here's the situation: I got the code working with the current naming scheme and it's not even too big a kludge, so dropping that patch is definitely the right thing to do.
Good to hear.
Unfortunately I've uncovered a few other issues while writing more test cases, that's why I haven't yet posted a respin.
Let me know if you need anything from my side. As I said on today's call, the qemu change to allow access to extended config space has made it in for qemu-2.9. -- David Gibson <dgibson@redhat.com> Senior Software Engineer, Virtualization, Red Hat
participants (3)
-
Andrea Bolognani -
Daniel P. Berrange -
David Gibson