10:03 a.m.
On Mon, Apr 10, 2017 at 02:21:50PM +0200, Michal Privoznik wrote:
On 04/07/2017 06:04 PM, Martin Kletzander wrote:
> We are currently parsing only rx_max_coalesced_frames because that's
> the only value that makes sense for us. The tun device just added
> support for this one and the others are only supported by hardware
> devices which we don't need to worry about as the only way we'd pass
> those to the domain is using <hostdev/> or <interface
type='hostdev'/>.
> And in those cases the guest can modify the settings itself.
>
> Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
> ---
> docs/formatdomain.html.in | 24 ++++++
> docs/schemas/domaincommon.rng | 3 +
> docs/schemas/networkcommon.rng | 17 ++++
> src/conf/domain_conf.c | 9 +++
> src/conf/domain_conf.h | 2 +
> src/conf/networkcommon_conf.c | 91 ++++++++++++++++++++++
> src/conf/networkcommon_conf.h | 9 +++
> src/libvirt_private.syms | 2 +
> src/qemu/qemu_domain.c | 31 ++++++++
> .../qemuxml2argvdata/qemuxml2argv-net-coalesce.xml | 64 +++++++++++++++
> .../qemuxml2xmlout-net-coalesce.xml | 69 ++++++++++++++++
> tests/qemuxml2xmltest.c | 1 +
> 12 files changed, 322 insertions(+)
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-coalesce.xml
> create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-net-coalesce.xml
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index b1e38f00e423..ea64b7fd1193 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -5405,6 +5405,30 @@ qemu-kvm -net nic,model=? /dev/null
> <span class="since">Since 3.1.0</span>
> </p>
>
> + <h5><a name="coalesce">Coalesce
settings</a></h5>
> +<pre>
> +...
> +<devices>
> + <interface type='network'>
> + <source network='default'/>
> + <target dev='vnet0'/>
> + <b><coalesce>
> +
<rx_max_coalesced_frames>5</rx_max_coalesced_frames>
> + </coalesce></b>
> + </interface>
> +</devices>
> +...</pre>
> +
> + <p>
> + This element provides means of setting coalesce settings for some
> + interface devices (currently only type <code>network</code>
> + and <code>bridge</code>. Currently there is just one sub-element
> + named <code>rx_max_coalesced_frames</code> which accepts a
non-negative
> + integer that specifies the maximum number of packets that will be received
> + before an interrupt.
> + <span class="since">Since 3.3.0</span>
> + </p>
> +
> <h5><a name="ipconfig">IP
configuration</a></h5>
> <pre>
> ...
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index edc225fe50c5..b703515fb897 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -2509,6 +2509,9 @@
> <ref name="mtu"/>
> </optional>
> <optional>
> + <ref name="coalesce"/>
> + </optional>
> + <optional>
> <element name="target">
> <attribute name="dev">
> <ref name="deviceName"/>
> diff --git a/docs/schemas/networkcommon.rng b/docs/schemas/networkcommon.rng
> index 26995556d48d..27e78d13e4a6 100644
> --- a/docs/schemas/networkcommon.rng
> +++ b/docs/schemas/networkcommon.rng
> @@ -275,4 +275,21 @@
> </attribute>
> </element>
> </define>
> +
> + <define name="coalesce">
> + <element name="coalesce">
> + <interleave>
> + <!--
> + Other parameters can just be added here the same way the
> + following one is.
> + -->
> + <optional>
> + <element name="rx_max_coalesced_frames">
> + <ref name="unsignedInt"/>
> + </element>
> + </optional>
> + </interleave>
> + </element>
> + </define>
> +
Not sure what's happening here (one has to love libxml2 error
reporting), but when I try to add the following to my domain XML I get a
schema error:
I wonder why I didn't see the error when tests were running. Where did
you add it?
<coalesce>
<rx_max_coalesced_frames>7</rx_max_coalesced_frames>
</coalesce>
virsh # edit fedora
error: XML document failed to validate against schema: Unable to
validate doc against
/home/zippy/work/libvirt/libvirt.git/docs/schemas/domain.rng
Extra element devices in interleave
Element domain failed to validate content
BTW: is there a reason why this element is defined in networkcommon.rng?
There was. I first had it defined for both domains and networks, but
then scratched the network option as that's not available now and it
would be there for no reason. But I didn't move the definition. I'll
put it in domaincommon.rng in v2.
And a side note, frankly I'm not a big fan of
<elements_with_underscore_in_their_name/> but I guess this is the best
we can do - everybody touching this setting is already familiar with
that from
I first encountered this when adding dumpCore to <memory/>. I first
proposed it as dump_core and that was rejected due to the fact that
we're trying to use camelCase everywhere. OK, so I changed it. Now
when adding this, I 1) forgot about it and 2) saw all the
on_{reboot,poweroff,crash}, event_idx and during this I was working on
another patch where I saw os_type, uri_transports, migration_features,
you get the point. All cpuid features and other things that we're
modelling after something else have the same (usually underscore-based)
naming as the thing they are modelled after.
I'm fine with changing it. Although the code won't be that extensible.
Also, what I was thinking about wen writing it, we could somehow get
prepared for the future by splitting it into categories, e.g.:
<coalesce>
<rx>
<frames max='3' high='100' low='2'/>
<usecs max='3' high='100' low='2'/>
</rx>
<tx>
<frames irq='5'/>
</tx>
</coalesce>
There would be no need for camelCasing, multiple prameters would be way
nicer to read, the XML wouldn't be so huge, it would be extensible, and
maybe something else as well. It's just that for now, someone might not
like it:
<coalesce>
<rx>
<frames max='3'/>
</rx>
</coalesce>
But if it was my choice, I would go with this one ^^ and maybe even
added all of the possible attributes that are there currently.
A very simple and dumb test, just to show how consistent we are (the
regexps are not fool-proof, just the quickest one that popped out of my
head):
Number of camelCase elements
$ sed -n 's/.*element name=.*\(\b[a-z]\+[A-Z][a-zA-Z]*\b\).*/\1/p' $(g ls-files
'*.rng') | sort -u | wc -l
10
Number of camelCase attributes
$ sed -n 's/.*attribute name=.*\(\b[a-z]\+[A-Z][a-zA-Z]*\b\).*/\1/p' $(g
ls-files '*.rng') | sort -u | wc -l
27
Number of under_score elements
$ sed -n 's/.*element name=.*\(\b.*[a-z]_[a-z]*\b\).*/\1/p' $(g ls-files
'*.rng') | sort -u | wc -l
82
Number of under_score attributes
$ sed -n 's/.*attribute name=.*\(\b.*[a-z]_[a-z]*\b\).*/\1/p' $(g ls-files
'*.rng') | sort -u | wc -l
20
>
> +static bool
> +qemuDomainNetSupportsCoalesce(virDomainNetType type)
> +{
> + switch (type) {
> + case VIR_DOMAIN_NET_TYPE_NETWORK:
> + case VIR_DOMAIN_NET_TYPE_BRIDGE:
> + return true;
Maybe I'm running old kernel, but I get ENOSUPP for type network.
Do you get it from libvirt or from the kernel? In upstream, it's in
4.11-rc3 onwards. I hpe I didn't forget to mention it in the commit
message or cover letter.