[libvirt] [PATCH v4 0/7] Add support for setting QoS

This patch series add support for setting traffic shaping and policing on both domain's interface and network's virtual bridge. Basically, this is done via 'tc' from iproute2 package. For shaping is HTB used, for policing we need u32 match selector. Both should be available in RHEL-6 kernel. How this works: On an virtual interface which has limits defined a root qdisc are replaced. Ingress root for outbound traffic shaping and egress for inbound. Basically, in inbound traffic policing is applied, on outbound shaping. New qdiscs are set to limit the traffic to rate set in XML. For shaping it is possible to set the size of buffer. Accepted values for rate, peak and burst are integer numbers. Units are kilobytes per second for rate or kilobytes for size. Supported devices are VIR_DOMAIN_NET_TYPE_NETWORK, VIR_DOMAIN_NET_TYPE_BRIDGE and VIR_DOMAIN_NET_TYPE_DIRECT. diff to v3: -Laine's review included diff to v2: -Jirka's review included diff to v1: -rebase to current HEAD -add support for macvtap devices Michal Privoznik (7): bandwidth: Define schema and create documentation bandwidth: Declare internal structures bandwidth: Add parsing and free functions bandwidth: Create format functions bandwidth: Implement functions to enable and disable QoS bandwidth: Add test cases for network bandwidth: Add domain schema test suite configure.ac | 4 + docs/formatdomain.html.in | 34 +++ docs/formatnetwork.html.in | 30 ++ docs/schemas/domain.rng | 3 + docs/schemas/network.rng | 3 + docs/schemas/networkcommon.rng | 48 +++ libvirt.spec.in | 2 +- src/conf/domain_conf.c | 8 + src/conf/domain_conf.h | 1 + src/conf/network_conf.c | 10 + src/conf/network_conf.h | 1 + src/libvirt_private.syms | 5 + src/network/bridge_driver.c | 18 ++ src/qemu/qemu_command.c | 11 +- src/util/macvtap.c | 12 +- src/util/macvtap.h | 3 +- src/util/network.c | 380 +++++++++++++++++++++++++ src/util/network.h | 22 ++ tests/domainschemadata/domain-bandwidth.xml | 72 +++++ tests/networkxml2xmlin/bandwidth-network.xml | 16 + tests/networkxml2xmlout/bandwidth-network.xml | 16 + tests/networkxml2xmltest.c | 1 + 22 files changed, 696 insertions(+), 4 deletions(-) create mode 100644 tests/domainschemadata/domain-bandwidth.xml create mode 100644 tests/networkxml2xmlin/bandwidth-network.xml create mode 100644 tests/networkxml2xmlout/bandwidth-network.xml -- 1.7.5.rc3

Define new 'bandwidth' element with possible child element 'inbound' and 'outbound' addressing incoming and outgoing traffic respectively: <bandwidth> <inbound average='1000' peak='2000' burst='5120'/> <outbound average='500'/> </bandwidth> Leaving any element out means not to shape traffic in that direction. The units for average and peak (rate) are in kilobytes per second, for burst (size) are just in kilobytes. This element can be inserted into domain's 'interface' and 'network'. --- docs/formatdomain.html.in | 34 ++++++++++++++++++++++++++++ docs/formatnetwork.html.in | 30 +++++++++++++++++++++++++ docs/schemas/domain.rng | 3 ++ docs/schemas/network.rng | 3 ++ docs/schemas/networkcommon.rng | 48 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 118 insertions(+), 0 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 83146ed..f40afc4 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1882,6 +1882,40 @@ qemu-kvm -net nic,model=? /dev/null <span class="since">Since 0.8.8</span> </p> + <h5><a name="elementQoS">Quality of service</a></h5> + +<pre> + ... + <devices> + <interface type='network'> + <source network='default'/> + <target dev='vnet0'/> + <b><bandwidth> + <inbound average='1000' peak='5000' burst='1024'/> + <outbound average='128' peak='256' burst='256'/> + </bandwidth></b> + </interface> + <devices> + ...</pre> + + <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>. It specifies average bit rate on + 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. + <span class="since">Since 0.9.4</span> + </p> + <h4><a name="elementsInput">Input devices</a></h4> <p> diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in index f9421c3..f0ff703 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -101,6 +101,36 @@ to this host. <span class="since">Since 0.3.0; 'mode' attribute since 0.4.2</span></dd> </dl> + <h5><a name="elementQoS">Quality of service</a></h5> + +<pre> +... + <forward mode='nat' dev='eth0'/> + <b><bandwidth> + <inbound average='1000' peak='5000' burst='5120'/> + <outbound average='128' peak='256' burst='256'/> + </bandwidth></b> + <mac address='00:16:3E:5D:C7:9E'/> +...</pre> + + <p> + This part of network 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 network's incoming traffic, use + <code>inbound</code> only, and vice versa. Each of these elements have one + mandatory attribute <code>average</code>. It specifies average bit rate on + interface being shaped. Then there are two optional attributes: + <code>peak</code>, which specifies maximum rate at which bridge 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. + The rate is shared equally within domains connected to the network. + <span class="since">Since 0.9.4</span> + </p> <h3><a name="elementsAddress">Addressing</a></h3> diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index 43326ab..aa4ce69 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -1189,6 +1189,9 @@ <optional> <ref name="deviceBoot"/> </optional> + <optional> + <ref name="bandwidth"/> + </optional> </interleave> </define> <!-- diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng index 1adb553..1c44471 100644 --- a/docs/schemas/network.rng +++ b/docs/schemas/network.rng @@ -147,6 +147,9 @@ </zeroOrMore> </element> </optional> + <optional> + <ref name="bandwidth"/> + </optional> <!-- <ip> element --> <zeroOrMore> diff --git a/docs/schemas/networkcommon.rng b/docs/schemas/networkcommon.rng index 0251813..3a168c3 100644 --- a/docs/schemas/networkcommon.rng +++ b/docs/schemas/networkcommon.rng @@ -47,4 +47,52 @@ </group> </choice> </define> + + <define name="bandwidth"> + <element name="bandwidth"> + <interleave> + <optional> + <element name="inbound"> + <ref name="bandwidth-attributes"/> + <empty/> + </element> + </optional> + <optional> + <element name="outbound"> + <ref name="bandwidth-attributes"/> + <empty/> + </element> + </optional> + </interleave> + </element> + </define> + + <define name="bandwidth-attributes"> + <attribute name="average"> + <ref name="speed"/> + </attribute> + <optional> + <attribute name="peak"> + <ref name="speed"/> + </attribute> + </optional> + <optional> + <attribute name='burst'> + <ref name="BurstSize"/> + </attribute> + </optional> + </define> + + <define name="speed"> + <data type="unsignedInt"> + <param name="pattern">[0-9]+</param> + <param name="minInclusive">1</param> + </data> + </define> + <define name="BurstSize"> + <data type="unsignedInt"> + <param name="pattern">[0-9]+</param> + <param name="minInclusive">1</param> + </data> + </define> </grammar> -- 1.7.5.rc3

