>>> 在 1:02 的 2014/12/18 上，在讯息 <5491B730.5050804@xxxxxxxxx> 中，Laine Stump <laine@xxxxxxxxx> 写入： > (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@xxxxxxxxxx> 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, In my understanding, virsh net-destroy should be changed as well and I will include it in patch. > 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) What if I add a "--force" option for virsh net-destroy? then those existing scrips can work without too much changes. Thanks, Lin -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list