<html><head><meta http-equiv=Content-Type content="text/html; charset=utf-8"><META name="Author" content="Novell GroupWise WebAccess"><style type="text/css"> body p { margin: 0px; } </style></head><body style='font-family: Helvetica, Arial, sans-serif; font-size: 13px; '><div id="GroupWiseSection_1418830693000_lma@suse.com_B118D7016C3D0000BC7D000BCD69755F_" class="GroupWiseMessageBody"><div> </div> >>> Michal Privoznik <mprivozn@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. >> >> Signed-off-by: Lin Ma <lma@suse.com> </lma@suse.com><lma@suse.com>>> --- </lma@suse.com><lma@suse.com>>> tools/virsh-interface.c | 24 +++++++++++++++++++++--- </lma@suse.com><lma@suse.com>>> 1 file changed, 21 insertions(+), 3 deletions(-) </lma@suse.com><lma@suse.com>>> </lma@suse.com><lma@suse.com>>> diff --git a/tools/virsh-interface.c b/tools/virsh-interface.c </lma@suse.com><lma@suse.com>>> index 5f848b6..63ba5bb 100644 </lma@suse.com><lma@suse.com>>> --- a/tools/virsh-interface.c </lma@suse.com><lma@suse.com>>> +++ b/tools/virsh-interface.c </lma@suse.com><lma@suse.com>>> @@ -1040,11 +1040,11 @@ cmdInterfaceUnbridge(vshControl *ctl, const vshCmd *cmd) </lma@suse.com><lma@suse.com>>> const char *br_name; </lma@suse.com><lma@suse.com>>> char *if_type = NULL, *if_name = NULL; </lma@suse.com><lma@suse.com>>> bool nostart = false; </lma@suse.com><lma@suse.com>>> - char *br_xml = NULL; </lma@suse.com><lma@suse.com>>> + char *br_xml = NULL, *br_xml_transient_if = NULL; </lma@suse.com><lma@suse.com>>> xmlChar *if_xml = NULL; </lma@suse.com><lma@suse.com>>> int if_xml_size; </lma@suse.com><lma@suse.com>>> - xmlDocPtr xml_doc = NULL; </lma@suse.com><lma@suse.com>>> - xmlXPathContextPtr ctxt = NULL; </lma@suse.com><lma@suse.com>>> + xmlDocPtr xml_doc = NULL, xml_doc_transient_if = NULL; </lma@suse.com><lma@suse.com>>> + xmlXPathContextPtr ctxt = NULL, ctxt_transient_if = NULL; </lma@suse.com><lma@suse.com>>> xmlNodePtr top_node, if_node, cur; </lma@suse.com><lma@suse.com>>> </lma@suse.com><lma@suse.com>>> /* Get a handle to the original device */ </lma@suse.com><lma@suse.com>>> @@ -1103,6 +1103,21 @@ cmdInterfaceUnbridge(vshControl *ctl, const vshCmd *cmd) </lma@suse.com><lma@suse.com>>> goto cleanup; </lma@suse.com><lma@suse.com>>> } </lma@suse.com><lma@suse.com>>> </lma@suse.com><lma@suse.com>>> + /* verify whether there is any transient interface attached to bridge. */ </lma@suse.com><lma@suse.com>>> + if (!(br_xml_transient_if = virInterfaceGetXMLDesc(br_handle, 0))) </lma@suse.com><lma@suse.com>>> + goto cleanup; </lma@suse.com><lma@suse.com>>> + if (!(xml_doc_transient_if = virXMLParseStringCtxt(br_xml_transient_if, </lma@suse.com><lma@suse.com>>> + _("(bridge interface definition)"), </lma@suse.com><lma@suse.com>>> + &ctxt_transient_if))) { </lma@suse.com><lma@suse.com>>> + vshError(ctl, _("Failed to parse configuration of %s"), br_name); </lma@suse.com><lma@suse.com>>> + goto cleanup; </lma@suse.com><lma@suse.com>>> + } </lma@suse.com><lma@suse.com>>> + </lma@suse.com><lma@suse.com>>> + if (virXPathNode("./bridge/interface[2]", ctxt_transient_if) != NULL) { </lma@suse.com><lma@suse.com>>> + vshError(ctl, "%s", _("The bridge is in use by transient interfaces")); </lma@suse.com><lma@suse.com>>> + goto cleanup; </lma@suse.com><lma@suse.com>>> + } </lma@suse.com><lma@suse.com>>> + </lma@suse.com>> <lma@suse.com>> This effectively copies the code just a few lines above (not to be seen </lma@suse.com><lma@suse.com>> in this context though). The only difference is, that your code gest </lma@suse.com><lma@suse.com>> LIVE bridge XML, while pre-exisitng code dumps INACTIVE XML. Can we turn </lma@suse.com><lma@suse.com>> already existing code into checking LIVE XML instead? It feels more </lma@suse.com><lma@suse.com>> appropriate too. Live bridge can have only one interface plugged in (due </lma@suse.com><lma@suse.com>> to hot-unplugging) regardless of what INACTIVE XML says. 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". Now I thought about it, no body configures the bridge and guest interface like that, that </lma@suse.com><lma@suse.com>extreme circumstance is meaningless and useless scenario, So I agreed with your idea. any else suggestions? Thanks, Lin </lma@suse.com></div></body></html>