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

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.