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.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
