On 06/15/2017 10:17 AM, sferdjao(a)redhat.com wrote:
From: Sahid Orentino Ferdjaoui <sahid.ferdjaoui(a)redhat.com>
This patch finalizes support of 'poll_us' attribute as an option of
the NIC driver-specific element, the commit also takes care of
hotplug.
<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.
Actually, libvirt's qemu driver will use vhost-net automatically as long
as the qemu binary supports it (unless 1) the domain is using TCG
instead of KVM, or 2) the interface configuration specifically says
"<driver name='qemu" .../>).
Signed-off-by: Sahid Orentino Ferdjaoui <sahid.ferdjaoui(a)redhat.com>
---
docs/formatdomain.html.in | 12 ++++++++++-
docs/schemas/domaincommon.rng | 5 +++++
src/qemu/qemu_command.c | 22 +++++++++++++++++++++
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 | 23 ++++++++++++++++++++++
.../qemuxml2argv-net-virtio-netdev-pollus.xml | 23 ++++++++++++++++++++++
tests/qemuxml2argvtest.c | 9 +++++++++
The successful tests should all be done in qemuxml2xmltest.c as well
9 files changed, 151 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
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 52119f0..7bfeabc 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -5043,7 +5043,7 @@ qemu-kvm -net nic,model=? /dev/null
<model type='virtio'/>
<b><driver name='vhost' txmode='iothread'
ioeventfd='on' event_idx='off' queues='5'
rx_queue_size='256'>
<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'/>
+ <guest csum='off' tso4='off' tso6='off'
ecn='off' ufo='off' poll_us='50'/>
</driver>
</b>
</interface>
@@ -5171,6 +5171,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 4950ddc..f304385 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2687,6 +2687,11 @@
<optional>
<ref name="event_idx"/>
</optional>
+ <optional>
+ <attribute name='poll_us'>
+ <ref name='positiveInteger'/>
+ </attribute>
+ </optional>
</group>
</choice>
<ref name="virtioOptions"/>
As mentioned in the previous patch, the RNG and docs changes should be
in the previous patch. (In this patch you're going to want to add an
entry to docs/news.xml)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 8c12b2b..412496e 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3955,6 +3955,9 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
}
if (net->tune.sndbuf_specified)
virBufferAsprintf(&buf, "sndbuf=%lu,", net->tune.sndbuf);
+ if (net->driver.virtio.pollus)
+ virBufferAsprintf(&buf, "poll-us=%u,",
+ net->driver.virtio.pollus);
}
virObjectUnref(cfg);
@@ -8451,6 +8454,25 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
return -1;
}
+ if (net->driver.virtio.pollus > 0) {
+ if (net->driver.virtio.name != VIR_DOMAIN_NET_BACKEND_TYPE_VHOST) {
I haven't checked - does this get set automatically when the vhost-net
FD's are successfully opened?
+ virReportError(
+ VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Busy polling is only supported with vhost backend
driver"));
I wonder if anyone would be confused by the lack of the exact option
name "poll_us" in these error messages.
+ 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 0b8d3d8..33884b7 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -995,6 +995,18 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
return -1;
}
+ /* and nothing besides TAP devices supports busy polling. */
+ if (net->driver.virtio.pollus > 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;
+ }
Huh. I just noticed there's a lot of duplicated code between
qemuBuildInterfaceCommandLine() and qemuDomainAttachNetDevice()... (not
your problem, but it seems like this is just *screaming* to be
vombined/consolidated to eliminate behavioral differences / half-fixed
bugs etc.
+
/* 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'/>
So this is supposed to fail because the config doesn't explicitly say
"driver name='vhost'". But as I said in the review of patch 2,
vhost-net
is used by default. So this would normally be a correct config.
+ </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'/>
This one is okay - it *should* fail.
+ </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..8d44134
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-net-virtio-netdev-pollus.args
@@ -0,0 +1,23 @@
+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 \
+-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \
+-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 799aea9..54d1b07 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1133,6 +1133,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_VHOST_NET_POLL_US);
+ DO_TEST_FAILURE("net-virtio-netdev-pollus-fail",
+ QEMU_CAPS_NETDEV,
+ QEMU_CAPS_VHOST_NET_POLL_US);
+ DO_TEST_FAILURE("net-virtio-netdev-pollus-qemu-fail",
+ QEMU_CAPS_NETDEV,
+ QEMU_CAPS_VHOST_NET_POLL_US);
DO_TEST("net-virtio-s390",
QEMU_CAPS_VIRTIO_S390);
DO_TEST("net-virtio-ccw",