Re: [libvirt] [PATCH v3 05/36] network: use 'bridge' as actual type instead of 'network'

On 3/21/19 10:16 PM, Laine Stump wrote: > On 3/21/19 9:07 PM, Cole Robinson wrote: >> On 3/19/19 8:46 AM, Daniel P. Berrangé wrote: >>> Ports allocated on virtual networks with type=nat|route|open all get >>> given an actual type of 'network'. >>> >>> Only ports in networks with type=bridge use an actual type of 'bridge'. >>> >>> This distinction makes little sense since the virtualization drivers >>> will treat both actual types in exactly the same way, as they're all >>> just bridge devices a VM needs to be connected to. >>> >>> This doesn't affect user visible XML since the "actual" device XML >>> is internal only, but we need code to convert the data upgrades. >>> >>> Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; >>> --- >>>   src/network/bridge_driver.c | 20 ++++++++++++-------- >>>   1 file changed, 12 insertions(+), 8 deletions(-) >>> >> Conceptually this makes sense. I wonder what the original motivation for >> the differentiation was. > > > I remember there was *some* reason for it, but it's very possible it's > not valid. So lacking any solid memory from that distant time, one thing > to do is audit all occurrences of VIR_DOMAIN_NET_TYPE_NETWORK where the > actualType is being compared (rather than the NetDef's type), and > behavior is different from VIR_DOMAIN_NET_TYPE_BRIDGE (which is > apparently what Cole did, since he's already found most of the > "interesting" places I found :-)). He mentions one that he found below, > and he also pointed out the class_id thing during parsing in the > previous patch, which will now be broken even if we don't support > bandwidth on unmanaged bridge networks (because class_id is only parsed > when actualType == NETWORK). I'll see if I find any others with a quick > search... > It's difficult to audit: to know whether actualType can even be TYPE_NETWORK after this patch requires knowing if the caller is acting on a live/starting VM or not which is context dependent.