[libvirt] [PATCH v1 01/11] bandwidth: add new 'floor' attribute

[Michal Privoznik]

On 30.11.2012 13:14, Laine Stump wrote:
> On 11/19/2012 11:51 AM, Michal Privoznik wrote:
>> This is however supported only on domain interfaces with
>> type='network'. Moreover, target network needs to have at least
>> inbound QoS set. This is required by hierarchical traffic shaping.
>>
>> >From now on, the required attribute for <inbound/> is either 'average'
>> (old) or 'floor' (new). This new attribute can be used just for
>> interfaces type of network (<interface type='network'/>) currently.
>> ---
>>  docs/formatdomain.html.in        |   20 ++++++++++++--
>>  docs/schemas/networkcommon.rng   |    5 +++
>>  src/conf/domain_conf.c           |    6 +++-
>>  src/conf/netdev_bandwidth_conf.c |   54 +++++++++++++++++++++++++++++++++-----
>>  src/conf/netdev_bandwidth_conf.h |    3 +-
>>  src/conf/network_conf.c          |    4 +-
>>  src/util/virnetdevbandwidth.h    |    1 +
>>  7 files changed, 78 insertions(+), 15 deletions(-)
>>

>> diff --git a/src/conf/netdev_bandwidth_conf.c b/src/conf/netdev_bandwidth_conf.c
>> index 261718f..38c5a5b 100644
>> --- a/src/conf/netdev_bandwidth_conf.c
>> +++ b/src/conf/netdev_bandwidth_conf.c
>> @@ -26,6 +26,7 @@
>>  #include "virterror_internal.h"
>>  #include "util.h"
>>  #include "memory.h"
>> +#include "domain_conf.h"
>>  
>>  #define VIR_FROM_THIS VIR_FROM_NONE
>>  
>> @@ -36,6 +37,7 @@ virNetDevBandwidthParseRate(xmlNodePtr node, virNetDevBandwidthRatePtr rate)
>>      char *average = NULL;
>>      char *peak = NULL;
>>      char *burst = NULL;
>> +    char *floor = NULL;
>>  
>>      if (!node || !rate) {
>>          virReportError(VIR_ERR_INVALID_ARG, "%s",
>> @@ -46,6 +48,7 @@ virNetDevBandwidthParseRate(xmlNodePtr node, virNetDevBandwidthRatePtr rate)
>>      average = virXMLPropString(node, "average");
>>      peak = virXMLPropString(node, "peak");
>>      burst = virXMLPropString(node, "burst");
>> +    floor = virXMLPropString(node, "floor");
>>  
>>      if (average) {
>>          if (virStrToLong_ull(average, NULL, 10, &rate->average) < 0) {
>> @@ -54,9 +57,15 @@ virNetDevBandwidthParseRate(xmlNodePtr node, virNetDevBandwidthRatePtr rate)
>>                             average);
>>              goto cleanup;
>>          }
>> -    } else {
>> +    } else if (!floor) {
>>          virReportError(VIR_ERR_XML_DETAIL, "%s",
>> -                       _("Missing mandatory average attribute"));
>> +                       _("Missing mandatory average or floor attributes"));
>> +        goto cleanup;
>> +    }
>> +
>> +    if ((peak || burst) && !average) {
>> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>> +                       _("'peak' and 'burst' require 'average' attribute"));
>>          goto cleanup;
>>      }
> 
> I didn't understand if floor and average were mutually exclusive. If
> that's the case, then you need to check for it here. Otherwise, I just
> need to understand better :-)

They are not mutually exclusive. The interface (of correct type as
you've pointed out above) can has both 'floor' and 'average'. The first
is applied on bridge the second directly on the interface.
I admit that this QoS magic is kind of black box. But if you ever saw a
picture of flow of a packet within linux kernel, where are which
{ip,eb}tables rules applied, then you know why is that. See [1] for
instance.

> 
> 
> ACK with the minor things fixed (the bit about having a different log
> message for network vs. interface is optional; or maybe you can think of
> a single message that works better for both cases).
> 
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
> 


1: http://blog.propter.net/wp-content/uploads/2012/02/PacketFlow.png