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. > > 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. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
