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

Re: [libvirt] [PATCH v1 08/11] network: Update status file on (un)plug



On 11/19/2012 11:51 AM, Michal Privoznik wrote:
> Status file keeps track of class_id and floor_sum. It's better
> to keep it updated in case libvirtd is killed.

I'm not sure why you're doing this type of iterative improvement in
separate patches. Since you would want this functionality in any working
version of the code, and you haven't already pushed the earlier
versions, why not just do it in the original patch that introduces these
functions?

Likewise, doing part of the functionality, then a bit of infrastructure
to allow the new functionality to work better, and then another patch to
improve the new functionality makes it a bit of a treasure hunt to
review; I like to make my patchsets so that the first patches contain
all the improvements/changes to existing infrastructure that will be
needed, then the following patches introduce the new code, fully
functioning from the beginning.


> ---
>  src/network/bridge_driver.c |   14 ++++++++++++++
>  1 files changed, 14 insertions(+), 0 deletions(-)
>
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index 8dc9d19..5a0f43f 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -4300,6 +4300,15 @@ networkNotifyPlug(virNetworkPtr network,
>      net->class_id++;
>      /* update sum of 'floor'-s of attached NICs */
>      net->floor_sum += iface->bandwidth->in->floor;
> +    /* update status file */
> +    if (virNetworkSaveStatus(NETWORK_STATE_DIR, net) < 0) {
> +        net->class_id--;
> +        net->floor_sum -= iface->bandwidth->in->floor;
> +        iface->class_id = 0;
> +        ignore_value(virNetDevBandwidthUnplug(net->def->bridge,
> +                                              net->class_id));
> +        goto cleanup;
> +    }
>      /* update rate for non guaranteed NICs */
>      new_rate -= net->floor_sum;
>      if (virNetDevBandwidthUpdateRate(net->def->bridge, "1:2",
> @@ -4339,6 +4348,11 @@ networkNotifyUnplug(virDomainNetDefPtr iface)
>              goto cleanup;
>          /* update sum of 'floor'-s of attached NICs */
>          net->floor_sum -= iface->bandwidth->in->floor;
> +        /* update status file */
> +        if (virNetworkSaveStatus(NETWORK_STATE_DIR, net) < 0) {
> +            net->floor_sum += iface->bandwidth->in->floor;
> +            goto cleanup;
> +        }
>          /* update rate for non guaranteed NICs */
>          new_rate -= net->floor_sum;
>          if (virNetDevBandwidthUpdateRate(net->def->bridge, "1:2",


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