Re: [PATCH v3 1/2] network: Refactor code to add bandwidth support for ethernet interfaces

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

 



On 10/08/2014 01:55 PM, Anirban Chakraborty wrote:
> Modified code to set and clear bandwidth cleanly.

[meta-comment - this mail wasn't threaded to the 0/2 cover letter, but
came through as its own top-level thread.  This makes it harder to track
the series]

> 
> Signed-off-by: Anirban Chakraborty <abchak@xxxxxxxxxxx>
> ---
>  src/conf/domain_conf.h      |  7 +++++++
>  src/lxc/lxc_driver.c        |  3 +++
>  src/lxc/lxc_process.c       | 18 +++++++++---------
>  src/qemu/qemu_command.c     | 25 +++++++++++++++++++------
>  src/qemu/qemu_command.h     |  2 ++
>  src/qemu/qemu_driver.c      |  3 +++
>  src/qemu/qemu_hotplug.c     | 12 ++++++++++++
>  src/qemu/qemu_process.c     |  3 +++
>  src/util/virnetdevmacvlan.c | 10 ----------
>  src/util/virnetdevmacvlan.h |  1 -
>  10 files changed, 58 insertions(+), 26 deletions(-)

Style review (I'll leave the content review to someone a bit more
familiar with network code)

> 
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index afa3da6..f03599e 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -2848,4 +2848,11 @@ int virDomainObjSetMetadata(virDomainObjPtr vm,
>  bool virDomainDefNeedsPlacementAdvice(virDomainDefPtr def)
>      ATTRIBUTE_NONNULL(1);
> 
> +static inline bool virNetDevSupportBandwidth(int type)
> +{
> +    return ((type == VIR_DOMAIN_NET_TYPE_BRIDGE ||
> +     type == VIR_DOMAIN_NET_TYPE_NETWORK ||
> +     type == VIR_DOMAIN_NET_TYPE_DIRECT) ? true : false);

I think 'return cond ? true : false;' is a waste of source code.  Just
'return cond;'.  Indentation looks awkward; I would have done it:

    return (type == VIR_DOMAIN_NET_TYPE_BRIDGE ||
            type == VIR_DOMAIN_NET_TYPE_NETWORK ||
            type == VIR_DOMAIN_NET_TYPE_DIRECT);

> +};

Spurious semicolon.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]