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

Re: [libvirt] [PATCHv3 01/11] Introduce /domain/devices/interface/driver/@queues attribute



On 05/16/2013 08:49 AM, Michal Privoznik wrote:
> This attribute is going to represent number of queues for
> multique vhost network interface. This commit implements XML
> extension part of the feature and add one test as well. For now,
> we can only do xml2xml test as qemu command line generation code
> is not adapted yet.
> ---
>  docs/formatdomain.html.in                          | 12 ++++-
>  docs/schemas/domaincommon.rng                      |  5 +++
>  src/conf/domain_conf.c                             | 15 +++++++
>  src/conf/domain_conf.h                             |  1 +
>  .../qemuxml2argvdata/qemuxml2argv-vhost_queues.xml | 51 ++++++++++++++++++++++
>  tests/qemuxml2xmltest.c                            |  1 +
>  6 files changed, 84 insertions(+), 1 deletion(-)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-vhost_queues.xml
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 5e89a92..217202d 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -3248,7 +3248,7 @@ qemu-kvm -net nic,model=? /dev/null
>        <source network='default'/>
>        <target dev='vnet1'/>
>        <model type='virtio'/>
> -      <b>&lt;driver name='vhost' txmode='iothread' ioeventfd='on' event_idx='off'/&gt;</b>
> +      <b>&lt;driver name='vhost' txmode='iothread' ioeventfd='on' event_idx='off' queues='5'/&gt;</b>
>      &lt;/interface&gt;
>    &lt;/devices&gt;
>    ...</pre>
> @@ -3342,6 +3342,16 @@ qemu-kvm -net nic,model=? /dev/null
>          <b>In general you should leave this option alone, unless you
>          are very certain you know what you are doing.</b>
>        </dd>
> +      <dt><code>queues</code></dt>
> +      <dd>
> +        The optional <code>queues</code> attribute controls number of
> +        queues for <a href="http://www.linux-kvm.org/page/Multiqueue";>

"the number of queues to be used for the  <a href..."

