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

[Michal Privoznik]

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.

Michal