On Sat, Jul 25, 2015 at 03:58:27PM -0400, Laine Stump wrote:
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:
<controller type='pci' model='pci-bridge' index='2'/>
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 <target> 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:
<controller type='pci' model='pci-bridge' index='2'>
<model type='pci-bridge'/>
<target chassisNr='2'/>
</controller>
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 <target> 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.
---
changes from V2:
* check chassisNr for 0-255 range
* multiple <target>s in a single controller is now an error
* 1.3.0 -> 1.2.18
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 +
tests/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 9abceee..fdf7e82 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -3055,6 +3055,30 @@
only).</span>
</p>
<p>
+ PCI controllers also have an optional
+ subelement <code><target></code> 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 <code><target></code> subelement
+ to a controller, nor should you modify the values in the those
+ that are automatically generated by
+ libvirt. <span class="since">Since 1.2.18 (QEMU only).</span>
s/18/19/
+ </p>
+ <dl>
+ <dt><code>chassisNr</code></dt>
+ <dd>
+ PCI controllers that have attribute model="pci-bridge", can
+ also have a <code>chassisNr</code> attribute in
+ the <code><target></code> 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.
+ </dd>
+ </dl>
+ <p>
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.
pci-root has no address.
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d58e679..fbad7e9 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7826,6 +7830,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 <target> elements in "
+ "controller definition not allowed"));
+ goto error;
+ }
+ chassisNr = virXMLPropString(cur, "chassisNr");
+ processedTarget = true;
Similarly to the previous patch you could omit the boolean here. Also there will be way more similarities in following patches that could be coupled in a function. But that's more like suggestion for future since it wouldn't go with the rest of the file, the whole parsing could use a re-factor :) ACK
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list