[libvirt] [PATCHv2 06/17] qemu: implement <model> subelement to <controller>
Martin Kletzander
mkletzan at redhat.com
Fri Jul 24 13:01:25 UTC 2015
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 at redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150724/a613aa04/attachment-0001.sig>
More information about the libvir-list
mailing list