[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 1/2] qemu: Fix updating bandwidth limits in live XML




On 09/22/2014 06:41 AM, Erik Skultety wrote:
> When trying to update bandwidth limits on a running domain, limits get
> updated in our internal structures, however XML parser reads
> bandwidth limits from network 'actual' definition. Commiting this patch

s/Commiting/Committing

> it is now available to update bandwidth 'actual' definition as well,
> thus updating domain runtime XML
> ---
>  src/qemu/qemu_driver.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 702d3cc..ede8880 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -10028,7 +10028,19 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
>          } else {
>              net->bandwidth = NULL;
>          }
> +
> +        if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
> +                virNetDevBandwidthFree(net->data.network.actual->bandwidth);

This will set net->data.network.actual->bandwidth to NULL

It's also remove it when net->bandwidth == NULL thus
causing actual to be lost, which it doesn't seem is 
desired, but perhaps it is.  Gues

> +                if (!net->bandwidth ||
> +                    virNetDevBandwidthCopy(&net->data.network.actual->bandwidth,
> +                                           net->bandwidth) < 0)
> +                    net->data.network.actual->bandwidth = NULL;

Making this irrelevant, but I wonder if the < 0 here meant to do
something else perhaps?

> +        }

The above hunk needs some space formatting.  Also since the
virNetDevBandwidthCopy() has a if (!src) check, "(!net->bandwidth) ||"
is unnecessary.

if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
    virNetDevBandwidthFree(net->data.network.actual->bandwidth);
    if (virNetDevBandwidthCopy(&net->data.network.actual->bandwidth,
                               net->bandwidth) < 0)
        goto cleanup
}

Whether this is what is "expected" is perhaps something Laine can answer...

John

> +
> +        if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
> +            goto cleanup;
>      }
> +
>      if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
>          if (!persistentNet->bandwidth) {
>              persistentNet->bandwidth = bandwidth;
> 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]