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

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.

> - (<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?

> + </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

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].