From 8dc88aeed612aeea1ae0d249ec760938cecce5aa Mon Sep 17 00:00:00 2001 From: Laine Stump Date: Wed, 1 Jul 2015 12:47:55 -0400 Subject: [PATCH] conf: add new subelement with chassisNr attribute to There are some configuration options to some types of pci controllers that are currently automatically derived from other parts of the controller's configuration. For example, in qemu a pci-bridge controller has an option that is called "chassis_nr"; up until now libvirt has always set chassis_nr to the index of the pci-bridge. So this: will always result in: -device pci-bridge,chassis_nr=2,... on the qemu commandline. In the future we may decide there is a better way to derive that option, but even in that case we will need for existing domains to retain the same chassis_nr they were using in the past - that is something that is visible to the guest so it is part of the guest ABI and changing it would lead to problems for migrating guests (or just guests with very picky OSes). The subelement has been added as a place to put the new "chassisNr" attribute that will be filled in by libvirt when it auto-generates the chassisNr; it will be saved in the config, then reused any time the domain is started: The one oddity of all this is that if the controller configuration is changed (for example to change the index or the pci address where the controller is plugged in), the items in will *not* be re-generated, which might lead to conflict. I can't really see any way around this, but fortunately if there is a material conflict qemu will let us know and we will pass that on to the user. --- docs/formatdomain.html.in | 24 ++++++++++ docs/schemas/domaincommon.rng | 10 +++++ src/conf/domain_conf.c | 45 ++++++++++++++++++- src/conf/domain_conf.h | 10 ++++- tests/qemuxml2argvdata/qemuxml2argv-q35.xml | 1 + .../qemuxml2xmloutdata/qemuxml2xmlout-q35.xml | 1 + 6 files changed, 88 insertions(+), 3 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 33db6a5c20..6de40e9683 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3056,6 +3056,30 @@ generated by libvirt. Since 1.2.19 (QEMU only).

+

+ PCI controllers also have an optional + subelement <target> with the attributes + listed below. These are configurable items that 1) are visible + to the guest OS so must be preserved for guest ABI + compatibility, and 2) are usually left to default values or + derived automatically by libvirt. In almost all cases, you + should not manually add a <target> subelement + to a controller, nor should you modify the values in the those + that are automatically generated by + libvirt. Since 1.2.19 (QEMU only). +

+
+
chassisNr
+
+ PCI controllers that have attribute model="pci-bridge", can + also have a chassisNr attribute in + the <target> subelement, which is used to + control QEMU's "chassis_nr" option for the pci-bridge device + (normally libvirt automatically sets this to the same value as + the index attribute of the pci controller). If set, chassisNr + must be between 0 and 255. +
+

For machine types which provide an implicit PCI bus, the pci-root controller with index=0 is auto-added and required to use PCI devices. diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index a4c1c9b578..a61e209824 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1744,6 +1744,16 @@ + + + + + + + + + + diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b04f20503e..ec5088c349 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1551,6 +1551,8 @@ virDomainControllerDefNew(virDomainControllerType type) def->opts.vioserial.vectors = -1; break; case VIR_DOMAIN_CONTROLLER_TYPE_PCI: + def->opts.pciopts.chassisNr = -1; + break; case VIR_DOMAIN_CONTROLLER_TYPE_IDE: case VIR_DOMAIN_CONTROLLER_TYPE_FDC: case VIR_DOMAIN_CONTROLLER_TYPE_SCSI: @@ -7810,6 +7812,8 @@ virDomainControllerDefParseXML(xmlNodePtr node, char *max_sectors = NULL; bool processedModel = false; char *modelName = NULL; + bool processedTarget = false; + char *chassisNr = NULL; xmlNodePtr saved = ctxt->node; int rc; @@ -7862,6 +7866,15 @@ virDomainControllerDefParseXML(xmlNodePtr node, } modelName = virXMLPropString(cur, "name"); processedModel = true; + } else if (xmlStrEqual(cur->name, BAD_CAST "target")) { + if (processedTarget) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Multiple elements in " + "controller definition not allowed")); + goto error; + } + chassisNr = virXMLPropString(cur, "chassisNr"); + processedTarget = true; } } cur = cur->next; @@ -7978,6 +7991,23 @@ virDomainControllerDefParseXML(xmlNodePtr node, modelName); goto error; } + if (chassisNr) { + if (virStrToLong_i(chassisNr, NULL, 0, + &def->opts.pciopts.chassisNr) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid chassisNr '%s' in PCI controller"), + chassisNr); + goto error; + } + if (def->opts.pciopts.chassisNr < 0 || + def->opts.pciopts.chassisNr > 255) { + virReportError(VIR_ERR_XML_ERROR, + _("PCI controller chassisNr '%s' out of range " + "- must be 0-255"), + chassisNr); + goto error; + } + } break; default: @@ -8004,6 +8034,7 @@ virDomainControllerDefParseXML(xmlNodePtr node, VIR_FREE(cmd_per_lun); VIR_FREE(max_sectors); VIR_FREE(modelName); + VIR_FREE(chassisNr); return def; @@ -19016,7 +19047,7 @@ virDomainControllerDefFormat(virBufferPtr buf, const char *type = virDomainControllerTypeToString(def->type); const char *model = NULL; const char *modelName = NULL; - bool pcihole64 = false, pciModel = false; + bool pcihole64 = false, pciModel = false, pciTarget = false; if (!type) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -19058,13 +19089,15 @@ virDomainControllerDefFormat(virBufferPtr buf, pcihole64 = true; if (def->opts.pciopts.modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) pciModel = true; + if (def->opts.pciopts.chassisNr != -1) + pciTarget = true; break; default: break; } - if (pciModel || + if (pciModel || pciTarget || def->queues || def->cmd_per_lun || def->max_sectors || virDomainDeviceInfoNeedsFormat(&def->info, flags) || pcihole64) { virBufferAddLit(buf, ">\n"); @@ -19081,6 +19114,14 @@ virDomainControllerDefFormat(virBufferPtr buf, virBufferAsprintf(buf, "\n", modelName); } + if (pciTarget) { + virBufferAddLit(buf, "opts.pciopts.chassisNr != -1) + virBufferAsprintf(buf, " chassisNr='%d'", + def->opts.pciopts.chassisNr); + virBufferAddLit(buf, "/>\n"); + } + if (def->queues || def->cmd_per_lun || def->max_sectors) { virBufferAddLit(buf, "queues) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index fa9937bdd0..cd0b6a7168 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -812,7 +812,15 @@ struct _virDomainPCIControllerOpts { * ... */ int modelName; /* the exact name of the device in hypervisor */ -}; + + /* the following items are attributes of the "target" subelement + * of controller type='pci'. They are bits of configuration that + * are specified on the qemu commandline and are visible to the + * guest OS, so they must be preserved to ensure ABI + * compatibility. + */ + int chassisNr; /* used by pci-bridge, -1 == unspecified */ + }; /* Stores the virtual disk controller configuration */ struct _virDomainControllerDef { diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35.xml b/tests/qemuxml2argvdata/qemuxml2argv-q35.xml index 132c15f452..0c3da859ef 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-q35.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-q35.xml @@ -25,6 +25,7 @@ +