[libvirt] [PATCHv2 0/3] exposing busy polling support for vhost-net
From: Sahid Orentino Ferdjaoui <sahid.ferdjaoui@redhat.com> In version 2.7.0, QEMU introduced support of busy polling for vhost-net [0]. To avoid paraphrasing original authors of that feature and the purpose it I prefer to share a pointer [1]. This patch serie exposes throught the NIC driver-specific element a new option 'poll_us'. That option is only available with the backend driver 'vhost' and that because libvirt automatically fallback to QEMU if the driver is not specified where that option is not available. The option 'poll_us' takes a positive. 0 means that the option is not going to be exposed. [0] 69e87b32680a41d9761191443587c595b6f5fc3f [1] https://lists.gnu.org/archive/html/qemu-devel/2016-11/msg04595.html Sahid Orentino Ferdjaoui (3): qemu: detect support for vhost-net busy polling conf: introduce 'poll_us' attribute for domain interface This patch finalizes support of 'poll_us' attribute as an option of the NIC driver-specific element, the commit also takes care of hotplug. docs/formatdomain.html.in | 12 +++++++- docs/schemas/domaincommon.rng | 5 ++++ src/conf/domain_conf.c | 16 +++++++++++ src/conf/domain_conf.h | 1 + src/qemu/qemu_capabilities.c | 5 ++++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 28 ++++++++++++++++++ src/qemu/qemu_hotplug.c | 12 ++++++++ tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 1 + .../qemuxml2argv-net-virtio-netdev-pollus-fail.xml | 23 +++++++++++++++ ...xml2argv-net-virtio-netdev-pollus-qemu-fail.xml | 23 +++++++++++++++ .../qemuxml2argv-net-virtio-netdev-pollus.args | 25 ++++++++++++++++ .../qemuxml2argv-net-virtio-netdev-pollus.xml | 23 +++++++++++++++ .../qemuxml2argv-net-virtio-poll-us.xml | 23 +++++++++++++++ tests/qemuxml2argvtest.c | 9 ++++++ .../qemuxml2xmlout-net-virtio-poll-us.xml | 33 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 21 files changed, 244 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus-fail.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus-qemu-fail.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-virtio-poll-us.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-net-virtio-poll-us.xml -- 2.9.4
From: Sahid Orentino Ferdjaoui <sahid.ferdjaoui@redhat.com> Detect support for vhost-net busy polling (the "poll-us" commandline option). This was added to upstream qemu in version 2.7.0, commit 69e87b32. Signed-off-by: Sahid Orentino Ferdjaoui <sahid.ferdjaoui@redhat.com> --- Addressed comments: - Commit message updated - s/QEMU_CAPS_VHOST_NET_POLL_US/QEMU_CAPS_NETDEV_POLL_US - s/vhost-net.poll_us/netdev.poll-us src/qemu/qemu_capabilities.c | 5 +++++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 1 + 7 files changed, 11 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 04aa8d5..b3e5a3d 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -431,6 +431,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "virtio.ats", "loadparm", "spapr-pci-host-bridge", + "netdev.poll-us", ); @@ -4826,6 +4827,10 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, if (qemuCaps->version >= 2006000) virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_KERNEL_IRQCHIP_SPLIT); + /* Support for busy polling ("-netdev tap,...,poll-us=n") */ + if (qemuCaps->version >= 2007000) + virQEMUCapsSet(qemuCaps, QEMU_CAPS_NETDEV_POLL_US); + if (virQEMUCapsProbeQMPCommands(qemuCaps, mon) < 0) goto cleanup; diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 8250b57..7638e2e 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -417,6 +417,7 @@ typedef enum { QEMU_CAPS_VIRTIO_PCI_ATS, /* virtio-*-pci.ats */ QEMU_CAPS_LOADPARM, /* -machine loadparm */ QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE, /* -device spapr-pci-host-bridge */ + QEMU_CAPS_NETDEV_POLL_US, /* -netdev poll-us= */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml index ec79115..715b4f7 100644 --- a/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml @@ -131,6 +131,7 @@ <flag name='query-named-block-nodes'/> <flag name='kernel-irqchip'/> <flag name='kernel-irqchip.split'/> + <flag name='netdev.poll-us'/> <version>2007000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml index 5f98db8..66b5409 100644 --- a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml @@ -204,6 +204,7 @@ <flag name='kernel-irqchip'/> <flag name='kernel-irqchip.split'/> <flag name='intel-iommu.intremap'/> + <flag name='netdev.poll-us'/> <version>2007000</version> <kvmVersion>0</kvmVersion> <package> (v2.7.0)</package> diff --git a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml index ac40ecc..03c757e 100644 --- a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml @@ -133,6 +133,7 @@ <flag name='query-named-block-nodes'/> <flag name='kernel-irqchip'/> <flag name='kernel-irqchip.split'/> + <flag name='netdev.poll-us'/> <version>2007093</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml index e42e47a..9b73e80 100644 --- a/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml @@ -206,6 +206,7 @@ <flag name='kernel-irqchip.split'/> <flag name='intel-iommu.intremap'/> <flag name='intel-iommu.eim'/> + <flag name='netdev.poll-us'/> <version>2008000</version> <kvmVersion>0</kvmVersion> <package> (v2.8.0)</package> diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml index ae1530b..54f9cf3 100644 --- a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml @@ -217,6 +217,7 @@ <flag name='intel-iommu.device-iotlb'/> <flag name='virtio.iommu_platform'/> <flag name='virtio.ats'/> + <flag name='netdev.poll-us'/> <version>2009000</version> <kvmVersion>0</kvmVersion> <package> (v2.9.0)</package> -- 2.9.4
From: Sahid Orentino Ferdjaoui <sahid.ferdjaoui@redhat.com> In this commit we introduce 'poll_us' attribute which will be passed then to the QEMU command line to enable support of busy polling. This commit also take into account that attribute to generate the XML. Signed-off-by: Sahid Orentino Ferdjaoui <sahid.ferdjaoui@redhat.com> --- Addressed comments: - Moved the RNG and doc from patch3 to that one - Added an xml2xml test - Renamed var pollus to poll_us docs/formatdomain.html.in | 12 +++++++- docs/schemas/domaincommon.rng | 5 ++++ src/conf/domain_conf.c | 16 +++++++++++ src/conf/domain_conf.h | 1 + .../qemuxml2argv-net-virtio-poll-us.xml | 23 +++++++++++++++ .../qemuxml2xmlout-net-virtio-poll-us.xml | 33 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 7 files changed, 90 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-virtio-poll-us.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-net-virtio-poll-us.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index c12efcf..c0516e3 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -5071,7 +5071,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' queues='5' rx_queue_size='256'> + <b><driver name='vhost' txmode='iothread' ioeventfd='on' event_idx='off' queues='5' rx_queue_size='256' poll_us='50'> <host csum='off' gso='off' tso4='off' tso6='off' ecn='off' ufo='off' mrg_rxbuf='off'/> <guest csum='off' tso4='off' tso6='off' ecn='off' ufo='off'/> </driver> @@ -5201,6 +5201,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> + <dd> + The optional <code>poll_us</code> attribute configure the + maximum number of microseconds that could be spent on busy + polling. It improves the performance, but requires more + CPU. This option is only available with vhost backend driver. + <span class="since">Since 2.7.0 (QEMU and KVM only)</span><br/><br/> + + <b>In general you should leave this option alone, unless you + are very certain you know what you are doing.</b> + </dd> <dt>virtio options</dt> <dd> For virtio interfaces, diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index fc1a40f..0cd2dc0 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2723,6 +2723,11 @@ <optional> <ref name="event_idx"/> </optional> + <optional> + <attribute name='poll_us'> + <ref name='positiveInteger'/> + </attribute> + </optional> </group> </choice> <ref name="virtioOptions"/> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3feeccb..51735dc 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9920,6 +9920,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, char *event_idx = NULL; char *queues = NULL; char *rx_queue_size = NULL; + char *poll_us = NULL; char *str = NULL; char *filter = NULL; char *internal = NULL; @@ -10093,6 +10094,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, event_idx = virXMLPropString(cur, "event_idx"); queues = virXMLPropString(cur, "queues"); rx_queue_size = virXMLPropString(cur, "rx_queue_size"); + poll_us = virXMLPropString(cur, "poll_us"); } else if (xmlStrEqual(cur->name, BAD_CAST "filterref")) { if (filter) { virReportError(VIR_ERR_XML_ERROR, "%s", @@ -10490,6 +10492,17 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, } def->driver.virtio.rx_queue_size = q; } + if (poll_us) { + unsigned int us; + if (virStrToLong_uip(poll_us, NULL, 10, &us) < 0) { + virReportError(VIR_ERR_XML_DETAIL, + _("'poll_us' attribute must be positive: %s"), + poll_us); + goto error; + } + if (us > 1) + def->driver.virtio.poll_us = us; + } if ((str = virXPathString("string(./driver/host/@csum)", ctxt))) { if ((val = virTristateSwitchTypeFromString(str)) <= 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -10687,6 +10700,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, VIR_FREE(event_idx); VIR_FREE(queues); VIR_FREE(rx_queue_size); + VIR_FREE(poll_us); VIR_FREE(str); VIR_FREE(filter); VIR_FREE(type); @@ -22558,6 +22572,8 @@ virDomainVirtioNetDriverFormat(char **outstr, if (def->driver.virtio.rx_queue_size) virBufferAsprintf(&buf, " rx_queue_size='%u'", def->driver.virtio.rx_queue_size); + if (def->driver.virtio.poll_us) + virBufferAsprintf(&buf, " poll_us='%u'", def->driver.virtio.poll_us); virDomainVirtioOptionsFormat(&buf, def->virtio); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index af15ee8..ae3616b 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -969,6 +969,7 @@ struct _virDomainNetDef { virTristateSwitch event_idx; unsigned int queues; /* Multiqueue virtio-net */ unsigned int rx_queue_size; + unsigned int poll_us; /* busy polling for tap */ struct { virTristateSwitch csum; virTristateSwitch gso; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-poll-us.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-poll-us.xml new file mode 100644 index 0000000..46ef7d6 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-poll-us.xml @@ -0,0 +1,23 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <interface type='ethernet'> + <mac address='52:54:00:23:bc:ba'/> + <model type='virtio'/> + <driver name="vhost" poll_us='50'/> + </interface> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-net-virtio-poll-us.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-net-virtio-poll-us.xml new file mode 100644 index 0000000..7f81d15 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-net-virtio-poll-us.xml @@ -0,0 +1,33 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <controller type='usb' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <interface type='ethernet'> + <mac address='52:54:00:23:bc:ba'/> + <model type='virtio'/> + <driver name='vhost' poll_us='50'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </interface> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 5e4b1d1..7abba30 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -538,6 +538,7 @@ mymain(void) DO_TEST("net-eth-hostip", NONE); DO_TEST("net-virtio-network-portgroup", NONE); DO_TEST("net-virtio-rxqueuesize", NONE); + DO_TEST("net-virtio-poll-us", NONE); DO_TEST("net-hostdev", NONE); DO_TEST("net-hostdev-vfio", NONE); DO_TEST("net-midonet", NONE); -- 2.9.4
From: Sahid Orentino Ferdjaoui <sahid.ferdjaoui@redhat.com> <devices> <interface type='ethernet'> <mac address='52:54:00:23:bc:ba'/> <model type='virtio'/> <driver name="vhost" poll_us='50'/> </interface> </devices> That option is supported for all networks which are using a tap device. To be used the backend should be specificly set to use vhost. Signed-off-by: Sahid Orentino Ferdjaoui <sahid.ferdjaoui@redhat.com> --- Addressed comments: - Added comment to explain why we should explicitly set the driver name. src/qemu/qemu_command.c | 28 ++++++++++++++++++++++ src/qemu/qemu_hotplug.c | 12 ++++++++++ .../qemuxml2argv-net-virtio-netdev-pollus-fail.xml | 23 ++++++++++++++++++ ...xml2argv-net-virtio-netdev-pollus-qemu-fail.xml | 23 ++++++++++++++++++ .../qemuxml2argv-net-virtio-netdev-pollus.args | 25 +++++++++++++++++++ .../qemuxml2argv-net-virtio-netdev-pollus.xml | 23 ++++++++++++++++++ tests/qemuxml2argvtest.c | 9 +++++++ 7 files changed, 143 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus-fail.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus-qemu-fail.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus.xml diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6ac26af..1fc2a88 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3899,6 +3899,9 @@ qemuBuildHostNetStr(virDomainNetDefPtr net, } if (net->tune.sndbuf_specified) virBufferAsprintf(&buf, "sndbuf=%lu,", net->tune.sndbuf); + if (net->driver.virtio.poll_us) + virBufferAsprintf(&buf, "poll-us=%u,", + net->driver.virtio.poll_us); } virObjectUnref(cfg); @@ -8330,6 +8333,31 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver, return -1; } + if (net->driver.virtio.poll_us > 0) { + if (net->driver.virtio.name != VIR_DOMAIN_NET_BACKEND_TYPE_VHOST) { + /* virtio.name is set only if driver is specificly + * indicated. Since 'poll_us' option is only available for + * vhost-net we don't want libvirt to fallback to QEMU if + * not available. That's why we request users to set the + * driver. */ + virReportError( + VIR_ERR_CONFIG_UNSUPPORTED, + _("Busy polling (poll_us) is only supported with vhost " + "backend driver")); + return -1; + } + /* Nothing besides TAP devices supports busy polling. */ + if (!(actualType == VIR_DOMAIN_NET_TYPE_NETWORK || + actualType == VIR_DOMAIN_NET_TYPE_BRIDGE || + actualType == VIR_DOMAIN_NET_TYPE_DIRECT || + actualType == VIR_DOMAIN_NET_TYPE_ETHERNET)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Busy polling is not supported for: %s"), + virDomainNetTypeToString(actualType)); + return -1; + } + } + /* and only TAP devices support nwfilter rules */ if (net->filter && !(actualType == VIR_DOMAIN_NET_TYPE_NETWORK || diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 4bc4972..78f61e7 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1015,6 +1015,18 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, return -1; } + /* and nothing besides TAP devices supports busy polling. */ + if (net->driver.virtio.poll_us > 0 && + !(actualType == VIR_DOMAIN_NET_TYPE_NETWORK || + actualType == VIR_DOMAIN_NET_TYPE_BRIDGE || + actualType == VIR_DOMAIN_NET_TYPE_DIRECT || + actualType == VIR_DOMAIN_NET_TYPE_ETHERNET)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Busy polling is not supported for: %s"), + virDomainNetTypeToString(actualType)); + return -1; + } + /* and only TAP devices support nwfilter rules */ if (net->filter && !(actualType == VIR_DOMAIN_NET_TYPE_NETWORK || diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus-fail.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus-fail.xml new file mode 100644 index 0000000..3b664b9 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus-fail.xml @@ -0,0 +1,23 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <interface type='ethernet'> + <mac address='52:54:00:23:bc:ba'/> + <model type='virtio'/> + <driver poll_us='50'/> + </interface> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus-qemu-fail.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus-qemu-fail.xml new file mode 100644 index 0000000..18999c3 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus-qemu-fail.xml @@ -0,0 +1,23 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <interface type='ethernet'> + <mac address='52:54:00:23:bc:ba'/> + <model type='virtio'/> + <driver name="qemu" poll_us='50'/> + </interface> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus.args b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus.args new file mode 100644 index 0000000..15e26fb --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus.args @@ -0,0 +1,25 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-i686 \ +-name QEMUGuest1 \ +-S \ +-M pc \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nographic \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-no-acpi \ +-boot c \ +-usb \ +-netdev tap,fd=3,id=hostnet0,poll-us=50 \ +-device virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:23:bc:ba,bus=pci.0,\ +addr=0x3 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus.xml new file mode 100644 index 0000000..46ef7d6 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus.xml @@ -0,0 +1,23 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <interface type='ethernet'> + <mac address='52:54:00:23:bc:ba'/> + <model type='virtio'/> + <driver name="vhost" poll_us='50'/> + </interface> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index b95ea46..8311d9a 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1156,6 +1156,15 @@ mymain(void) QEMU_CAPS_NODEFCONFIG); DO_TEST("net-virtio-netdev", QEMU_CAPS_NETDEV, QEMU_CAPS_NODEFCONFIG); + DO_TEST("net-virtio-netdev-pollus", + QEMU_CAPS_NETDEV, + QEMU_CAPS_NETDEV_POLL_US); + DO_TEST_FAILURE("net-virtio-netdev-pollus-fail", + QEMU_CAPS_NETDEV, + QEMU_CAPS_NETDEV_POLL_US); + DO_TEST_FAILURE("net-virtio-netdev-pollus-qemu-fail", + QEMU_CAPS_NETDEV, + QEMU_CAPS_NETDEV_POLL_US); DO_TEST("net-virtio-s390", QEMU_CAPS_VIRTIO_S390); DO_TEST("net-virtio-ccw", -- 2.9.4
On Mon, Jul 17, 2017 at 07:27:24 -0400, sferdjao@redhat.com wrote:
From: Sahid Orentino Ferdjaoui <sahid.ferdjaoui@redhat.com>
In version 2.7.0, QEMU introduced support of busy polling for vhost-net [0]. To avoid paraphrasing original authors of that feature and the purpose it I prefer to share a pointer [1].
This patch serie exposes throught the NIC driver-specific element a new option 'poll_us'. That option is only available with the backend driver 'vhost' and that because libvirt automatically fallback to QEMU if the driver is not specified where that option is not available.
The option 'poll_us' takes a positive. 0 means that the option is not going to be exposed.
We had a similar attempt to do this for disk polling, but that was rejected since it's not very straightforward for the users to tune this variable. I think this falls into the same category. Here's the discussion for iothread polling: https://www.redhat.com/archives/libvir-list/2017-February/msg01048.html
On 07/17/2017 07:39 AM, Peter Krempa wrote:
On Mon, Jul 17, 2017 at 07:27:24 -0400, sferdjao@redhat.com wrote:
From: Sahid Orentino Ferdjaoui <sahid.ferdjaoui@redhat.com>
In version 2.7.0, QEMU introduced support of busy polling for vhost-net [0]. To avoid paraphrasing original authors of that feature and the purpose it I prefer to share a pointer [1].
This patch serie exposes throught the NIC driver-specific element a new option 'poll_us'. That option is only available with the backend driver 'vhost' and that because libvirt automatically fallback to QEMU if the driver is not specified where that option is not available.
The option 'poll_us' takes a positive. 0 means that the option is not going to be exposed. We had a similar attempt to do this for disk polling, but that was rejected since it's not very straightforward for the users to tune this variable. I think this falls into the same category.
Here's the discussion for iothread polling:
https://www.redhat.com/archives/libvir-list/2017-February/msg01048.html
The problem is there are already many similar driver-specific odd/not-straightforward tuning parameters present in the XML, so it's difficult to see where to draw the line. I think this may be due to the fact that now whenever a new option is added to qemu, a bugzilla record is semi-automatically opened (by a human, but they almost always do it) requesting libvirt to support that new option, and libvirt developers like to be accommodating (and while some reviews/reviewers base the conversation on "should we even do this?", many/most are simply based on "assuming we want to do this, is this a proper way to do it?", or even "assuming we want to do this, and this is the proper way to do it, has all allocated memory been freed, and error conditions handled?" (e.g. my own review of V1 of these patches :-P). In one of the followups to the iothread polling above, Stefan pointed out a way to accomplish the tuning via qemu commandline passthrough, using qemu's "-set" commandline argument. If libvirt doesn't add support for this attribute in the XML, that's how it will need to be set if someone requires it. And it *is* possible to set poll-us for an interface in this way, e.g.: <qemu:commandline> <qemu:arg value='-set'/> <qemu:arg value='netdev.hostnet1.poll-us=20'/> </qemu:commandline> which results in this addition to the commandline: -set netdev.hostnet1.poll-us=20 As long as the commandline generated for the interface was something like this: -netdev tap,fd=30,id=hostnet1,vhost=on,vhostfd=32 everything will work out properly. There are 3 issues with this though: 1) it depends on the specific interface you want to control having the id ("alias" in libvirt-speak) "hostnet0". If the config is modified to add/remove any interfaces prior to this one in the config, the alias will change, and the qemu:commandline will silently begin modifying the wrong interface. (although we've discussed it, I think it is still not possible to explicitly set the alias for a device - they are always automatically determined when the domain is started / device is hotplugged) 2) libvirt documentation explicitly says that use of <qemu:commandline> is not supported, and any use of it will taint the domain config. This means that nobody will be able to use it in a supported commercial setting. .... Well, I forget what the 3rd reason was, but it was a *good* one, and it made a lot of sense 3 days ago when I started writing this message :-P. Oh, wait, now I remember: 3) It's not possible to set options using <qemu:commandline> when hotplugging a device, so the functionality would only be available for interfaces that were present when the domain was started. Anyway, I'm sympathetic to the sentiment of "don't add frivolous new knobs to libvirt config just because someone asked for it and it's easy to do". I've made that same argument before myself. I just think that if we're going to be restrictive like that, we need to be consistent about it, and need to have a story for how to accommodate the request for the functionality in a different way that doesn't have limited functionality, and doesn't render the software unsupportable. I think that in this case none of those is true.
On Mon, Jul 17, 2017 at 01:39:13PM +0200, Peter Krempa wrote:
On Mon, Jul 17, 2017 at 07:27:24 -0400, sferdjao@redhat.com wrote:
From: Sahid Orentino Ferdjaoui <sahid.ferdjaoui@redhat.com>
In version 2.7.0, QEMU introduced support of busy polling for vhost-net [0]. To avoid paraphrasing original authors of that feature and the purpose it I prefer to share a pointer [1].
This patch serie exposes throught the NIC driver-specific element a new option 'poll_us'. That option is only available with the backend driver 'vhost' and that because libvirt automatically fallback to QEMU if the driver is not specified where that option is not available.
The option 'poll_us' takes a positive. 0 means that the option is not going to be exposed.
We had a similar attempt to do this for disk polling, but that was rejected since it's not very straightforward for the users to tune this variable. I think this falls into the same category.
Here's the discussion for iothread polling:
https://www.redhat.com/archives/libvir-list/2017-February/msg01048.html
Yes, thanks for pointing that out. I do have the same objections to this patch, as for the previous disk polling patch. I just don't think they are practical for a appliction to use - they're too low level to expose to sysadmins, and there's no obvious way for an application to pick the right settings automatically. So I think we're best served by letting QEMU pick defaults Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
participants (4)
-
Daniel P. Berrange -
Laine Stump -
Peter Krempa -
sferdjao＠redhat.com