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.