[libvirt] [PATCH] bandwidth: Require network QoS if interface uses 'floor'

[Michal Privoznik]

On 11.03.2013 04:53, Laine Stump wrote:
> On 03/07/2013 05:02 AM, Michal Privoznik wrote:
>> By current implementation, network inbound is required in order
>> to use 'floor' for guaranteeing  minimal throughput. This is so,
>> because we want user to tell us the maximal throughput of the
>> network instead of finding out ourselves (and detect bogus values
>> in case of virtual interfaces). However, we are nowadays
>> requiring this only on documentation level. So if user starts a
>> domain with 'floor' set on one its interfaces, we silently ignore
>> the setting. We should error out instead.
>> ---
>>  src/network/bridge_driver.c | 17 ++++++++++++++---
>>  1 file changed, 14 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
>> index 31c8585..330c147 100644
>> --- a/src/network/bridge_driver.c
>> +++ b/src/network/bridge_driver.c
>> @@ -4535,11 +4535,22 @@ networkCheckBandwidth(virNetworkObjPtr net,
>>      unsigned long long tmp_new_rate = 0;
>>      char ifmac[VIR_MAC_STRING_BUFLEN];
>>  
>> +    virMacAddrFormat(&iface->mac, ifmac);
>> +
>> +    if (ifaceBand && ifaceBand->in && ifaceBand->in->floor &&
>> +        !(netBand && netBand->in)) {
>> +        virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
>> +                       _("Cannot use 'floor' on %s because network '%s' "
>> +                         "does not have any inbound QoS set"),
> 
> How about "Invalid use of 'floor' on interface with mac address %s -
> network '%s' has no inbound QoS set"
> 
>> +                       ifmac, net->def->name);
>> +        return -1;
>> +    }
>> +
>>      if (!ifaceBand || !ifaceBand->in || !ifaceBand->in->floor ||
>> -        !netBand || !netBand->in)
>> +        !netBand || !netBand->in) {
>> +        /* no QoS required, claim success */
>>          return 1;
>> -
>> -    virMacAddrFormat(&iface->mac, ifmac);
>> +    }
>>  
>>      tmp_new_rate = netBand->in->average;
>>      tmp_floor_sum += ifaceBand->in->floor;
> 
> Looks safe enough. ACK with or without a changed log message.

I've changed the error message and pushed. Thanks.

Michal