[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] Introduce a sub-element <driver> for controller



On 24/04/13 23:27, Eric Blake wrote:
On 04/24/2013 03:24 AM, Osier Yang wrote:
Like what we did for "disk", "filesystem" and "interface", this
introduces sub-element <driver> for "controller", and put the "queues"
into it.
---
  docs/formatdomain.html.in                          | 26 ++++++++++--------
  docs/schemas/domaincommon.rng                      | 14 ++++++----
  src/conf/domain_conf.c                             | 31 +++++++++++++++-------
  .../qemuxml2argv-disk-virtio-scsi-num_queues.xml   |  4 ++-
  4 files changed, 48 insertions(+), 27 deletions(-)
Ah, this is what I was looking for.

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 3dbd58b..9dd283b 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2135,17 +2135,14 @@
        controller.  A "scsi" controller has an optional
        attribute <code>model</code>, which is one of "auto", "buslogic",
        "ibmvscsi", "lsilogic", "lsisas1068", "lsisas1078", "virtio-scsi" or
-      "vmpvscsi".  The attribute <code>queues</code>
and I see that it was on top of your other patch that did
s/num_queues/queues/ - maybe you should have sent the two patches as a
series.

Yeah, I should.


-      (<span class="since">1.0.5 (QEMU and KVM only)</span>) specifies
-      the number of queues for the controller. For best performance, it's
-      recommended to specify a value matching the number of vCPUs.  A "usb"
-      controller has an optional attribute <code>model</code>, which is one
-      of "piix3-uhci", "piix4-uhci", "ehci", "ich9-ehci1", "ich9-uhci1",
-      "ich9-uhci2", "ich9-uhci3", "vt82c686b-uhci", "pci-ohci" or "nec-xhci".
-      Additionally, <span class="since">since 0.10.0</span>, if the USB bus
-      needs to be explicitly disabled for the guest, <code>model='none'</code>
-      may be used.  The PowerPC64 "spapr-vio" addresses do not have an
-      associated controller.
+      "vmpvscsi".  A "usb" controller has an optional attribute
+      <code>model</code>, which is one of "piix3-uhci", "piix4-uhci", "ehci",
+      "ich9-ehci1", "ich9-uhci1", "ich9-uhci2", "ich9-uhci3", "vt82c686b-uhci",
+      "pci-ohci" or "nec-xhci".  Additionally,
+      <span class="since">since 0.10.0</span>, if the USB bus needs to be
+      explicitly disabled for the guest, <code>model='none'</code> may be
+      used.  The PowerPC64 "spapr-vio" addresses do not have an associated
+      controller.
      </p>
<p>
@@ -2156,6 +2153,13 @@
      </p>
<p>
+      An optional sub-element <code>driver</code> (<span class="since">1.0.5)
+      can specify the driver specific options. Currently it only supports
+      attribute <code>queues</code> (QEMU and KVM only), which specifies the
+      number of queues for the controller. For best performance, it's recommended
+      to specify a value matching the number of vCPUs.
Can this sub-element appear for every type of controller, or is it
limited to particular types of controllers?

It's only valid for virtio-scsi controller of qemu driver, but what I thought is we can make it generally for all drivers & all controllers. And do the checking
inside qemu driver instead.


+    </p>
+    <p>
        USB companion controllers have an optional
        sub-element <code>&lt;master&gt;</code> to specify the exact
        relationship of the companion to its master controller.
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index b1c4c2f..d22bb80 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1443,11 +1443,6 @@
                  </choice>
                </attribute>
              </optional>
-            <optional>
-              <attribute name="queues">
-                <ref name="unsignedInt"/>
-              </attribute>
-            </optional>
Spot [1].

            </group>
            <!-- usb has an optional attribute "model", and optional subelement "master" -->
            <group>
@@ -1493,6 +1488,15 @@
            </group>
          </choice>
        </interleave>
+      <optional>
You want the sublement to be inside the interleave, so that the user can

Agreed for the interleave.

specify <address> and <driver> in either order.  Also, if we only
support queues for a particular type of controller (previously, you only
had it under the type='scsi' controller), then this block should
probably appear as part of the scsi group back at [1].

Previously, since the queues is only valid for virtio-scsi controller,
I put in the scsi group, but now we changed to introduce a <driver>
sub-element, I think it makes more sense to make the <driver>
generic, and doesn't hurt to put the "queues" into it.

Osier


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]