[libvirt] [PATCH] bandwidth: Require network QoS if interface uses 'floor'
By current implementation, network inbound is required in order to use 'floor' for guaranteeing minimal throughput. This is so, because we want user to tell us the maximal throughput of the network instead of finding out ourselves (and detect bogus values in case of virtual interfaces). However, we are nowadays requiring this only on documentation level. So if user starts a domain with 'floor' set on one its interfaces, we silently ignore the setting. We should error out instead. --- src/network/bridge_driver.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 31c8585..330c147 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -4535,11 +4535,22 @@ networkCheckBandwidth(virNetworkObjPtr net, unsigned long long tmp_new_rate = 0; char ifmac[VIR_MAC_STRING_BUFLEN]; + virMacAddrFormat(&iface->mac, ifmac); + + if (ifaceBand && ifaceBand->in && ifaceBand->in->floor && + !(netBand && netBand->in)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("Cannot use 'floor' on %s because network '%s' " + "does not have any inbound QoS set"), + ifmac, net->def->name); + return -1; + } + if (!ifaceBand || !ifaceBand->in || !ifaceBand->in->floor || - !netBand || !netBand->in) + !netBand || !netBand->in) { + /* no QoS required, claim success */ return 1; - - virMacAddrFormat(&iface->mac, ifmac); + } tmp_new_rate = netBand->in->average; tmp_floor_sum += ifaceBand->in->floor; -- 1.8.1.5
On 03/07/2013 05:02 AM, Michal Privoznik wrote:
By current implementation, network inbound is required in order to use 'floor' for guaranteeing minimal throughput. This is so, because we want user to tell us the maximal throughput of the network instead of finding out ourselves (and detect bogus values in case of virtual interfaces). However, we are nowadays requiring this only on documentation level. So if user starts a domain with 'floor' set on one its interfaces, we silently ignore the setting. We should error out instead. --- src/network/bridge_driver.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 31c8585..330c147 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -4535,11 +4535,22 @@ networkCheckBandwidth(virNetworkObjPtr net, unsigned long long tmp_new_rate = 0; char ifmac[VIR_MAC_STRING_BUFLEN];
+ virMacAddrFormat(&iface->mac, ifmac); + + if (ifaceBand && ifaceBand->in && ifaceBand->in->floor && + !(netBand && netBand->in)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("Cannot use 'floor' on %s because network '%s' " + "does not have any inbound QoS set"),
How about "Invalid use of 'floor' on interface with mac address %s - network '%s' has no inbound QoS set"
+ ifmac, net->def->name); + return -1; + } + if (!ifaceBand || !ifaceBand->in || !ifaceBand->in->floor || - !netBand || !netBand->in) + !netBand || !netBand->in) { + /* no QoS required, claim success */ return 1; - - virMacAddrFormat(&iface->mac, ifmac); + }
tmp_new_rate = netBand->in->average; tmp_floor_sum += ifaceBand->in->floor;
Looks safe enough. ACK with or without a changed log message.
On 11.03.2013 04:53, Laine Stump wrote:
On 03/07/2013 05:02 AM, Michal Privoznik wrote:
By current implementation, network inbound is required in order to use 'floor' for guaranteeing minimal throughput. This is so, because we want user to tell us the maximal throughput of the network instead of finding out ourselves (and detect bogus values in case of virtual interfaces). However, we are nowadays requiring this only on documentation level. So if user starts a domain with 'floor' set on one its interfaces, we silently ignore the setting. We should error out instead. --- src/network/bridge_driver.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 31c8585..330c147 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -4535,11 +4535,22 @@ networkCheckBandwidth(virNetworkObjPtr net, unsigned long long tmp_new_rate = 0; char ifmac[VIR_MAC_STRING_BUFLEN];
+ virMacAddrFormat(&iface->mac, ifmac); + + if (ifaceBand && ifaceBand->in && ifaceBand->in->floor && + !(netBand && netBand->in)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("Cannot use 'floor' on %s because network '%s' " + "does not have any inbound QoS set"),
How about "Invalid use of 'floor' on interface with mac address %s - network '%s' has no inbound QoS set"
+ ifmac, net->def->name); + return -1; + } + if (!ifaceBand || !ifaceBand->in || !ifaceBand->in->floor || - !netBand || !netBand->in) + !netBand || !netBand->in) { + /* no QoS required, claim success */ return 1; - - virMacAddrFormat(&iface->mac, ifmac); + }
tmp_new_rate = netBand->in->average; tmp_floor_sum += ifaceBand->in->floor;
Looks safe enough. ACK with or without a changed log message.
I've changed the error message and pushed. Thanks. Michal
participants (2)
-
Laine Stump -
Michal Privoznik