[libvirt PATCH 2/5] qemu: check if 'floor' is supported for given interface and network
Michal Privoznik
mprivozn at redhat.com
Wed Feb 12 08:20:55 UTC 2020
On 2/10/20 5:10 PM, Pavel Mores wrote:
> Even if an interface of type 'network', setting 'floor' is only supported
> if the network's forward type is nat, route, open or none.
>
> Signed-off-by: Pavel Mores <pmores at redhat.com>
> ---
> src/network/bridge_driver.c | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index 94212eec77..3b70e52afd 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -5062,9 +5062,26 @@ networkCheckBandwidth(virNetworkObjPtr obj,
> unsigned long long tmp_floor_sum = virNetworkObjGetFloorSum(obj);
> unsigned long long tmp_new_rate = 0;
> char ifmac[VIR_MAC_STRING_BUFLEN];
> + virNetworkForwardType fwdType;
> + bool floorSupported;
> + bool floorRequested;
>
> virMacAddrFormat(ifaceMac, ifmac);
>
> + fwdType = def->forward.type;
> + floorSupported = fwdType == VIR_NETWORK_FORWARD_NONE ||
> + fwdType == VIR_NETWORK_FORWARD_NAT ||
> + fwdType == VIR_NETWORK_FORWARD_ROUTE ||
> + fwdType == VIR_NETWORK_FORWARD_OPEN;
What if this was turned into a function? For instance:
static inline bool virNetDevSupportBandwidthFloor(virNetworkForwardType
type)
{
switch (type) {
case VIR_NETWORK_FORWARD_NONE:
case VIR_NETWORK_FORWARD_NAT:
case VIR_NETWORK_FORWARD_ROUTE:
case VIR_NETWORK_FORWARD_OPEN:
return true;
case VIR_NETWORK_FORWARD_BRIDGE:
case VIR_NETWORK_FORWARD_PRIVATE:
case VIR_NETWORK_FORWARD_VEPA:
case VIR_NETWORK_FORWARD_PASSTHROUGH:
case VIR_NETWORK_FORWARD_HOSTDEV:
case VIR_NETWORK_FORWARD_LAST:
break;
}
return false;
}
which could live next to virNetDevSupportBandwidth(). Then you could
just call this function instead of having an explicit variable.
> +
> + floorRequested = ifaceBand && ifaceBand->in && ifaceBand->in->floor != 0;
So this pattern repeats enough times to be turned into a separate
function IMO. Again, it can be a simple inline function, e.g.:
static inline bool virNetDevBandwidthHasFloor(const virNetDevBandwidth *b)
{
return b && b->in && b->in->floor != 0;
}
[Side note, out->floor can never be set, as it doesn't make any sense.
See virNetDevBandwidthParse()]
> +
> + if (floorRequested && !floorSupported) {
> + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> + _("'floor' is only supported for interface type 'network' with forward type 'nat', 'route', 'open' or none"));
> + return -1;
> + }
> +
> if (ifaceBand && ifaceBand->in && ifaceBand->in->floor &&
> !(netBand && netBand->in)) {
> virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
>
Then, I suggest some reordering of patches. Personally, I like cleanup
patches to go first and only then patches that do something useful, i.e.
bugfixes, feature implementation.
Michal
More information about the libvir-list
mailing list