On 07/22/2011 10:07 AM, Michal Privoznik wrote:
Define new 'bandwidth' element with possible child element 'inbound' and 'outbound' addressing incoming and outgoing traffic respectively:
<bandwidth> <inbound average='1000' peak='2000' burst='5120'/> <outbound average='500'/> </bandwidth>
Leaving any element out means not to shape traffic in that direction. The units for average and peak (rate) are in kilobytes per second, for burst (size) are just in kilobytes. This element can be inserted into domain's 'interface' and 'network'. --- docs/formatdomain.html.in | 34 ++++++++++++++++++++++++++++ docs/formatnetwork.html.in | 30 +++++++++++++++++++++++++ docs/schemas/domain.rng | 3 ++ docs/schemas/network.rng | 3 ++ docs/schemas/networkcommon.rng | 48 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 118 insertions(+), 0 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 83146ed..f40afc4 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1882,6 +1882,40 @@ qemu-kvm -net nic,model=? /dev/null <span class="since">Since 0.8.8</span> </p>
+<h5><a name="elementQoS">Quality of service</a></h5> + +<pre> + ... +<devices> +<interface type='network'> +<source network='default'/> +<target dev='vnet0'/> +<b><bandwidth> +<inbound average='1000' peak='5000' burst='1024'/> +<outbound average='128' peak='256' burst='256'/> +</bandwidth></b> +</interface> +<devices> + ...</pre> + +<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>. It specifies average bit rate on + 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. +<span class="since">Since 0.9.4</span> +</p> + <h4><a name="elementsInput">Input devices</a></h4>
<p> diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in index f9421c3..f0ff703 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -101,6 +101,36 @@ to this host.<span class="since">Since 0.3.0; 'mode' attribute since 0.4.2</span></dd> </dl> +<h5><a name="elementQoS">Quality of service</a></h5> + +<pre> +... +<forward mode='nat' dev='eth0'/> +<b><bandwidth> +<inbound average='1000' peak='5000' burst='5120'/> +<outbound average='128' peak='256' burst='256'/> +</bandwidth></b> +<mac address='00:16:3E:5D:C7:9E'/> +...</pre> + +<p> + This part of network 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 network's incoming traffic, use +<code>inbound</code> only, and vice versa. Each of these elements have one + mandatory attribute<code>average</code>. It specifies average bit rate on + interface being shaped. Then there are two optional attributes: +<code>peak</code>, which specifies maximum rate at which bridge 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. + The rate is shared equally within domains connected to the network. +<span class="since">Since 0.9.4</span> +</p>
<h3><a name="elementsAddress">Addressing</a></h3>
diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index 43326ab..aa4ce69 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -1189,6 +1189,9 @@ <optional> <ref name="deviceBoot"/> </optional> +<optional> +<ref name="bandwidth"/> +</optional> </interleave> </define> <!-- diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng index 1adb553..1c44471 100644 --- a/docs/schemas/network.rng +++ b/docs/schemas/network.rng @@ -147,6 +147,9 @@ </zeroOrMore> </element> </optional> +<optional> +<ref name="bandwidth"/> +</optional>
<!--<ip> element --> <zeroOrMore> diff --git a/docs/schemas/networkcommon.rng b/docs/schemas/networkcommon.rng index 0251813..3a168c3 100644 --- a/docs/schemas/networkcommon.rng +++ b/docs/schemas/networkcommon.rng @@ -47,4 +47,52 @@ </group> </choice> </define> + +<define name="bandwidth"> +<element name="bandwidth"> +<interleave> +<optional> +<element name="inbound"> +<ref name="bandwidth-attributes"/> +<empty/> +</element> +</optional> +<optional> +<element name="outbound"> +<ref name="bandwidth-attributes"/> +<empty/> +</element> +</optional> +</interleave> +</element> +</define> + +<define name="bandwidth-attributes"> +<attribute name="average"> +<ref name="speed"/> +</attribute> +<optional> +<attribute name="peak"> +<ref name="speed"/> +</attribute> +</optional> +<optional> +<attribute name='burst'> +<ref name="BurstSize"/> +</attribute> +</optional> +</define> + +<define name="speed"> +<data type="unsignedInt"> +<param name="pattern">[0-9]+</param> +<param name="minInclusive">1</param> +</data> +</define> +<define name="BurstSize"> +<data type="unsignedInt"> +<param name="pattern">[0-9]+</param> +<param name="minInclusive">1</param> +</data> +</define> </grammar>
ACK.

