[libvirt] [PATCH v2] bandwidth: Adjust documentation

Michal Privoznik mprivozn at redhat.com
Mon Feb 17 14:53:55 UTC 2014


On 13.02.2014 19:33, John Ferlan wrote:
> Recent autotest/virt-test testing on f20 discovered an anomaly in how
> the bandwidth options are documented and used. This was discovered due
> to a bug fix in the /sbin/tc utility found in iproute-3.11.0.1 (on f20)
> in which overflow was actually caught and returned as an error. The fix
> was first introduced in iproute-3.10 (search on iproute2 commit 'a303853e').
>
> The autotest/virt-test test for virsh domiftune was attempting to send
> the largest unsigned integer value (4294967295) for maximum value
> testing. The libvirt xml implementation was designed to manage values
> in kilobytes thus when this value was passed to /sbin/tc, it (now)
> properly rejected the 4294967295kbps value.
>
> Investigation of the problem discovered that formatdomain.html.in and
> formatnetwork.html.in described the elements and property types slightly
> differently, although they use the same code - virNetDevBandwidthParseRate()
> (shared by portgroups, domains, and networks xml parsers). Rather than
> have the descriptions in two places, this patch will combine and reword
> the description under formatnetwork.html.in and have formatdomain.html.in
> link to that description.
>
> This documentation faux pas was continued into the virsh man page where
> the bandwidth description for both 'attach-interface' and 'domiftune'
> did not indicate the format of each value, thus leading to the test using
> largest unsigned integer value assuming "bps" rather than "kbps", which
> ultimately was wrong.
>
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>
> Changes since v1:
>   -> Reflection and some private IRC validation results in changing
>      to use "kilobyte" and not "kibibyte"
>   -> Although the smallest unit is 1 kb/kbps, it's felt that things
>      aren't headed in the going smaller direction, rather things are
>      desired to be larger.  So the inability to "test" the maximum of
>      4294967295 bps in favor of 4294967 kbps shouldn't really matter.
>
>
>   docs/formatdomain.html.in  | 35 +++-----------------
>   docs/formatnetwork.html.in | 81 ++++++++++++++++++++++++++++++++++------------
>   tools/virsh.pod            | 10 ++++--
>   3 files changed, 73 insertions(+), 53 deletions(-)
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 4660983..ea1a97b 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -3750,37 +3750,10 @@ qemu-kvm -net nic,model=? /dev/null
>
>       <p>
>         This part of interface XML provides setting quality of service. Incoming
> -      and outgoing traffic can be shaped independently. The
> -      <code>bandwidth</code> element can have at most one <code>inbound</code>
> -      and at most one <code>outbound</code> child elements. Leaving any of these
> -      children element out result in no QoS applied on that traffic direction.
> -      So, when you want to shape only domain's incoming traffic, use
> -      <code>inbound</code> only, and vice versa. Each of these elements have one
> -      mandatory attribute <code>average</code> (or <code>floor</code> as
> -      described below). <code>average</code> specifies average bit rate on
> -      the interface being shaped. Then there are two optional attributes:
> -      <code>peak</code>, which specifies maximum rate at which interface can send
> -      data, and <code>burst</code>, amount of bytes that can be burst at
> -      <code>peak</code> speed. Accepted values for attributes are integer
> -      numbers. The units for <code>average</code> and <code>peak</code> attributes
> -      are kilobytes per second, and for the <code>burst</code> just kilobytes.
> -      Note the limitation of implementation: the <code>peak</code> attribute in
> -      <code>outbound</code> element is ignored (as linux ingress filters don't
> -      know it yet). <span class="since">Since 0.9.4</span> The <code>inbound</code> can
> -      optionally have <code>floor</code> attribute. This is there for
> -      guaranteeing minimal throughput for shaped interfaces. This, however,
> -      requires that all traffic goes through one point where QoS decisions can
> -      take place. That's why this attribute works only for virtual networks for
> -      now (that is <code><interface type='network'/></code> with a
> -      forward type of route, nat, or no forward at all). Moreover, the
> -      virtual network the interface is connected to is required to have at least
> -      inbound QoS set (<code>average</code> at least). Moreover, with
> -      <code>floor</code> attribute users don't need to specify
> -      <code>average</code>. However, <code>peak</code> and <code>burst</code>
> -      attributes still require <code>average</code>. Currently, linux kernel
> -      doesn't allow ingress qdiscs to have any classes therefore
> -      <code>floor</code> can be applied only on <code>inbound</code> and not
> -      <code>outbound</code>. <span class="since">Since 1.0.1</span>
> +      and outgoing traffic can be shaped independently.
> +      The <code>bandwidth</code> element and its child elements are described
> +      in the <a href="formatnetwork.html#elementQoS">QoS</a> section of
> +      the Network XML.
>       </p>
>
>       <h5><a name="elementVlanTag">Setting VLAN tag (on supported network types only)</a></h5>
> diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in
> index 1ca1bec..d4c390a 100644
> --- a/docs/formatnetwork.html.in
> +++ b/docs/formatnetwork.html.in
> @@ -412,40 +412,81 @@
>
>         <p>
>           The <code><bandwidth></code> element allows setting
> -        quality of service for a particular network.
> -        <span class="since">Since 0.9.4</span> The limits specified
> +        quality of service for a particular network
> +        (<span class="since">since 0.9.4</span>). For a <code>domain</code>
> +        object, the limits specified are applied to the domain traffic.

I'm quite sure about the 'domain traffic'. The <bandwidth/> under domain 
limits the particular <interface/> that has <bandwidth/>. Having 'domain 
traffic' written here may sound like if the domain traffic was 
aggregated and then shaped (which is done in network not in domain).
Maybe 'domain interface traffic'?

Other than that, the patch looks good.

Michal




More information about the libvir-list mailing list