[libvirt] [PATCH] Clear bandwidth settings for a shutoff domain using domiftune

[jiahu]

On 08/11/2014 06:05 PM, Michal Privoznik wrote:
> On 11.08.2014 08:41, Jianwei Hu wrote:
>> qemu: To clear bandwidth settings for a shutoff domain by using 
>> domiftune.
>>
>> After applying this patch, we can use virsh domiftune command to 
>> clear inbound
>> or/and outbound setting for a shutoff domain.
>>
>> for example:
>> virsh domiftune $domain $interface 0 0
>
> Thanks for catching this.
>
>>
>> Please refer to below virsh help message:
>>
>> man virsh:
>>
>> To clear inbound or outbound settings, use --inbound or --outbound 
>> respectfully with average value of zero.
>> ---
>>   src/qemu/qemu_driver.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index 82a82aa..7db2e9c 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -9983,11 +9983,17 @@ qemuDomainSetInterfaceParameters(virDomainPtr 
>> dom,
>>                   VIR_FREE(persistentNet->bandwidth->in);
>>                   persistentNet->bandwidth->in = bandwidth->in;
>>                   bandwidth->in = NULL;
>> +            } else {
>> +                VIR_FREE(persistentNet->bandwidth->in);
>> +                persistentNet->bandwidth->in = 0;
>
> We like NULL for pointer more than 0. Moreover, there's no need to 
> explicitly set pointer freed to NULL as the VIR_FREE() macro does that 
> already for you (in fact virFree() function does that, whatever).
>
>>               }
>>               if (bandwidth->out) {
>>                   VIR_FREE(persistentNet->bandwidth->out);
>>                   persistentNet->bandwidth->out = bandwidth->out;
>>                   bandwidth->out = NULL;
>> +            } else {
>> +                VIR_FREE(persistentNet->bandwidth->out);
>> +                persistentNet->bandwidth->out = 0;
>>               }
>>           }
>>
>>
>
> But the fix isn't quite right. For instance:
>
> virsh # domiftune dummy 52:54:00:89:3a:c2 --config
> inbound.average: 10
> inbound.peak   : 0
> inbound.burst  : 0
> outbound.average: 10
> outbound.peak  : 0
> outbound.burst : 0
>
> virsh # domiftune dummy 52:54:00:89:3a:c2 --config 100
>
> virsh # domiftune dummy 52:54:00:89:3a:c2 --config
> inbound.average: 100
> inbound.peak   : 0
> inbound.burst  : 0
> outbound.average: 0
> outbound.peak  : 0
> outbound.burst : 0
>
> The bandwidth is cleared unconditionally. What we really need is:
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 82a82aa..2c3f179 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -9983,11 +9983,15 @@ qemuDomainSetInterfaceParameters(virDomainPtr 
> dom,
>                  VIR_FREE(persistentNet->bandwidth->in);
>                  persistentNet->bandwidth->in = bandwidth->in;
>                  bandwidth->in = NULL;
> +            } else  if (inboundSpecified) {
> +                VIR_FREE(persistentNet->bandwidth->in);
>              }
>              if (bandwidth->out) {
>                  VIR_FREE(persistentNet->bandwidth->out);
>                  persistentNet->bandwidth->out = bandwidth->out;
>                  bandwidth->out = NULL;
> +            } else if (outboundSpecified) {
> +                VIR_FREE(persistentNet->bandwidth->out);
>              }
>          }
>
> I'm fixing this patch though and pushing. ACK.
Thanks for your advice and correction.
>
> Michal