On 05/13/2013 01:22 PM, 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 | 11 ++++- docs/schemas/domaincommon.rng | 5 +++ src/conf/domain_conf.c | 15 +++++++ src/conf/domain_conf.h | 1 + src/qemu/qemu_domain.c | 27 +++++++----- tests/qemuxml2argvdata/qemuxml2argv-event_idx.xml | 2 +- .../qemuxml2argv-net-virtio-device.xml | 2 +- .../qemuxml2argvdata/qemuxml2argv-vhost_queues.xml | 51 ++++++++++++++++++++++ tests/qemuxml2argvdata/qemuxml2argv-virtio-lun.xml | 2 +- tests/qemuxml2xmltest.c | 1 + 10 files changed, 103 insertions(+), 14 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-vhost_queues.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 9ade507..68cd3d4 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3186,7 +3186,7 @@ qemu-kvm -net nic,model=? /dev/null <source network='default'/> <target dev='vnet1'/> <model type='virtio'/> - <b><driver name='vhost' txmode='iothread' ioeventfd='on' event_idx='off'/></b> + <b><driver name='vhost' txmode='iothread' ioeventfd='on' event_idx='off' queues='5'/></b> </interface> </devices> ...</pre> @@ -3280,6 +3280,15 @@ 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">M + ultiqueue virtio-net</a> feature. Long story short, in case of a
"Long story short" is a bit too informal for documentation. Maybe instead you could say something like: If the interface has <model type='virtio'/>, multiple packet processing queues can be created; each queue will potentially be handled by a different processor, resulting in much higher throughput.
+ virtio net device, multiple queues can be created so each queue is + handled by different processor resulting in much higher throughput. + <span class="since">Since 1.0.5 (QEMU and KVM only)</span>
s/1.0.5/1.0.6/
+ </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 8004428..d671491 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1990,6 +1990,11 @@ </attribute> </optional> <optional> + <attribute name='queues'> + <ref name="positiveInteger"/>
Should a lower limit be put on it in the RNG? (does qemu have a documented limit?)
+ </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 862b997..429f0ed 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5935,6 +5935,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; @@ -6046,6 +6047,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", @@ -6336,6 +6338,16 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, } def->driver.virtio.event_idx = idx; } + if (queues) { + unsigned int q; + if (virStrToLong_ui(queues, NULL, 10, &q) < 0) { + virReportError(VIR_ERR_XML_DETAIL, + _("'queues' attribute must be unsigned integer: %s"),
How about "Invalid queues value %s, must be a positive number" or "must be 1 - [some value]"?
+ queues); + goto error; + } + def->driver.virtio.queues = q; + } }
def->linkstate = VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DEFAULT; @@ -6389,6 +6401,7 @@ cleanup: VIR_FREE(txmode); VIR_FREE(ioeventfd); VIR_FREE(event_idx); + VIR_FREE(queues); VIR_FREE(filter); VIR_FREE(type); VIR_FREE(internal); @@ -14354,6 +14367,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 a9d3410..0a30406 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -923,6 +923,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/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 97a8307..ce22afc 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -727,17 +727,24 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, virQEMUDriverPtr driver = opaque; virQEMUDriverConfigPtr cfg = NULL;
- if (dev->type == VIR_DOMAIN_DEVICE_NET && - dev->data.net->type != VIR_DOMAIN_NET_TYPE_HOSTDEV && - !dev->data.net->model) { - if (def->os.arch == VIR_ARCH_S390 || - def->os.arch == VIR_ARCH_S390X) - dev->data.net->model = strdup("virtio"); - else - dev->data.net->model = strdup("rtl8139"); + if (dev->type == VIR_DOMAIN_DEVICE_NET) { + if (dev->data.net->type != VIR_DOMAIN_NET_TYPE_HOSTDEV && + !dev->data.net->model) { + if (def->os.arch == VIR_ARCH_S390 || + def->os.arch == VIR_ARCH_S390X) + dev->data.net->model = strdup("virtio"); + else + dev->data.net->model = strdup("rtl8139");
- if (!dev->data.net->model) - goto no_memory; + if (!dev->data.net->model) + goto no_memory; + } + + if (STREQ(dev->data.net->model, "virtio") && + dev->data.net->driver.virtio.queues == 0) { + /* default number of queues for multiqueue NIC */ + dev->data.net->driver.virtio.queues = 1; + }
Rather than relying on some code in an out-of-the-way post-parse callback to set this to 1, I think it would be better to just interpret 0 as 1 where the value is used. Aside from removing the mystery for someone casually reading the code who doesn't know about the post-parse function, it would also allow us to use "0" as a sentinel value which means "don't emit" during formatting. This is one of those attributes which I think *shouldn't* have its default value auto-filled (as long as migrating from one machine to another with the number of queues changing from source to dest doesn't create a problem, that is).
}
/* set default disk types and drivers */ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-event_idx.xml b/tests/qemuxml2argvdata/qemuxml2argv-event_idx.xml index b3b7e89..44ce184 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-event_idx.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-event_idx.xml @@ -38,7 +38,7 @@ <interface type='user'> <mac address='52:54:00:e5:48:58'/> <model type='virtio'/> - <driver name='vhost' event_idx='off'/> + <driver name='vhost' event_idx='off' queues='1'/>
... and you would then avoid all of these gratuitous changes to xml test data.
</interface> <serial type='pty'> <target port='0'/> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-device.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-device.xml index 1782831..04f3471 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-device.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-device.xml @@ -25,7 +25,7 @@ <interface type='user'> <mac address='00:11:22:33:44:55'/> <model type='virtio'/> - <driver txmode='iothread'/> + <driver txmode='iothread' queues='1'/> </interface> <memballoon model='virtio'/> </devices> 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/qemuxml2argvdata/qemuxml2argv-virtio-lun.xml b/tests/qemuxml2argvdata/qemuxml2argv-virtio-lun.xml index 077ca92..2717439 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-virtio-lun.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-lun.xml @@ -37,7 +37,7 @@ <interface type='user'> <mac address='52:54:00:e5:48:58'/> <model type='virtio'/> - <driver name='vhost' event_idx='off'/> + <driver name='vhost' event_idx='off' queues='1'/> </interface> <serial type='pty'> <target port='0'/> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 492ac60..c06c189 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -244,6 +244,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");