--- src/util/network.h | 14 ++++++++++++++ 1 files changed, 14 insertions(+), 0 deletions(-) diff --git a/src/util/network.h b/src/util/network.h index 27347f8..3c090d8 100644 --- a/src/util/network.h +++ b/src/util/network.h @@ -48,6 +48,20 @@ typedef struct { typedef virSocketAddr *virSocketAddrPtr; +typedef struct { + unsigned long long average; /* kbytes/s */ + unsigned long long peak; /* kbytes/s */ + unsigned long long burst; /* kbytes */ +} virRate; + +typedef virRate *virRatePtr; + +typedef struct { + virRatePtr in, out; +} virBandwidth; + +typedef virBandwidth *virBandwidthPtr; + int virSocketParseAddr (const char *val, virSocketAddrPtr addr, int hint); -- 1.7.5.rc3

On 07/22/2011 10:07 AM, Michal Privoznik wrote:
--- src/util/network.h | 14 ++++++++++++++ 1 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/src/util/network.h b/src/util/network.h index 27347f8..3c090d8 100644 --- a/src/util/network.h +++ b/src/util/network.h @@ -48,6 +48,20 @@ typedef struct {
typedef virSocketAddr *virSocketAddrPtr;
+typedef struct { + unsigned long long average; /* kbytes/s */ + unsigned long long peak; /* kbytes/s */ + unsigned long long burst; /* kbytes */ +} virRate; + +typedef virRate *virRatePtr; + +typedef struct { + virRatePtr in, out; +} virBandwidth; + +typedef virBandwidth *virBandwidthPtr; + int virSocketParseAddr (const char *val, virSocketAddrPtr addr, int hint);
ACK

These functions parse given XML node and return pointer to the output. Unknown elements are silently ignored. Attributes must be integer and must fit in unsigned long long. Free function frees elements of virBandwidth structure. --- src/conf/domain_conf.c | 5 ++ src/conf/domain_conf.h | 1 + src/conf/network_conf.c | 7 ++ src/conf/network_conf.h | 1 + src/libvirt_private.syms | 2 + src/util/network.c | 147 ++++++++++++++++++++++++++++++++++++++++++++++ src/util/network.h | 2 + 7 files changed, 165 insertions(+), 0 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 919a75a..e11ad98 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -810,6 +810,8 @@ void virDomainNetDefFree(virDomainNetDefPtr def) VIR_FREE(def->filter); virNWFilterHashTableFree(def->filterparams); + virBandwidthDefFree(def->bandwidth); + VIR_FREE(def); } @@ -2823,6 +2825,9 @@ virDomainNetDefParseXML(virCapsPtr caps, xmlStrEqual(cur->name, BAD_CAST "actual")) { if (virDomainActualNetDefParseXML(cur, ctxt, &actual) < 0) goto error; + } else if (xmlStrEqual(cur->name, BAD_CAST "bandwidth")) { + if (!(def->bandwidth = virBandwidthDefParseNode(cur))) + goto error; } } cur = cur->next; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 551946b..212f0c5 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -425,6 +425,7 @@ struct _virDomainNetDef { virDomainDeviceInfo info; char *filter; virNWFilterHashTablePtr filterparams; + virBandwidthPtr bandwidth; }; enum virDomainChrDeviceType { diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 860eea3..01c094c 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -167,6 +167,8 @@ void virNetworkDefFree(virNetworkDefPtr def) virNetworkDNSDefFree(def->dns); + virBandwidthDefFree(def->bandwidth); + VIR_FREE(def); } @@ -814,6 +816,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) int nIps, nPortGroups, nForwardIfs; char *forwardDev = NULL; xmlNodePtr save = ctxt->node; + xmlNodePtr bandwidthNode = NULL; if (VIR_ALLOC(def) < 0) { virReportOOMError(); @@ -848,6 +851,10 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) /* Parse network domain information */ def->domain = virXPathString("string(./domain[1]/@name)", ctxt); + if ((bandwidthNode = virXPathNode("./bandwidth", ctxt)) != NULL && + (def->bandwidth = virBandwidthDefParseNode(bandwidthNode)) == NULL) + goto error; + /* Parse bridge information */ def->bridge = virXPathString("string(./bridge[1]/@name)", ctxt); stp = virXPathString("string(./bridge[1]/@stp)", ctxt); diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index 92cc03e..6d0845e 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -154,6 +154,7 @@ struct _virNetworkDef { size_t nPortGroups; virPortGroupDefPtr portGroups; + virBandwidthPtr bandwidth; }; typedef struct _virNetworkObj virNetworkObj; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index acf7bb1..832c8a6 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -707,6 +707,8 @@ nlComm; # network.h +virBandwidthDefFree; +virBandwidthDefParseNode; virSocketAddrBroadcast; virSocketAddrBroadcastByPrefix; virSocketAddrIsNetmask; diff --git a/src/util/network.c b/src/util/network.c index 64f5645..2ba6f76 100644 --- a/src/util/network.c +++ b/src/util/network.c @@ -883,3 +883,150 @@ virVirtualPortProfileFormat(virBufferPtr buf, virBufferAsprintf(buf, "%s</virtualport>\n", indent); } + +static int +virBandwidthParseChildDefNode(xmlNodePtr node, virRatePtr rate) +{ + int ret = -1; + char *average = NULL; + char *peak = NULL; + char *burst = NULL; + + if (!node || !rate) { + virSocketError(VIR_ERR_INVALID_ARG, "%s", + _("invalid argument supplied")); + return -1; + } + + average = virXMLPropString(node, "average"); + peak = virXMLPropString(node, "peak"); + burst = virXMLPropString(node, "burst"); + + if (average) { + if (virStrToLong_ull(average, NULL, 10, &rate->average) < 0) { + virSocketError(VIR_ERR_CONFIG_UNSUPPORTED, + _("could not convert %s"), + average); + goto cleanup; + } + } else { + virSocketError(VIR_ERR_XML_DETAIL, "%s", + _("Missing mandatory average attribute")); + goto cleanup; + } + + if (peak && virStrToLong_ull(peak, NULL, 10, &rate->peak) < 0) { + virSocketError(VIR_ERR_CONFIG_UNSUPPORTED, + _("could not convert %s"), + peak); + goto cleanup; + } + + if (burst && virStrToLong_ull(burst, NULL, 10, &rate->burst) < 0) { + virSocketError(VIR_ERR_CONFIG_UNSUPPORTED, + _("could not convert %s"), + burst); + goto cleanup; + } + + ret = 0; + +cleanup: + VIR_FREE(average); + VIR_FREE(peak); + VIR_FREE(burst); + + return ret; +} + +/** + * virBandwidthDefParseNode: + * @node: XML node + * + * Parse bandwidth XML and return pointer to structure + * + * Returns !NULL on success, NULL on error. + */ +virBandwidthPtr +virBandwidthDefParseNode(xmlNodePtr node) +{ + virBandwidthPtr def = NULL; + xmlNodePtr cur = node->children; + xmlNodePtr in = NULL, out = NULL; + + if (VIR_ALLOC(def) < 0) { + virReportOOMError(); + return NULL; + } + + if (!node || !xmlStrEqual(node->name, BAD_CAST "bandwidth")) { + virSocketError(VIR_ERR_INVALID_ARG, "%s", + _("invalid argument supplied")); + goto error; + } + + while (cur) { + if (cur->type == XML_ELEMENT_NODE) { + if (xmlStrEqual(cur->name, BAD_CAST "inbound")) { + if (in) { + virSocketError(VIR_ERR_XML_DETAIL, "%s", + _("Only one child <inbound> " + "element allowed")); + goto error; + } + in = cur; + } else if (xmlStrEqual(cur->name, BAD_CAST "outbound")) { + if (out) { + virSocketError(VIR_ERR_XML_DETAIL, "%s", + _("Only one child <outbound> " + "element allowed")); + goto error; + } + out = cur; + } + /* Silently ignore unknown elements */ + } + cur = cur->next; + } + + if (in) { + if (VIR_ALLOC(def->in) < 0) { + virReportOOMError(); + goto error; + } + + if (virBandwidthParseChildDefNode(in, def->in) < 0) { + /* helper reported error for us */ + goto error; + } + } + + if (out) { + if (VIR_ALLOC(def->out) < 0) { + virReportOOMError(); + goto error; + } + + if (virBandwidthParseChildDefNode(out, def->out) < 0) { + /* helper reported error for us */ + goto error; + } + } + + return def; + +error: + virBandwidthDefFree(def); + return NULL; +} + +void +virBandwidthDefFree(virBandwidthPtr def) +{ + if (!def) + return; + + VIR_FREE(def->in); + VIR_FREE(def->out); + VIR_FREE(def); +} diff --git a/src/util/network.h b/src/util/network.h index 3c090d8..4d35388 100644 --- a/src/util/network.h +++ b/src/util/network.h @@ -150,4 +150,6 @@ virVirtualPortProfileFormat(virBufferPtr buf, virVirtualPortProfileParamsPtr virtPort, const char *indent); +virBandwidthPtr virBandwidthDefParseNode(xmlNodePtr node); +void virBandwidthDefFree(virBandwidthPtr def); #endif /* __VIR_NETWORK_H__ */ -- 1.7.5.rc3

