[libvirt] [PATCHv2 07/17] conf: add new <target> subelement with chassisNr attribute to <controller>

Laine Stump laine at laine.org
Fri Jul 17 18:43:34 UTC 2015


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, a pci-bridge
controller has an option that is called "chassis_nr" in qemu; libvirt
always sets 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.
---

new in V2 (previously was a part of the patch to add pcie-root-port,
but adding the attributes under <model> instead of <target>)

docs/formatdomain.html.in                       | 23 ++++++++++++++++++++
 docs/schemas/domaincommon.rng                   | 10 +++++++++
 src/conf/domain_conf.c                          | 28 +++++++++++++++++++++++--
 src/conf/domain_conf.h                          | 10 ++++++++-
 tests/qemuxml2argvdata/qemuxml2argv-q35.xml     |  1 +
 tests/qemuxml2xmloutdata/qemuxml2xmlout-q35.xml |  1 +
 6 files changed, 70 insertions(+), 3 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index fa46276..ae0d66a 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -3049,6 +3049,29 @@
       libvirt. <span class="since">Since 1.3.0 (QEMU 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.3.0 (QEMU only).</span>
+    </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).
+      </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/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 66518f9..1b1f592 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1744,6 +1744,16 @@
               <empty/>
               </element>
             </optional>
+            <optional>
+              <element name="target">
+                <optional>
+                  <attribute name='chassisNr'>
+                    <ref name='uint8range'/>
+                  </attribute>
+                </optional>
+                <empty/>
+              </element>
+            </optional>
             <!-- *-root controllers have an optional element "pcihole64"-->
             <choice>
               <group>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 380b758..17526d4 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1544,6 +1544,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:
@@ -7638,6 +7640,7 @@ virDomainControllerDefParseXML(xmlNodePtr node,
     char *cmd_per_lun = NULL;
     char *max_sectors = NULL;
     char *guestModel = NULL;
+    char *chassisNr = NULL;
     xmlNodePtr saved = ctxt->node;
     int rc;
 
@@ -7686,6 +7689,9 @@ virDomainControllerDefParseXML(xmlNodePtr node,
             } else if (xmlStrEqual(cur->name, BAD_CAST "model")) {
                 if (!guestModel)
                     guestModel = virXMLPropString(cur, "type");
+            } else if (xmlStrEqual(cur->name, BAD_CAST "target")) {
+                if (!chassisNr)
+                    chassisNr = virXMLPropString(cur, "chassisNr");
             }
         }
         cur = cur->next;
@@ -7798,6 +7804,13 @@ virDomainControllerDefParseXML(xmlNodePtr node,
             def->opts.pciopts.type = guestModel;
             guestModel = 0;
         }
+        if (chassisNr && virStrToLong_i(chassisNr, NULL, 0,
+                                         &def->opts.pciopts.chassisNr) < 0) {
+            virReportError(VIR_ERR_XML_ERROR,
+                           _("Invalid chassisNr '%s' in PCI controller"),
+                           chassisNr);
+            goto error;
+        }
         break;
 
     default:
@@ -7824,6 +7837,7 @@ virDomainControllerDefParseXML(xmlNodePtr node,
     VIR_FREE(cmd_per_lun);
     VIR_FREE(max_sectors);
     VIR_FREE(guestModel);
+    VIR_FREE(chassisNr);
 
     return def;
 
@@ -18833,7 +18847,7 @@ virDomainControllerDefFormat(virBufferPtr buf,
 {
     const char *type = virDomainControllerTypeToString(def->type);
     const char *model = NULL;
-    bool pcihole64 = false, pciModel = false;
+    bool pcihole64 = false, pciModel = false, pciTarget = false;
 
     if (!type) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -18875,13 +18889,15 @@ virDomainControllerDefFormat(virBufferPtr buf,
             pcihole64 = true;
         if (def->opts.pciopts.type)
             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");
@@ -18893,6 +18909,14 @@ virDomainControllerDefFormat(virBufferPtr buf,
             virBufferAddLit(buf, "/>\n");
         }
 
+        if (pciTarget) {
+            virBufferAddLit(buf, "<target");
+            if (def->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, "<driver");
             if (def->queues)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 09fe3c0..32d1073 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -805,7 +805,15 @@ struct _virDomainPCIControllerOpts {
      * similar to the model of <interface> devices.
      */
     char *type; /* 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 4623a5c..815eb08 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-q35.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-q35.xml
@@ -25,6 +25,7 @@
     </controller>
     <controller type='pci' index='2' model='pci-bridge'>
       <model type='pci-bridge'/>
+      <target chassisNr='56'/>
     </controller>
     <video>
       <model type='qxl' ram='65536' vram='32768' vgamem='8192' heads='1'/>
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35.xml
index 760830a..13e4734 100644
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35.xml
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35.xml
@@ -25,6 +25,7 @@
     </controller>
     <controller type='pci' index='2' model='pci-bridge'>
       <model type='pci-bridge'/>
+      <target chassisNr='56'/>
     </controller>
     <controller type='sata' index='0'/>
     <video>
-- 
2.1.0




More information about the libvir-list mailing list