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"); -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list