[libvirt] [PATCH 6/7] Introduce virNetDevBandwidthUpdateFilter

[Laine Stump]

On 04/14/2015 12:59 PM, Michal Privoznik wrote:
> This is practically a wrapper over

"This is a simple wrapper around"

> virNetDevBandwidthManipulateFilter() that will update the desired
> filter on an interface (usually a network bridge) with a new MAC
> address. Although, the MAC address in question usually refers to
> some other interface - the one that the filter is constructed
> for. Yeah, hard to parse. Thing is, our NATed network has a

Again with the extra unnecessary verbiage :-)

> bridge where some part of QoS takes place. And vNICs from guests
> are plugged into the bridge. However, if a guest decides to
> change the MAC of its vNIC, corresponding qemu process emits an

s/corresponding/the corresponding/

> event to which we respond somehow. 

"... an event which we can use to update the QoS configuration based on
the new MAC address."

> However, our QoS hierarchy is
> currently not notified, therefore it falls apart.

"falls apart" is a bit lacking on the details :-)

>  This function
> (when called from the correct place)

(when called in response to the aforementioned event)

>  will update our QoS
> hierarchy and duck tape it together again.

Technically, it's "duct" tape (although there is one company that
capitalizes on the mispronunciation by calling theirs "Duck(tm) Tape".

>
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/libvirt_private.syms      |  1 +
>  src/util/virnetdevbandwidth.c | 38 ++++++++++++++++++++++++++++++++++++++
>  src/util/virnetdevbandwidth.h |  6 ++++++
>  3 files changed, 45 insertions(+)
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 7166283..547a919 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1753,6 +1753,7 @@ virNetDevBandwidthFree;
>  virNetDevBandwidthPlug;
>  virNetDevBandwidthSet;
>  virNetDevBandwidthUnplug;
> +virNetDevBandwidthUpdateFilter;
>  virNetDevBandwidthUpdateRate;
>  
>  
> diff --git a/src/util/virnetdevbandwidth.c b/src/util/virnetdevbandwidth.c
> index c57c8c0..0c49b41 100644
> --- a/src/util/virnetdevbandwidth.c
> +++ b/src/util/virnetdevbandwidth.c
> @@ -680,3 +680,41 @@ virNetDevBandwidthUpdateRate(const char *ifname,
>      VIR_FREE(ceil);
>      return ret;
>  }
> +
> +/**
> + * virNetDevBandwidthUpdateFilter:
> + * @ifname: interface to operate on
> + * @ifmac_ptr: new MAC to update the filter with
> + * @id: filter ID
> + *
> + * Sometimes the host environment is so dynamic, that even
> + * guest's MAC addresses change on the fly. That's when we must

s/guest's/a guest's/

s/That's when/When that happens/

> + * update our QoS hierarchy so that guest's traffic is placed

s/guest's/the guest's/

> + * into correct QDiscs.

s/correct/the correct/

>  This function does exactly that. On the
> + * interface @ifname (usually a bridge) updates filter with
> + * unique identifier @id so that it reflects fact, that new
> + * address corresponding to the filter has changed to @ifmac_ptr.

This function updates the filter for the interface @ifname with the
unique identifier @id so that it uses the new MAC address of the guest
interface @ifmac_ptr.

> + *
> + * Returns: 0 on success,
> + *         -1 on failure (with error reported).
> + */
> +int
> +virNetDevBandwidthUpdateFilter(const char *ifname,
> +                               const virMacAddr *ifmac_ptr,
> +                               unsigned int id)
> +{
> +    int ret = -1;
> +    char *class_id = NULL;
> +
> +    if (virAsprintf(&class_id, "1:%x", id) < 0)
> +        goto cleanup;
> +
> +    if (virNetDevBandwidthManipulateFilter(ifname, ifmac_ptr, id,
> +                                           class_id, true, true) < 0)
> +        goto cleanup;
> +
> +    ret = 0;
> + cleanup:
> +    VIR_FREE(class_id);
> +    return ret;
> +}
> diff --git a/src/util/virnetdevbandwidth.h b/src/util/virnetdevbandwidth.h
> index efcf95a..9b1d2a6 100644
> --- a/src/util/virnetdevbandwidth.h
> +++ b/src/util/virnetdevbandwidth.h
> @@ -73,4 +73,10 @@ int virNetDevBandwidthUpdateRate(const char *ifname,
>                                   unsigned long long new_rate)
>      ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
>      ATTRIBUTE_RETURN_CHECK;
> +
> +int virNetDevBandwidthUpdateFilter(const char *ifname,
> +                                   const virMacAddr *ifmac_ptr,
> +                                   unsigned int id)
> +    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
> +    ATTRIBUTE_RETURN_CHECK;
>  #endif /* __VIR_NETDEV_BANDWIDTH_H__ */

ACK with the description cleaned up a bit.