When the <bandwidth> of an interface is changed with update-device, the old settings are cleared with tc, then new settings added with tc. But if the <bandwidth has been removed, the old settings weren't being removed, so the bandwidth restrictions would still be active on the interface although the interface status in libvirt showed that they had been removed. This patch fixes it by calling virNetDevBandwidthClear() if the "modification" to the interface bandwidth was to completely clear it. An alternative could have been to modify virNetDevBandwidthSet() to always clear existing bandwith settings at the beginning of the function (currently it short circuits in that case, doing nothing), but that would have led to cases where virNetDevBandwidthClear() was now being called in cases where it previously wasn't, and while many of those cases would be NOPs, there could be cases where it would cause an error. The way this patch works, the ...Clear() function is only called in cases where the ...Set() function had previously been called successfully, so the risk of regression is minimized. Resolves: https://bugzilla.redhat.com/1454709 --- I'm currently unable to test this fix because https://bugzilla.redhat.com/1514963 (filed against the iproute package) is unresolved in the version of Fedora running on my host. src/qemu/qemu_hotplug.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 24631cb8f..3658fc20a 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3318,11 +3318,19 @@ qemuDomainChangeNet(virQEMUDriverPtr driver, } if (needBandwidthSet) { - if (virNetDevBandwidthSet(newdev->ifname, - virDomainNetGetActualBandwidth(newdev), - false, - !virDomainNetTypeSharesHostView(newdev)) < 0) - goto cleanup; + virNetDevBandwidthPtr newb = virDomainNetGetActualBandwidth(newdev); + + if (newb) { + if (virNetDevBandwidthSet(newdev->ifname, newb, false, + !virDomainNetTypeSharesHostView(newdev)) < 0) + goto cleanup; + } else { + /* + * virNetDevBandwidthSet() doesn't clear any existing + * setting unless something new is being set. + */ + virNetDevBandwidthClear(newdev->ifname); + } needReplaceDevDef = true; } -- 2.13.6

ebtables/iptables processing is skipped for any interface connected to Open vSwitch (they have their own packet filtering), likewise for midonet (according to http://blog.midokura.com/2016/04/midonet-rule-chains), but libvirt would allow adding a <filterref> to interfaces connected in these ways, so the user might mistakenly believe they were being protected. This patch checks for a non-NULL <virtualport> element for an interface (or its network) and logs an error if <virtualport> and <filterref> are both present. This could cause some previously working domains to no longer start, but that's really the whole point of this patch - to warn people that their filterref isn't protecting them as they might have thought. I don't bother checking this during post-parse validation, because such a check would be incomplete - it's possible that a network would have a <virtualport> that would be applied to an interface, and you can't know that until the domain is started. Resolves: https://bugzilla.redhat.com/1502754 --- src/qemu/qemu_command.c | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 2dd50a214..4d0c141e5 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8545,15 +8545,25 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver, } /* and only TAP devices support nwfilter rules */ - if (net->filter && - !(actualType == VIR_DOMAIN_NET_TYPE_NETWORK || - actualType == VIR_DOMAIN_NET_TYPE_BRIDGE || - actualType == VIR_DOMAIN_NET_TYPE_ETHERNET)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("filterref is not supported for " - "network interfaces of type %s"), - virDomainNetTypeToString(actualType)); - return -1; + if (net->filter) { + virNetDevVPortProfilePtr vport = virDomainNetGetActualVirtPortProfile(net); + if (!(actualType == VIR_DOMAIN_NET_TYPE_NETWORK || + actualType == VIR_DOMAIN_NET_TYPE_BRIDGE || + actualType == VIR_DOMAIN_NET_TYPE_ETHERNET)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("filterref is not supported for " + "network interfaces of type %s"), + virDomainNetTypeToString(actualType)); + return -1; + } + if (vport && vport->virtPortType != VIR_NETDEV_VPORT_PROFILE_NONE) { + /* currently none of the defined virtualport types support iptables */ + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("filterref is not supported for " + "network interfaces with virtualport type %s"), + virNetDevVPortTypeToString(vport->virtPortType)); + return -1; + } } if (net->backend.tap && -- 2.13.6