On Fri, Jul 17, 2015 at 02:43:33PM -0400, Laine Stump wrote:
This patch provides qemu support for the contents of <model> in
<controller> for the two existing PCI controller types that need it
(i.e. the two controller types that are backed by a device that must
be specified on the qemu commandline):
1) pci-bridge - sets <model> type attribute default as "pci-bridge"
2) dmi-to-pci-bridge - sets <model> type attribute default as
"i82801b11-bridge".
These both match current hardcoded practice.
The defaults are set at the end of qemuDomainAssignPCIAddresses(), It
can't be done earlier because some of the options that will be
autogenerated need full PCI address info for the controller and
because qemuDomainAssignPCIAddresses() might create extra controllers
which would need default settings added. This is still prior to the
XML being written to disk, though, so the autogenerated defaults are
persistent.
qemu capabilities bits aren't checked until the commandline is
actually created (so the domain can possibly be defined on a host that
doesn't yet have support for the give n device, or a host different
from the one where it will eventually be run). At that time we compare
the type strings to known qemu device names and check for the
capabilities bit for that device.
---
new in V2 (previously was a part of the patch to add pcie-root-port)
src/qemu/qemu_command.c | 70 ++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 64 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 74f02f5..8868e18 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2243,11 +2243,37 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def,
Pity it has to be done in this function, but I understand we need to
have all controllers in and all of them have to have an address
assigned already.
virDomainControllerDefPtr cont =
def->controllers[i];
int idx = cont->idx;
virDevicePCIAddressPtr addr;
+ virDomainPCIControllerOptsPtr options;
+ const char *deviceName = NULL;
if (cont->type != VIR_DOMAIN_CONTROLLER_TYPE_PCI)
continue;
addr = &cont->info.addr.pci;
+ options = &cont->opts.pciopts;
+
+ /* set defaults for any other auto-generated config
+ * options for this controller that haven't been
+ * specified in config.
+ */
+ switch ((virDomainControllerModelPCI)cont->model) {
I'd rather make sure that cont->model >= -1 as you can't know how the
cast will handle the '-1'. Why didn't we go with MODEL_UNDEFINED == 0
in the first place? And it's a fairly recent change, only 5 years back.
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
+ if (!options->type)
+ deviceName = "pci-bridge";
+ break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE:
+ if (!options->type)
+ deviceName = "i82801b11-bridge";
+ break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
+ break;
+ }
+ if (deviceName &&
+ VIR_STRDUP(options->type, deviceName) < 0)
+ goto cleanup;
+
/* check if every PCI bridge controller's ID is greater than
* the bus it is placed onto
*/
@@ -4614,17 +4640,49 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef,
}
switch (def->model) {
case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
- virBufferAsprintf(&buf, "pci-bridge,chassis_nr=%d,id=%s",
+ if (!def->opts.pciopts.type) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("autogenerated pci-bridge options not set"));
+ goto error;
+ }
+ if (STREQ(def->opts.pciopts.type, "pci-bridge")) {
When the type gets changed to enum, I guess these could be simplified
to another function, e.g. qemuControllerModelPCIToCaps(), similarly to
qemuControllerModelUSBToCaps() or qemuSoundCodecTypeToCaps(). It
could even do the check and error setting if unsupported. But that's
just a readability hint.
+ if (!virQEMUCapsGet(qemuCaps,
QEMU_CAPS_DEVICE_PCI_BRIDGE)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("the pci-bridge controller "
+ "is not supported in this QEMU binary"));
+ goto error;
+ }
+ } else {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown pci-bridge device '%s'"),
+ def->opts.pciopts.type);
+ goto error;
+ }
+ virBufferAsprintf(&buf, "%s,chassis_nr=%d,id=%s",
+ def->opts.pciopts.type,
def->idx, def->info.alias);
break;
case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE:
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("The dmi-to-pci-bridge (i82801b11-bridge) "
- "controller is not supported in this QEMU
binary"));
+ if (!def->opts.pciopts.type) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("autogenerated dmi-to-pci-bridge options not
set"));
+ goto error;
+ }
+ if (STREQ(def->opts.pciopts.type, "i82801b11-bridge")) {
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("the dmi-to-pci-bridge (i82801b11-bridge)
"
+ "controller is not supported in this QEMU
binary"));
+ goto error;
+ }
+ } else {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown dmi-to-pci-bridge device
'%s'"),
+ def->opts.pciopts.type);
goto error;
}
- virBufferAsprintf(&buf, "i82801b11-bridge,id=%s",
def->info.alias);
+ virBufferAsprintf(&buf, "%s,id=%s",
+ def->opts.pciopts.type, def->info.alias);
break;
}
break;
--
2.1.0
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list