On 4/18/19 6:32 AM, Daniel P. Berrangé wrote:
On Wed, Apr 17, 2019 at 04:33:54PM -0400, Laine Stump wrote:
On 4/17/19 1:19 PM, Daniel P. Berrangé wrote:
The virNetDevBandwidthParse method uses the interface type to decide
whether to allow use of the "floor" parameter. Using the interface
type is not convenient as callers may not have that available, but
still wish to allow use of "floor".
Also, this is one of the things that gave rise to the distinction between
actualType == NETWORK and actualType == BRIDGE, and this patch allows us to
eliminate that (hopefully) without causing breakage.
Well sort of. If using a plain bridge virtual network you can't
use floor - that's only valid for routed networks. Trying to enforce
this at parse time is doomed though - you can only corrrectly report
this at runtime as you need the context of the virtual network itself.
This was already an issue when parsing the top level netdef - only
the actual netdef had any better checking.
I'm almost inclined to remove all the logic preventing "floor" at
XML parse time and rely on runtime checks entirely.
Except that the same parse function is used for the <bandwidth> element
in <network>, which never allows floor to be set. For the <bandwidth> in
an interface though, you're correct that it's impossible to know beforehand.
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 51aa48f421..0df3c2ed49 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11270,7 +11270,7 @@ virDomainActualNetDefParseXML(xmlNodePtr node,
if (bandwidth_node &&
virNetDevBandwidthParse(&actual->bandwidth,
bandwidth_node,
- actual->type) < 0)
+ def->type == VIR_DOMAIN_NET_TYPE_NETWORK) < 0)
This bit here ^^^ doesn't compile, since def is undefined. You would need to
check "parent->type" instead.
Should be "actual->type" in fact.
I was thinking in terms of making it still correct in the "new world
order" where actual->type will always be bridge rather than network. But
then later I saw that this line is removed in the next patch anyway, so
it's kind of irrelevant - for the state of everything at the point of
*this* patch, the two are equivalent.
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
