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

[Daniel P. Berrangé]

On Fri, Mar 22, 2019 at 10:30:08AM -0400, Cole Robinson wrote:
> 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 at redhat.com>
> >>> ---
> >>>   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.

The notion of "actual type" should not ever exist for VMs which are
not running. Only once we start the VM and resolve type=network into
a real type, does "actual type" become useful information.

There is a small window where we  are part way though starting
which it might be TYPE_NETWORK but if any code sees that it
indicates a bug in that code trying to access the network info
too early in startup.

The other place we'd see it is during shutdown/teardown of a
VM that failed during our setup process. In this case the switch
just has to allow TYPE_NETWORK but be a no-op.


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 :|