On 07/22/2011 10:07 AM, Michal Privoznik wrote:
These functions parse given XML node and return pointer to the output. Unknown elements are silently ignored. Attributes must be integer and must fit in unsigned long long.
Free function frees elements of virBandwidth structure. --- src/conf/domain_conf.c | 5 ++ src/conf/domain_conf.h | 1 + src/conf/network_conf.c | 7 ++ src/conf/network_conf.h | 1 + src/libvirt_private.syms | 2 + src/util/network.c | 147 ++++++++++++++++++++++++++++++++++++++++++++++ src/util/network.h | 2 + 7 files changed, 165 insertions(+), 0 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 919a75a..e11ad98 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -810,6 +810,8 @@ void virDomainNetDefFree(virDomainNetDefPtr def) VIR_FREE(def->filter); virNWFilterHashTableFree(def->filterparams);
+ virBandwidthDefFree(def->bandwidth); + VIR_FREE(def); }
@@ -2823,6 +2825,9 @@ virDomainNetDefParseXML(virCapsPtr caps, xmlStrEqual(cur->name, BAD_CAST "actual")) { if (virDomainActualNetDefParseXML(cur, ctxt,&actual)< 0) goto error; + } else if (xmlStrEqual(cur->name, BAD_CAST "bandwidth")) { + if (!(def->bandwidth = virBandwidthDefParseNode(cur))) + goto error; } } cur = cur->next; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 551946b..212f0c5 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -425,6 +425,7 @@ struct _virDomainNetDef { virDomainDeviceInfo info; char *filter; virNWFilterHashTablePtr filterparams; + virBandwidthPtr bandwidth; };
enum virDomainChrDeviceType { diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 860eea3..01c094c 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -167,6 +167,8 @@ void virNetworkDefFree(virNetworkDefPtr def)
virNetworkDNSDefFree(def->dns);
+ virBandwidthDefFree(def->bandwidth); + VIR_FREE(def); }
@@ -814,6 +816,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) int nIps, nPortGroups, nForwardIfs; char *forwardDev = NULL; xmlNodePtr save = ctxt->node; + xmlNodePtr bandwidthNode = NULL;
if (VIR_ALLOC(def)< 0) { virReportOOMError(); @@ -848,6 +851,10 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) /* Parse network domain information */ def->domain = virXPathString("string(./domain[1]/@name)", ctxt);
+ if ((bandwidthNode = virXPathNode("./bandwidth", ctxt)) != NULL&& + (def->bandwidth = virBandwidthDefParseNode(bandwidthNode)) == NULL) + goto error;
Heh. Having the domain and network versions of adding bandwidth together in the same patch points out just how inconsistent the style is in various parts of our XML parsing - this one uses virXPathNode, and the domain version uses a loop through all the elements with a bunch of xmlStrEqual comparisons against each element's name. Almost perl-like in its quest to do the same thing in as many different ways as possible :-P (BTW, I still would have preferred to have the patches organized as 1) all levels of change for bandwidth object, 2) all levels of change for domain parse/format changes (RNG, data, parse, format, test cases, docs), 3) all levels of change for network parse/format changes, 4) implement the actual functionality. However, the end result is the same, the patches provided pass make check at each step (so bisecting isn't broken, and the end result is the same, so I'm fine with it going in like this.)
+ /* Parse bridge information */ def->bridge = virXPathString("string(./bridge[1]/@name)", ctxt); stp = virXPathString("string(./bridge[1]/@stp)", ctxt); diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index 92cc03e..6d0845e 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -154,6 +154,7 @@ struct _virNetworkDef {
size_t nPortGroups; virPortGroupDefPtr portGroups; + virBandwidthPtr bandwidth;
Changed into a pointer to the object, so we check for presence of the element by looking for a NULL pointer instead of a special value of one of the attributes.
};
typedef struct _virNetworkObj virNetworkObj; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index acf7bb1..832c8a6 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -707,6 +707,8 @@ nlComm;
# network.h +virBandwidthDefFree; +virBandwidthDefParseNode; virSocketAddrBroadcast; virSocketAddrBroadcastByPrefix; virSocketAddrIsNetmask; diff --git a/src/util/network.c b/src/util/network.c index 64f5645..2ba6f76 100644 --- a/src/util/network.c +++ b/src/util/network.c @@ -883,3 +883,150 @@ virVirtualPortProfileFormat(virBufferPtr buf,
virBufferAsprintf(buf, "%s</virtualport>\n", indent); } + +static int +virBandwidthParseChildDefNode(xmlNodePtr node, virRatePtr rate) +{ + int ret = -1; + char *average = NULL; + char *peak = NULL; + char *burst = NULL; + + if (!node || !rate) { + virSocketError(VIR_ERR_INVALID_ARG, "%s", + _("invalid argument supplied")); + return -1; + } + + average = virXMLPropString(node, "average"); + peak = virXMLPropString(node, "peak"); + burst = virXMLPropString(node, "burst"); + + if (average) { + if (virStrToLong_ull(average, NULL, 10,&rate->average)< 0) { + virSocketError(VIR_ERR_CONFIG_UNSUPPORTED, + _("could not convert %s"), + average); + goto cleanup; + } + } else { + virSocketError(VIR_ERR_XML_DETAIL, "%s", + _("Missing mandatory average attribute")); + goto cleanup; + } + + if (peak&& virStrToLong_ull(peak, NULL, 10,&rate->peak)< 0) { + virSocketError(VIR_ERR_CONFIG_UNSUPPORTED, + _("could not convert %s"), + peak); + goto cleanup; + } + + if (burst&& virStrToLong_ull(burst, NULL, 10,&rate->burst)< 0) { + virSocketError(VIR_ERR_CONFIG_UNSUPPORTED, + _("could not convert %s"), + burst); + goto cleanup; + } + + ret = 0; + +cleanup: + VIR_FREE(average); + VIR_FREE(peak); + VIR_FREE(burst); + + return ret; +} + +/** + * virBandwidthDefParseNode: + * @node: XML node + * + * Parse bandwidth XML and return pointer to structure + * + * Returns !NULL on success, NULL on error. + */ +virBandwidthPtr +virBandwidthDefParseNode(xmlNodePtr node) +{ + virBandwidthPtr def = NULL; + xmlNodePtr cur = node->children; + xmlNodePtr in = NULL, out = NULL; + + if (VIR_ALLOC(def)< 0) { + virReportOOMError(); + return NULL; + } + + if (!node || !xmlStrEqual(node->name, BAD_CAST "bandwidth")) { + virSocketError(VIR_ERR_INVALID_ARG, "%s", + _("invalid argument supplied")); + goto error; + } + + while (cur) { + if (cur->type == XML_ELEMENT_NODE) { + if (xmlStrEqual(cur->name, BAD_CAST "inbound")) { + if (in) { + virSocketError(VIR_ERR_XML_DETAIL, "%s", + _("Only one child<inbound> " + "element allowed")); + goto error; + } + in = cur; + } else if (xmlStrEqual(cur->name, BAD_CAST "outbound")) { + if (out) { + virSocketError(VIR_ERR_XML_DETAIL, "%s", + _("Only one child<outbound> " + "element allowed")); + goto error; + } + out = cur; + } + /* Silently ignore unknown elements */ + } + cur = cur->next; + } + + if (in) { + if (VIR_ALLOC(def->in)< 0) { + virReportOOMError(); + goto error; + } + + if (virBandwidthParseChildDefNode(in, def->in)< 0) { + /* helper reported error for us */ + goto error; + } + } + + if (out) { + if (VIR_ALLOC(def->out)< 0) { + virReportOOMError(); + goto error; + } + + if (virBandwidthParseChildDefNode(out, def->out)< 0) { + /* helper reported error for us */ + goto error; + } + } + + return def; + +error: + virBandwidthDefFree(def); + return NULL; +} + +void +virBandwidthDefFree(virBandwidthPtr def) +{ + if (!def) + return; + + VIR_FREE(def->in); + VIR_FREE(def->out); + VIR_FREE(def); +}
virBandwidthFree() needs to be added to the list of "VIR_FREE()-like" functions in cfg.mk (be sure to keep the list alphabetical!) ACK with that one nit fixed.
diff --git a/src/util/network.h b/src/util/network.h index 3c090d8..4d35388 100644 --- a/src/util/network.h +++ b/src/util/network.h @@ -150,4 +150,6 @@ virVirtualPortProfileFormat(virBufferPtr buf, virVirtualPortProfileParamsPtr virtPort, const char *indent);
+virBandwidthPtr virBandwidthDefParseNode(xmlNodePtr node); +void virBandwidthDefFree(virBandwidthPtr def); #endif /* __VIR_NETWORK_H__ */

