[libvirt] [PATCH] virsh: add transient interfaces check for iface-unbridge

[Laine Stump]

(BTW, I assume that you are running suse, and using netcf behind the
iface-* commands. I didn't realize that the suse port of netcf was being
officially used anywhere, as there hasn't been any suse-specific
modification to upstream netcf since the suse port was originally added
in 2011. If there are downstream patches, it would be great to have them
upstream as well.)

On 12/17/2014 05:34 AM, Lin Ma wrote:
> iface-unbridge(netcf interface backend) checks multiple interfaces
> attaching based on static configuration.
> If guests interfaces(says tun/tap devices) are attaching to the bridge,
> iface-unbridge doesn't check them, It causes the bridge is removed
> although it's in use by other guests.
>
> Please refer to:
> https://bugzilla.suse.com/show_bug.cgi?id=813117

In my opinion, if we're going to do this, then we should also modify
virsh net-destroy to not allow shutting down a network if there are
guests still using it - it's essentially the same thing. This isn't a
simple bug that has a definite simple fix, it requires agreement on
philosophy. For a very long time virsh has allowed removing networks
(net-destroy) even when a guest is connected (just as it permits "virsh
destroy" of a guest that hasn't cleanly shutdown its OS); I don't know
if this was originally an omission, if it was done because there was
originally no simple way to check (the reporting of number of
connections in the network status xml is a fairly recent addition), or
if it was done on purpose in order to not prevent something that someone
may legitimately want to do, but virsh iface-unbridge was written to
have the same laissez faire attitude about guest connections.

The existing check for multiple *configured* attached devices though, is
there because I wanted iface-unbridge to be able to undo exactly what
iface-bridge was doing - put a single ethernet interface behind a
bridge. If there are multiple configured interfaces, that can't be done
because there is no way to know which of those two interfaces should get
the IP configuration from the bridge device (and if there are *0*
configured interfaces, the IP config from the bridge would be lost).


On 12/17/2014 10:48 AM, Lin Ma wrote:
>
>  
>
> >>> Michal Privoznik <mprivozn at redhat.com> 2014-12-17 下午 20:34 >>>
> >On 17.12.2014 11:34, Lin Ma wrote:
> >> iface-unbridge(netcf interface backend) checks multiple interfaces
> >> attaching based on static configuration.
> >> If guests interfaces(says tun/tap devices) are attaching to the bridge,
> >> iface-unbridge doesn't check them, It causes the bridge is removed
> >> although it's in use by other guests.
> >>
> >> Please refer to:
> >> https://bugzilla.suse.com/show_bug.cgi?id=813117
> >
> > The bug is not publicly viewable. We tend to not put the BZ URL in the
> > commit message in that case.
>
> sorry about that, the URL will be removed, Thanks.


Even better - if there is no sensitive information in the BZ, perhaps
you could make it public (or maybe the BZ can be sanitized to mark only
the sensitive parts as private).


>
> >> @@ -1103,6 +1103,21 @@ cmdInterfaceUnbridge(vshControl *ctl, const
> vshCmd *cmd)
> >> goto cleanup;
> >> }
> >>
> >> + /* verify whether there is any transient interface attached to
> bridge. */
> >> + if (!(br_xml_transient_if = virInterfaceGetXMLDesc(br_handle, 0)))
> >> + goto cleanup;
> >> + if (!(xml_doc_transient_if =
> virXMLParseStringCtxt(br_xml_transient_if,
> >> + _("(bridge interface definition)"),
> >> + &ctxt_transient_if))) {
> >> + vshError(ctl, _("Failed to parse configuration of %s"), br_name);
> >> + goto cleanup;
> >> + }
> >> +
> >> + if (virXPathNode("./bridge/interface[2]", ctxt_transient_if) !=
> NULL) {
> >> + vshError(ctl, "%s", _("The bridge is in use by transient
> interfaces"));
> >> + goto cleanup;
> >> + }
> >> +
> >
> > This effectively copies the code just a few lines above (not to be seen
> > in this context though). The only difference is, that your code gest
> > LIVE bridge XML, while pre-exisitng code dumps INACTIVE XML. Can we turn
> > already existing code into checking LIVE XML instead?


No. That is there for a different reason, and is checking something
different (see above).


> It feels more
> > appropriate too. Live bridge can have only one interface plugged in (due
> > to hot-unplugging) regardless of what INACTIVE XML says.


The interfaces listed in the config are the ethernet/bond/vlan
interfaces that are configured to be attached to the bridge when it is
created. Any interfaces beyond that are most likely tap devices
connecting guests (although that isn't necessarily the case).


> Actually, at the beginning of writing this patch, I tended to turn
> already existing code
> into checking LIVE XML instead, But if I do so, it can't deal with
> this extreme circumstance:
> says no any physical interface attached to the bridge, But one guest
> tap backend is
> attaching to this bridge. The bridge will be removed in this case
> instead of reporting "No
> interface attached to bridge".


Again, if there is no configured interface that is attached to the
bridge, there is no place to put the bridge's IP config, so executing
"iface-unbridge" would permanently lose that information, and must be
avoided.


>
> Now I thought about it, no body configures the bridge and guest
> interface like that,
> that extreme circumstance is meaningless and useless scenario, So I
> agreed with your idea.
>
> any else suggestions?


I think the reasons for those two checks are being misunderstood. When
there is no configured interface attached to the bridge, removing the
bridge will lose the IP config; where there are multiple configured
interfaces attached to the bridge, the desired outcome is ambiguous. The
live XML gives us different information.

My opinion is that this bug report should be closed as NOTABUG (or virsh
net-destroy should also be changed to be prohibited when guests are
connected, but it's possible that could break existing scripts - I
remember that before "virsh net-update" existed, when somebody wanted to
change a running network, they would make the changes to the network,
net-destroy and net-start it, then go through all the running domains,
detaching and re-attaching all interfaces using that network. It's very
likely there are still scripts like that hanging around)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20141217/7c5650e0/attachment-0001.htm>