[libvirt] [PATCH 1/2] network: only clear bandwidth if it has been set

[Michal Privoznik]

On 25.02.2015 17:29, Laine Stump wrote:
> On 02/25/2015 06:12 AM, Michal Privoznik wrote:
>> On 24.02.2015 20:39, Laine Stump wrote:
>>> libvirt was unconditionally calling virNetDevBandwidthClear() for
>>> every interface (and network bridge) of a type that supported
>>> bandwidth, whether it actually had anything set or not. This doesn't
>>> hurt anything (unless ifname == NULL!), but is wasteful.
>>>
>>> This patch makes sure that all calls to virNetDevBandwidthClear() are
>>> qualified by checking that ifname != NULL and that it really had some
>>> bandwidth setup done.
>>>
>>> (NB: an sa_assert(detach->ifname) was added in
>>> lxcDomainDetachDeviceNetLive() because the preceding new check of
>>> detach->ifname before calling virNetDevBandwidthClear() caused
>>> Coverity to complain about a possible null dereference.)
>>> ---
>>>  src/conf/netdev_bandwidth_conf.c | 6 ++++--
>>>  src/lxc/lxc_driver.c             | 7 ++++++-
>>>  src/network/bridge_driver.c      | 6 ++++--
>>>  src/qemu/qemu_hotplug.c          | 4 +++-
>>>  4 files changed, 17 insertions(+), 6 deletions(-)
>>>
>> How about moving the check for non-NULL ifname into
>> virNetDevBandwidthClear()? Its counterpart virNetDevBandwidthSet() does
>> something similar over @bandwidth argument. And after 2/2 it'll done the
>> same over @ifname.
> 
> Yeah, I had some reason for doing it externally originally, but then
> forgot the reason (or it went away), then after I fixed
> virNetDevBandwidthSet() I thought about changing this one, and decided
> against it, mostly because I was tired and couldn't be certain that I
> *didn't* have a valid reason in the first place. I'll do another spin
> that way.
> 

I don't want to discourage you to post patches, but I believe they are
easy enough to be fixed locally and pushed. I don't want to enforce v2
just because of this small nit.

Michal