6:27 a.m.
From: Sahid Orentino Ferdjaoui <sahid.ferdjaoui(a)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
6:27 a.m.
New subject: [libvirt] [PATCHv2 1/3] qemu: detect support for vhost-net busy polling
From: Sahid Orentino Ferdjaoui <sahid.ferdjaoui(a)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(a)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
6:27 a.m.
New subject: [libvirt] [PATCHv2 2/3] conf: introduce 'poll_us' attribute for domain interface
From: Sahid Orentino Ferdjaoui <sahid.ferdjaoui(a)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(a)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
6:27 a.m.
New subject: [libvirt] [PATCHv2 3/3] This patch finalizes support of 'poll_us' attribute as an option of the NIC driver-specific element, the commit also takes care of hotplug.
From: Sahid Orentino Ferdjaoui <sahid.ferdjaoui(a)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(a)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
6:39 a.m.
On Mon, Jul 17, 2017 at 07:27:24 -0400, sferdjao(a)redhat.com wrote:
From: Sahid Orentino Ferdjaoui <sahid.ferdjaoui(a)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
8:55 p.m.
On 07/17/2017 07:39 AM, Peter Krempa wrote:
On Mon, Jul 17, 2017 at 07:27:24 -0400, sferdjao(a)redhat.com wrote:
> From: Sahid Orentino Ferdjaoui <sahid.ferdjaoui(a)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.
4:45 a.m.
On Mon, Jul 17, 2017 at 01:39:13PM +0200, Peter Krempa wrote:
On Mon, Jul 17, 2017 at 07:27:24 -0400, sferdjao(a)redhat.com wrote:
> From: Sahid Orentino Ferdjaoui <sahid.ferdjaoui(a)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 :|
2655
days inactive
2684
days old
6 comments
4 participants
participants (4)
-
Daniel P. Berrange -
Laine Stump -
Peter Krempa -
sferdjao＠redhat.com