> +        Multiqueue virtio-net</a> feature. If the interface has <code>&lt;model
> +        type='virtio'/&gt;</code>, multiple packet processing queues can be
> +        created; each queue will potentially be handled by a different
> +        processor, resulting in much higher throughput.
> +        <span class="since">Since 1.0.6 (QEMU and KVM only)</span>
> +      </dd>
>      </dl>
>  
>      <h5><a name="elementsNICSTargetOverride">Overriding the target element</a></h5>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 361794e..500c43c 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -2008,6 +2008,11 @@
>                  </attribute>
>                </optional>
>                <optional>
> +                <attribute name='queues'>
> +                  <ref name="positiveInteger"/>
> +                </attribute>
> +              </optional>
> +              <optional>
>                  <attribute name="txmode">
>                    <choice>
>                      <value>iothread</value>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index f0ca9d5..4a120f6 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -5950,6 +5950,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
>      char *txmode = NULL;
>      char *ioeventfd = NULL;
>      char *event_idx = NULL;
> +    char *queues = NULL;
>      char *filter = NULL;
>      char *internal = NULL;
>      char *devaddr = NULL;
> @@ -6061,6 +6062,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
>                  txmode = virXMLPropString(cur, "txmode");
>                  ioeventfd = virXMLPropString(cur, "ioeventfd");
>                  event_idx = virXMLPropString(cur, "event_idx");
> +                queues = virXMLPropString(cur, "queues");
>              } else if (xmlStrEqual(cur->name, BAD_CAST "filterref")) {
>                  if (filter) {
>                      virReportError(VIR_ERR_XML_ERROR, "%s",
> @@ -6351,6 +6353,16 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
>              }
>              def->driver.virtio.event_idx = idx;
>          }
> +        if (queues) {
> +            unsigned int q;
> +            if (virStrToLong_ui(queues, NULL, 10, &q) < 0) {

Shouldn't this be <= 0 ?

> +                virReportError(VIR_ERR_XML_DETAIL,
> +                               _("'queues' attribute must be unsigned integer: %s"),


I still like "positive number" better, both because it is less
"programmer-ese", and because "unsigned" implies that 0 is okay, but in
this case it isn't.


> +                               queues);
> +                goto error;
> +            }
> +            def->driver.virtio.queues = q;
> +        }
>      }
>  
>      def->linkstate = VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DEFAULT;
> @@ -6404,6 +6416,7 @@ cleanup:
>      VIR_FREE(txmode);
>      VIR_FREE(ioeventfd);
>      VIR_FREE(event_idx);
> +    VIR_FREE(queues);
>      VIR_FREE(filter);
>      VIR_FREE(type);
>      VIR_FREE(internal);
> @@ -14425,6 +14438,8 @@ virDomainNetDefFormat(virBufferPtr buf,
>                  virBufferAsprintf(buf, " event_idx='%s'",
>                                    virDomainVirtioEventIdxTypeToString(def->driver.virtio.event_idx));
>              }
> +            if (def->driver.virtio.queues)
> +                virBufferAsprintf(buf, " queues='%u'", def->driver.virtio.queues);
>              virBufferAddLit(buf, "/>\n");
>          }
>      }
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index c176a4c..507dfd8 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -932,6 +932,7 @@ struct _virDomainNetDef {
>              enum virDomainNetVirtioTxModeType txmode;
>              enum virDomainIoEventFd ioeventfd;
>              enum virDomainVirtioEventIdx event_idx;
> +            unsigned int queues; /* Multiqueue virtio-net */
>          } virtio;
>      } driver;
>      union {
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-vhost_queues.xml b/tests/qemuxml2argvdata/qemuxml2argv-vhost_queues.xml
> new file mode 100644
> index 0000000..76f84f6
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-vhost_queues.xml
> @@ -0,0 +1,51 @@
> +<domain type='qemu'>
> +  <name>test</name>
> +  <uuid>bba65c0e-c049-934f-b6aa-4e2c0582acdf</uuid>
> +  <memory unit='KiB'>1048576</memory>
> +  <currentMemory unit='KiB'>1048576</currentMemory>
> +  <vcpu placement='static'>1</vcpu>
> +  <os>
> +    <type arch='x86_64' machine='pc-0.13'>hvm</type>
> +    <boot dev='cdrom'/>
> +    <boot dev='hd'/>
> +    <bootmenu enable='yes'/>
> +  </os>
> +  <clock offset='utc'/>
> +  <on_poweroff>destroy</on_poweroff>
> +  <on_reboot>restart</on_reboot>
> +  <on_crash>restart</on_crash>
> +  <devices>
> +    <emulator>/usr/bin/qemu</emulator>
> +    <disk type='file' device='disk'>
> +      <driver name='qemu' type='qcow2' event_idx='on'/>
> +      <source file='/var/lib/libvirt/images/f14.img'/>
> +      <target dev='vda' bus='virtio'/>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/>
> +    </disk>
> +    <disk type='file' device='cdrom'>
> +      <driver name='qemu' type='raw'/>
> +      <source file='/var/lib/libvirt/Fedora-14-x86_64-Live-KDE.iso'/>
> +      <target dev='hdc' bus='ide'/>
> +      <readonly/>
> +      <address type='drive' controller='0' bus='1' target='0' unit='0'/>
> +    </disk>
> +    <controller type='usb' index='0'/>
> +    <controller type='virtio-serial' index='0'>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/>
> +    </controller>
> +    <controller type='ide' index='0'/>
> +    <controller type='pci' index='0' model='pci-root'/>
> +    <interface type='user'>
> +      <mac address='52:54:00:e5:48:58'/>
> +      <model type='virtio'/>
> +      <driver name='vhost' queues='5'/>
> +    </interface>
> +    <serial type='pty'>
> +      <target port='0'/>
> +    </serial>
> +    <console type='pty'>
> +      <target type='serial' port='0'/>
> +    </console>
> +    <memballoon model='virtio'/>
> +  </devices>
> +</domain>
> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
> index 148a05a..96f3861 100644
> --- a/tests/qemuxml2xmltest.c
> +++ b/tests/qemuxml2xmltest.c
> @@ -246,6 +246,7 @@ mymain(void)
>      DO_TEST("smp");
>      DO_TEST("lease");
>      DO_TEST("event_idx");
> +    DO_TEST("vhost_queues");
>      DO_TEST("virtio-lun");
>  
>      DO_TEST("usb-redir");


ACK with those issues fixed.


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