--- src/conf/domain_conf.c | 3 ++ src/conf/network_conf.c | 3 ++ src/libvirt_private.syms | 1 + src/util/network.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++ src/util/network.h | 3 ++ 5 files changed, 82 insertions(+), 0 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e11ad98..072c970 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8855,6 +8855,9 @@ virDomainNetDefFormat(virBufferPtr buf, virBufferAddLit(buf, " </tune>\n"); } + if (virBandwidthDefFormat(buf, def->bandwidth, " ") < 0) + return -1; + if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) return -1; diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 01c094c..1ef80dc 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -1329,6 +1329,9 @@ char *virNetworkDefFormat(const virNetworkDefPtr def) if (virNetworkDNSDefFormat(&buf, def->dns) < 0) goto error; + if (virBandwidthDefFormat(&buf, def->bandwidth, " ") < 0) + goto error; + for (ii = 0; ii < def->nips; ii++) { if (virNetworkIpDefFormat(&buf, &def->ips[ii]) < 0) goto error; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 832c8a6..188d647 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -707,6 +707,7 @@ nlComm; # network.h +virBandwidthDefFormat; virBandwidthDefFree; virBandwidthDefParseNode; virSocketAddrBroadcast; diff --git a/src/util/network.c b/src/util/network.c index 2ba6f76..5639219 100644 --- a/src/util/network.c +++ b/src/util/network.c @@ -1030,3 +1030,75 @@ virBandwidthDefFree(virBandwidthPtr def) VIR_FREE(def->out); VIR_FREE(def); } + +static int +virBandwidthChildDefFormat(virBufferPtr buf, + virRatePtr def, + const char *elem_name) +{ + if (!buf || !def || !elem_name) + return -1; + + if (def->average) { + virBufferAsprintf(buf, "<%s average='%llu'", elem_name, def->average); + + if (def->peak) + virBufferAsprintf(buf, " peak='%llu'", def->peak); + + if (def->burst) + virBufferAsprintf(buf, " burst='%llu'", def->burst); + virBufferAddLit(buf, "/>\n"); + } + + return 0; +} + +/** + * virBandwidthDefFormat: + * @buf: Buffer to print to + * @def: Data source + * @indent: prepend all lines printed with this + * + * Formats bandwidth and prepend each line with @indent. + * Passing NULL to @indent is equivalent passing "". + * + * Returns 0 on success, else -1. + */ +int +virBandwidthDefFormat(virBufferPtr buf, + virBandwidthPtr def, + const char *indent) +{ + int ret = -1; + + if (!buf) + goto cleanup; + + if (!def) { + ret = 0; + goto cleanup; + } + + if (!indent) + indent = ""; + + virBufferAsprintf(buf, "%s<bandwidth>\n", indent); + if (def->in) { + virBufferAsprintf(buf, "%s ", indent); + if (virBandwidthChildDefFormat(buf, def->in, "inbound") < 0) + goto cleanup; + } + + if (def->out) { + virBufferAsprintf(buf, "%s ", indent); + if (virBandwidthChildDefFormat(buf, def->out, "outbound") < 0) + goto cleanup; + } + + virBufferAsprintf(buf, "%s</bandwidth>\n", indent); + + ret = 0; + +cleanup: + return ret; +} diff --git a/src/util/network.h b/src/util/network.h index 4d35388..d0181fd 100644 --- a/src/util/network.h +++ b/src/util/network.h @@ -152,4 +152,7 @@ virVirtualPortProfileFormat(virBufferPtr buf, virBandwidthPtr virBandwidthDefParseNode(xmlNodePtr node); void virBandwidthDefFree(virBandwidthPtr def); +int virBandwidthDefFormat(virBufferPtr buf, + virBandwidthPtr def, + const char *indent); #endif /* __VIR_NETWORK_H__ */ -- 1.7.5.rc3

On 07/22/2011 10:07 AM, Michal Privoznik wrote:
--- src/conf/domain_conf.c | 3 ++ src/conf/network_conf.c | 3 ++ src/libvirt_private.syms | 1 + src/util/network.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++ src/util/network.h | 3 ++ 5 files changed, 82 insertions(+), 0 deletions(-)
ACK.

These function executes 'tc' with appropriate arguments to set desired QoS setting on interface or bridge during its creation. --- configure.ac | 4 + libvirt.spec.in | 2 +- src/libvirt_private.syms | 2 + src/network/bridge_driver.c | 18 +++++ src/qemu/qemu_command.c | 11 +++- src/util/macvtap.c | 12 +++- src/util/macvtap.h | 3 +- src/util/network.c | 161 +++++++++++++++++++++++++++++++++++++++++++ src/util/network.h | 3 + 9 files changed, 212 insertions(+), 4 deletions(-) diff --git a/configure.ac b/configure.ac index 9e39f44..72fbc41 100644 --- a/configure.ac +++ b/configure.ac @@ -165,6 +165,8 @@ AC_PATH_PROG([RADVD], [radvd], [radvd], [/sbin:/usr/sbin:/usr/local/sbin:$PATH]) AC_PATH_PROG([BRCTL], [brctl], [brctl], [/sbin:/usr/sbin:/usr/local/sbin:$PATH]) +AC_PATH_PROG([TC], [tc], [tc], + [/sbin:/usr/sbin:/usr/local/sbin:$PATH]) AC_PATH_PROG([UDEVADM], [udevadm], [], [/sbin:/usr/sbin:/usr/local/sbin:$PATH]) AC_PATH_PROG([UDEVSETTLE], [udevsettle], [], @@ -178,6 +180,8 @@ AC_DEFINE_UNQUOTED([RADVD],["$RADVD"], [Location or name of the radvd program]) AC_DEFINE_UNQUOTED([BRCTL],["$BRCTL"], [Location or name of the brctl program (see bridge-utils)]) +AC_DEFINE_UNQUOTED([TC],["$TC"], + [Location or name of the tc profram (see iproute2)]) if test -n "$UDEVADM"; then AC_DEFINE_UNQUOTED([UDEVADM],["$UDEVADM"], [Location or name of the udevadm program]) diff --git a/libvirt.spec.in b/libvirt.spec.in index 6cbd9ac..4f0691d 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -250,7 +250,7 @@ Requires: %{name}-client = %{version}-%{release} Requires: bridge-utils # for modprobe of pci devices Requires: module-init-tools -# for /sbin/ip +# for /sbin/ip & /sbin/tc Requires: iproute %endif %if %{with_network} diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 188d647..85100dd 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -710,6 +710,8 @@ nlComm; virBandwidthDefFormat; virBandwidthDefFree; virBandwidthDefParseNode; +virBandwidthDisable; +virBandwidthEnable; virSocketAddrBroadcast; virSocketAddrBroadcastByPrefix; virSocketAddrIsNetmask; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 99033a2..37fb6cf 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -1821,10 +1821,23 @@ networkStartNetworkVirtual(struct network_driver *driver, if (v6present && networkStartRadvd(network) < 0) goto err4; + if (virBandwidthEnable(network->def->bandwidth, network->def->bridge) < 0) { + networkReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot set bandwidth limits on %s"), + network->def->bridge); + goto err5; + } + VIR_FREE(macTapIfName); return 0; + err5: + if (virBandwidthDisable(network->def->bridge, true) < 0) { + VIR_WARN("Failed to disable QoS on %s", + network->def->bridge); + } + err4: if (!save_err) save_err = virSaveLastError(); @@ -1882,6 +1895,11 @@ static int networkShutdownNetworkVirtual(struct network_driver *driver, int err; char ebuf[1024]; + if (virBandwidthDisable(network->def->bridge, true) < 0) { + VIR_WARN("Failed to disable QoS on %s", + network->def->name); + } + if (network->radvdPid > 0) { char *radvdpidbase; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index e785f00..f8fd4ee 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -132,7 +132,7 @@ qemuPhysIfaceConnect(virDomainDefPtr def, vnet_hdr, def->uuid, virDomainNetGetActualDirectVirtPortProfile(net), &res_ifname, - vmop, driver->stateDir); + vmop, driver->stateDir, net->bandwidth); if (rc >= 0) { virDomainAuditNetDevice(def, net, res_ifname, true); VIR_FREE(net->ifname); @@ -298,6 +298,15 @@ qemuNetworkIfaceConnect(virDomainDefPtr def, } } + if (tapfd >= 0 && + virBandwidthEnable(net->bandwidth, net->ifname) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot set bandwidth limits on %s"), + net->ifname); + VIR_FORCE_CLOSE(tapfd); + goto cleanup; + } + if (tapfd >= 0) { if ((net->filter) && (net->ifname)) { err = virDomainConfNWFilterInstantiate(conn, net); diff --git a/src/util/macvtap.c b/src/util/macvtap.c index 7b97c6a..9f7d320 100644 --- a/src/util/macvtap.c +++ b/src/util/macvtap.c @@ -267,7 +267,8 @@ openMacvtapTap(const char *tgifname, virVirtualPortProfileParamsPtr virtPortProfile, char **res_ifname, enum virVMOperationType vmOp, - char *stateDir) + char *stateDir, + virBandwidthPtr bandwidth) { const char *type = "macvtap"; int c, rc; @@ -361,6 +362,15 @@ create_name: } else goto disassociate_exit; + if (virBandwidthEnable(bandwidth, cr_ifname) < 0) { + macvtapError(VIR_ERR_INTERNAL_ERROR, + _("cannot set bandwidth limits on %s"), + cr_ifname); + rc = -1; + goto disassociate_exit; + } + + return rc; disassociate_exit: diff --git a/src/util/macvtap.h b/src/util/macvtap.h index 8e8613d..2b2d835 100644 --- a/src/util/macvtap.h +++ b/src/util/macvtap.h @@ -62,7 +62,8 @@ int openMacvtapTap(const char *ifname, virVirtualPortProfileParamsPtr virtPortProfile, char **res_ifname, enum virVMOperationType vmop, - char *stateDir); + char *stateDir, + virBandwidthPtr bandwidth); void delMacvtap(const char *ifname, const unsigned char *macaddress, diff --git a/src/util/network.c b/src/util/network.c index 5639219..5561012 100644 --- a/src/util/network.c +++ b/src/util/network.c @@ -16,6 +16,7 @@ #include "network.h" #include "util.h" #include "virterror_internal.h" +#include "command.h" #define VIR_FROM_THIS VIR_FROM_NONE #define virSocketError(code, ...) \ @@ -1102,3 +1103,163 @@ virBandwidthDefFormat(virBufferPtr buf, cleanup: return ret; } + +/** + * virBandwidthEnable: + * @bandwidth: rates to set + * @iface: on which interface + * + * This function enables QoS on specified interface + * and set given traffic limits for both, incoming + * and outgoing traffic. Any previous setting get + * overwritten. + * + * Return 0 on success, -1 otherwise. + */ +int +virBandwidthEnable(virBandwidthPtr bandwidth, + const char *iface) +{ + int ret = -1; + virCommandPtr cmd = NULL; + char *average = NULL; + char *peak = NULL; + char *burst = NULL; + + if (!iface) + return -1; + + if (!bandwidth) { + /* nothing to be enabled */ + ret = 0; + goto cleanup; + } + + if (virBandwidthDisable(iface, true) < 0) + goto cleanup; + + if (bandwidth->in) { + if (virAsprintf(&average, "%llukbps", bandwidth->in->average) < 0) + goto cleanup; + if (bandwidth->in->peak && + (virAsprintf(&peak, "%llukbps", bandwidth->in->peak) < 0)) + goto cleanup; + if (bandwidth->in->burst && + (virAsprintf(&burst, "%llukb", bandwidth->in->burst) < 0)) + goto cleanup; + + cmd = virCommandNew(TC); + virCommandAddArgList(cmd, "qdisc", "add", "dev", iface, "root", + "handle", "1:", "htb", "default", "1", NULL); + if (virCommandRun(cmd, NULL) < 0) + goto cleanup; + + virCommandFree(cmd); + cmd = virCommandNew(TC); + virCommandAddArgList(cmd,"class", "add", "dev", iface, "parent", + "1:", "classid", "1:1", "htb", NULL); + virCommandAddArgList(cmd, "rate", average, NULL); + + if (peak) + virCommandAddArgList(cmd, "ceil", peak, NULL); + if (burst) + virCommandAddArgList(cmd, "burst", burst, NULL); + + if (virCommandRun(cmd, NULL) < 0) + goto cleanup; + + virCommandFree(cmd); + cmd = virCommandNew(TC); + virCommandAddArgList(cmd,"filter", "add", "dev", iface, "parent", + "1:0", "protocol", "ip", "handle", "1", "fw", + "flowid", "1", NULL); + + if (virCommandRun(cmd, NULL) < 0) + goto cleanup; + + VIR_FREE(average); + VIR_FREE(peak); + VIR_FREE(burst); + } + + if (bandwidth->out) { + if (virAsprintf(&average, "%llukbps", bandwidth->out->average) < 0) + goto cleanup; + if (virAsprintf(&burst, "%llukb", bandwidth->out->burst ? + bandwidth->out->burst : bandwidth->out->average) < 0) + goto cleanup; + + virCommandFree(cmd); + cmd = virCommandNew(TC); + virCommandAddArgList(cmd, "qdisc", "add", "dev", iface, + "ingress", NULL); + + if (virCommandRun(cmd, NULL) < 0) + goto cleanup; + + virCommandFree(cmd); + cmd = virCommandNew(TC); + virCommandAddArgList(cmd, "filter", "add", "dev", iface, "parent", + "ffff:", "protocol", "ip", "u32", "match", "ip", + "src", "0.0.0.0/0", "police", "rate", average, + "burst", burst, "mtu", burst, "drop", "flowid", + ":1", NULL); + + if (virCommandRun(cmd, NULL) < 0) + goto cleanup; + } + + ret = 0; + +cleanup: + virCommandFree(cmd); + VIR_FREE(average); + VIR_FREE(peak); + VIR_FREE(burst); + return ret; +} + +/** + * virBandwidthDisable: + * @iface: on which interface + * @may_fail: should be unsuccessful disable considered fatal? + * + * This function tries to disable QoS on specified interface + * by deleting root and ingress qdisc. However, this may fail + * if we try to remove the default one. + * + * Return 0 on success, -1 otherwise. + */ +int +virBandwidthDisable(const char *iface, + bool may_fail) +{ + int ret = -1; + int status; + virCommandPtr cmd = NULL; + + if (!iface) + return -1; + + cmd = virCommandNew(TC); + virCommandAddArgList(cmd, "qdisc", "del", "dev", iface, "root", NULL); + + if ((virCommandRun(cmd, &status) < 0) || + (!may_fail && status)) + goto cleanup; + + virCommandFree(cmd); + + cmd = virCommandNew(TC); + virCommandAddArgList(cmd, "qdisc", "del", "dev", iface, "ingress", NULL); + + if ((virCommandRun(cmd, &status) < 0) || + (!may_fail && status)) + goto cleanup; + + ret = 0; + +cleanup: + virCommandFree(cmd); + return ret; +} diff --git a/src/util/network.h b/src/util/network.h index d0181fd..139f6cc 100644 --- a/src/util/network.h +++ b/src/util/network.h @@ -155,4 +155,7 @@ void virBandwidthDefFree(virBandwidthPtr def); int virBandwidthDefFormat(virBufferPtr buf, virBandwidthPtr def, const char *indent); + +int virBandwidthEnable(virBandwidthPtr bandwidth, const char *iface); +int virBandwidthDisable(const char *iface, bool may_fail); #endif /* __VIR_NETWORK_H__ */ -- 1.7.5.rc3

On 07/22/2011 10:07 AM, Michal Privoznik wrote:
These function executes 'tc' with appropriate arguments to set desired QoS setting on interface or bridge during its creation. --- configure.ac | 4 + libvirt.spec.in | 2 +- src/libvirt_private.syms | 2 + src/network/bridge_driver.c | 18 +++++ src/qemu/qemu_command.c | 11 +++- src/util/macvtap.c | 12 +++- src/util/macvtap.h | 3 +- src/util/network.c | 161 +++++++++++++++++++++++++++++++++++++++++++ src/util/network.h | 3 + 9 files changed, 212 insertions(+), 4 deletions(-)
ACK.

--- tests/networkxml2xmlin/bandwidth-network.xml | 16 ++++++++++++++++ tests/networkxml2xmlout/bandwidth-network.xml | 16 ++++++++++++++++ tests/networkxml2xmltest.c | 1 + 3 files changed, 33 insertions(+), 0 deletions(-) create mode 100644 tests/networkxml2xmlin/bandwidth-network.xml create mode 100644 tests/networkxml2xmlout/bandwidth-network.xml diff --git a/tests/networkxml2xmlin/bandwidth-network.xml b/tests/networkxml2xmlin/bandwidth-network.xml new file mode 100644 index 0000000..555ee18 --- /dev/null +++ b/tests/networkxml2xmlin/bandwidth-network.xml @@ -0,0 +1,16 @@ +<network> + <name>test-net</name> + <uuid>986fed9e-a488-186d-ef2d-17ebfd1993f8</uuid> + <forward mode='nat'/> + <bridge name='virbr1' stp='on' delay='0' /> + <mac address='52:54:00:E6:A2:C9'/> + <bandwidth> + <inbound average='1000' peak='2000' burst='1024'/> + <outbound average='2000'/> + </bandwidth> + <ip address='192.168.120.1' netmask='255.255.255.0'> + <dhcp> + <range start='192.168.120.2' end='192.168.120.254' /> + </dhcp> + </ip> +</network> diff --git a/tests/networkxml2xmlout/bandwidth-network.xml b/tests/networkxml2xmlout/bandwidth-network.xml new file mode 100644 index 0000000..555ee18 --- /dev/null +++ b/tests/networkxml2xmlout/bandwidth-network.xml @@ -0,0 +1,16 @@ +<network> + <name>test-net</name> + <uuid>986fed9e-a488-186d-ef2d-17ebfd1993f8</uuid> + <forward mode='nat'/> + <bridge name='virbr1' stp='on' delay='0' /> + <mac address='52:54:00:E6:A2:C9'/> + <bandwidth> + <inbound average='1000' peak='2000' burst='1024'/> + <outbound average='2000'/> + </bandwidth> + <ip address='192.168.120.1' netmask='255.255.255.0'> + <dhcp> + <range start='192.168.120.2' end='192.168.120.254' /> + </dhcp> + </ip> +</network> diff --git a/tests/networkxml2xmltest.c b/tests/networkxml2xmltest.c index f354b0d..5cdbedb 100644 --- a/tests/networkxml2xmltest.c +++ b/tests/networkxml2xmltest.c @@ -92,6 +92,7 @@ mymain(void) DO_TEST("direct-net"); DO_TEST("host-bridge-net"); DO_TEST("vepa-net"); + DO_TEST("bandwidth-network"); return (ret==0 ? EXIT_SUCCESS : EXIT_FAILURE); } -- 1.7.5.rc3

On 07/22/2011 10:07 AM, Michal Privoznik wrote:
--- tests/networkxml2xmlin/bandwidth-network.xml | 16 ++++++++++++++++ tests/networkxml2xmlout/bandwidth-network.xml | 16 ++++++++++++++++ tests/networkxml2xmltest.c | 1 + 3 files changed, 33 insertions(+), 0 deletions(-) create mode 100644 tests/networkxml2xmlin/bandwidth-network.xml create mode 100644 tests/networkxml2xmlout/bandwidth-network.xml
ACK.

--- tests/domainschemadata/domain-bandwidth.xml | 72 +++++++++++++++++++++++++++ 1 files changed, 72 insertions(+), 0 deletions(-) create mode 100644 tests/domainschemadata/domain-bandwidth.xml diff --git a/tests/domainschemadata/domain-bandwidth.xml b/tests/domainschemadata/domain-bandwidth.xml new file mode 100644 index 0000000..852c97b --- /dev/null +++ b/tests/domainschemadata/domain-bandwidth.xml @@ -0,0 +1,72 @@ +<domain type='kvm'> + <name>f14-60</name> + <uuid>38644c45-5227-a936-3b38-bc4f72c355bb</uuid> + <memory>1048576</memory> + <currentMemory>1048576</currentMemory> + <vcpu>2</vcpu> + <os> + <type arch='x86_64' machine='pc-0.13'>hvm</type> + <boot dev='cdrom'/> + <boot dev='hd'/> + <bootmenu enable='yes'/> + </os> + <features> + <acpi/> + <apic/> + <pae/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/bin/qemu-kvm</emulator> + <disk type='file' device='disk'> + <driver name='qemu' type='qcow2'/> + <source file='/var/lib/libvirt/images/f14-6.img'/> + <target dev='vda' bus='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </disk> + <disk type='file' device='cdrom'> + <driver name='qemu' type='raw'/> + <source file='/home/zippy/tmp/Fedora-14-x86_64-Live-KDE.iso'/> + <target dev='hdc' bus='ide'/> + <readonly/> + <address type='drive' controller='0' bus='1' unit='0'/> + </disk> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='virtio-serial' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> + </controller> + <interface type='network'> + <mac address='52:54:00:24:a5:9f'/> + <source network='default'/> + <bandwidth> + <inbound average='1000' peak='4000' burst='1024'/> + <outbound average='128' peak='256' burst='32768'/> + </bandwidth> + <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> + </interface> + <serial type='pty'> + <target port='0'/> + </serial> + <console type='pty'> + <target type='serial' port='0'/> + </console> + <input type='tablet' bus='usb'/> + <input type='mouse' bus='ps2'/> + <graphics type='vnc' port='-1' autoport='yes'/> + <sound model='ac97'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </sound> + <video> + <model type='vga' vram='9216' heads='1'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </video> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> + </memballoon> + </devices> +</domain> -- 1.7.5.rc3

On 07/22/2011 10:07 AM, Michal Privoznik wrote:
--- tests/domainschemadata/domain-bandwidth.xml | 72 +++++++++++++++++++++++++++ 1 files changed, 72 insertions(+), 0 deletions(-) create mode 100644 tests/domainschemadata/domain-bandwidth.xml
Instead of putting this file in the domainschemadata directory, you should put it in the qemuxml2argvdata directory (named as, e.g. "qemuxml2argv-net-bandwidth.xml", then add the line: DO_TEST("net-bandwidth"); to qemuxml2xmltest.c. Since all files in qemuxml2argv are run through domainschematest, you'll still get the test coverage you've got now, but will have the added advantage that it goes through the parse->format->compare-to-original (xml2xml) test as well. (it won't try to do the argv test (which we don't want in this case) unless you also add it to the list in qemuxml2argvtest.c) ACK if you move the file and add it to the xml2xml tests in qemuxml2xmltest.c.
diff --git a/tests/domainschemadata/domain-bandwidth.xml b/tests/domainschemadata/domain-bandwidth.xml new file mode 100644 index 0000000..852c97b --- /dev/null +++ b/tests/domainschemadata/domain-bandwidth.xml @@ -0,0 +1,72 @@ +<domain type='kvm'> +<name>f14-60</name> +<uuid>38644c45-5227-a936-3b38-bc4f72c355bb</uuid> +<memory>1048576</memory> +<currentMemory>1048576</currentMemory> +<vcpu>2</vcpu> +<os> +<type arch='x86_64' machine='pc-0.13'>hvm</type> +<boot dev='cdrom'/> +<boot dev='hd'/> +<bootmenu enable='yes'/> +</os> +<features> +<acpi/> +<apic/> +<pae/> +</features> +<clock offset='utc'/> +<on_poweroff>destroy</on_poweroff> +<on_reboot>restart</on_reboot> +<on_crash>restart</on_crash> +<devices> +<emulator>/usr/bin/qemu-kvm</emulator> +<disk type='file' device='disk'> +<driver name='qemu' type='qcow2'/> +<source file='/var/lib/libvirt/images/f14-6.img'/> +<target dev='vda' bus='virtio'/> +<address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> +</disk> +<disk type='file' device='cdrom'> +<driver name='qemu' type='raw'/> +<source file='/home/zippy/tmp/Fedora-14-x86_64-Live-KDE.iso'/> +<target dev='hdc' bus='ide'/> +<readonly/> +<address type='drive' controller='0' bus='1' unit='0'/> +</disk> +<controller type='ide' index='0'> +<address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> +</controller> +<controller type='virtio-serial' index='0'> +<address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> +</controller> +<interface type='network'> +<mac address='52:54:00:24:a5:9f'/> +<source network='default'/> +<bandwidth> +<inbound average='1000' peak='4000' burst='1024'/> +<outbound average='128' peak='256' burst='32768'/> +</bandwidth> +<address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> +</interface> +<serial type='pty'> +<target port='0'/> +</serial> +<console type='pty'> +<target type='serial' port='0'/> +</console> +<input type='tablet' bus='usb'/> +<input type='mouse' bus='ps2'/> +<graphics type='vnc' port='-1' autoport='yes'/> +<sound model='ac97'> +<address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> +</sound> +<video> +<model type='vga' vram='9216' heads='1'/> +<address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> +</video> +<memballoon model='virtio'> +<address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> +</memballoon> +</devices> +</domain>

On 22.07.2011 16:07, Michal Privoznik wrote:
This patch series add support for setting traffic shaping and policing on both domain's interface and network's virtual bridge. Basically, this is done via 'tc' from iproute2 package. For shaping is HTB used, for policing we need u32 match selector. Both should be available in RHEL-6 kernel.
How this works: On an virtual interface which has limits defined a root qdisc are replaced. Ingress root for outbound traffic shaping and egress for inbound. Basically, in inbound traffic policing is applied, on outbound shaping. New qdiscs are set to limit the traffic to rate set in XML. For shaping it is possible to set the size of buffer. Accepted values for rate, peak and burst are integer numbers. Units are kilobytes per second for rate or kilobytes for size.
Supported devices are VIR_DOMAIN_NET_TYPE_NETWORK, VIR_DOMAIN_NET_TYPE_BRIDGE and VIR_DOMAIN_NET_TYPE_DIRECT.
diff to v3: -Laine's review included
diff to v2: -Jirka's review included
diff to v1: -rebase to current HEAD -add support for macvtap devices
Michal Privoznik (7): bandwidth: Define schema and create documentation bandwidth: Declare internal structures bandwidth: Add parsing and free functions bandwidth: Create format functions bandwidth: Implement functions to enable and disable QoS bandwidth: Add test cases for network bandwidth: Add domain schema test suite
configure.ac | 4 + docs/formatdomain.html.in | 34 +++ docs/formatnetwork.html.in | 30 ++ docs/schemas/domain.rng | 3 + docs/schemas/network.rng | 3 + docs/schemas/networkcommon.rng | 48 +++ libvirt.spec.in | 2 +- src/conf/domain_conf.c | 8 + src/conf/domain_conf.h | 1 + src/conf/network_conf.c | 10 + src/conf/network_conf.h | 1 + src/libvirt_private.syms | 5 + src/network/bridge_driver.c | 18 ++ src/qemu/qemu_command.c | 11 +- src/util/macvtap.c | 12 +- src/util/macvtap.h | 3 +- src/util/network.c | 380 +++++++++++++++++++++++++ src/util/network.h | 22 ++ tests/domainschemadata/domain-bandwidth.xml | 72 +++++ tests/networkxml2xmlin/bandwidth-network.xml | 16 + tests/networkxml2xmlout/bandwidth-network.xml | 16 + tests/networkxml2xmltest.c | 1 + 22 files changed, 696 insertions(+), 4 deletions(-) create mode 100644 tests/domainschemadata/domain-bandwidth.xml create mode 100644 tests/networkxml2xmlin/bandwidth-network.xml create mode 100644 tests/networkxml2xmlout/bandwidth-network.xml
Thanks for review & push. Michal

On 07/25/2011 06:29 AM, Michal Privoznik wrote:
On 22.07.2011 16:07, Michal Privoznik wrote:
This patch series add support for setting traffic shaping and policing on both domain's interface and network's virtual bridge. Basically, this is done via 'tc' from iproute2 package. For shaping is HTB used, for policing we need u32 match selector. Both should be available in RHEL-6 kernel.
How this works: On an virtual interface which has limits defined a root qdisc are replaced. Ingress root for outbound traffic shaping and egress for inbound. Basically, in inbound traffic policing is applied, on outbound shaping. New qdiscs are set to limit the traffic to rate set in XML. For shaping it is possible to set the size of buffer. Accepted values for rate, peak and burst are integer numbers. Units are kilobytes per second for rate or kilobytes for size.
Supported devices are VIR_DOMAIN_NET_TYPE_NETWORK, VIR_DOMAIN_NET_TYPE_BRIDGE and VIR_DOMAIN_NET_TYPE_DIRECT.
diff to v3: -Laine's review included
diff to v2: -Jirka's review included
diff to v1: -rebase to current HEAD -add support for macvtap devices
Michal Privoznik (7): bandwidth: Define schema and create documentation bandwidth: Declare internal structures bandwidth: Add parsing and free functions bandwidth: Create format functions bandwidth: Implement functions to enable and disable QoS bandwidth: Add test cases for network bandwidth: Add domain schema test suite
configure.ac | 4 + docs/formatdomain.html.in | 34 +++ docs/formatnetwork.html.in | 30 ++ docs/schemas/domain.rng | 3 + docs/schemas/network.rng | 3 + docs/schemas/networkcommon.rng | 48 +++ libvirt.spec.in | 2 +- src/conf/domain_conf.c | 8 + src/conf/domain_conf.h | 1 + src/conf/network_conf.c | 10 + src/conf/network_conf.h | 1 + src/libvirt_private.syms | 5 + src/network/bridge_driver.c | 18 ++ src/qemu/qemu_command.c | 11 +- src/util/macvtap.c | 12 +- src/util/macvtap.h | 3 +- src/util/network.c | 380 +++++++++++++++++++++++++ src/util/network.h | 22 ++ tests/domainschemadata/domain-bandwidth.xml | 72 +++++ tests/networkxml2xmlin/bandwidth-network.xml | 16 + tests/networkxml2xmlout/bandwidth-network.xml | 16 + tests/networkxml2xmltest.c | 1 + 22 files changed, 696 insertions(+), 4 deletions(-) create mode 100644 tests/domainschemadata/domain-bandwidth.xml create mode 100644 tests/networkxml2xmlin/bandwidth-network.xml create mode 100644 tests/networkxml2xmlout/bandwidth-network.xml
Thanks for review & push.
Ah, right. I was distracted by the fact that the push went to Eric's mirror repo and forgot to send mail saying that I'd pushed the series.
participants (2)
-
Laine Stump
-
Michal Privoznik