<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_1423148693000_lma@suse.com_B118D7016C3D0000BC7D000BCD69755F_" class="GroupWiseMessageBody">>>>> Laine Stump <laine@laine.org> 2015-2-4 下午 17:12 >>> >On 02/03/2015 11:39 AM, Michal Privoznik wrote: >> On 02.02.2015 15:08, Lin Ma wrote: >>> By checking transient interfaces, It obtains the live information of >>> attached interfaces to see if the bridge is in use. >>> >>> Signed-off-by: Lin Ma <lma@suse.com> </lma@suse.com><lma@suse.com>>>> --- </lma@suse.com><lma@suse.com>>>> tools/virsh-interface.c | 25 ++++++++++++++++++++++--- </lma@suse.com><lma@suse.com>>>> 1 file changed, 22 insertions(+), 3 deletions(-) </lma@suse.com><lma@suse.com>>> Technically, this is a v2 to a previous patch (I mildly recall seeing </lma@suse.com><lma@suse.com>>> something like this in the past). </lma@suse.com>> <lma@suse.com>>It looks to be the same patch, just with reference to a private bug </lma@suse.com><lma@suse.com>>report removed, and preceded with the check for net-destroy (since I had </lma@suse.com><lma@suse.com>>said in my response to the original patch that the behavior of </lma@suse.com><lma@suse.com>>iface-unbridge was made to be similar to net-destroy, and that my </lma@suse.com><lma@suse.com>>opinion was that either neither should be changed, or both). </lma@suse.com>It seems like that we decided to keep the original net-destroy behaviour, Then let's keep iface-unbridge's too. <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..ff40be0 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,22 @@ 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>>>> + </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>>>> /* Change the type and name of the outer/master interface to </lma@suse.com><lma@suse.com>>>> * the type/name of the attached slave interface. </lma@suse.com><lma@suse.com>>>> */ </lma@suse.com><lma@suse.com>>>> @@ -1198,10 +1214,13 @@ cmdInterfaceUnbridge(vshControl *ctl, const vshCmd *cmd) </lma@suse.com><lma@suse.com>>>> virInterfaceFree(br_handle); </lma@suse.com><lma@suse.com>>>> VIR_FREE(if_xml); </lma@suse.com><lma@suse.com>>>> VIR_FREE(br_xml); </lma@suse.com><lma@suse.com>>>> + VIR_FREE(br_xml_transient_if); </lma@suse.com><lma@suse.com>>>> VIR_FREE(if_type); </lma@suse.com><lma@suse.com>>>> VIR_FREE(if_name); </lma@suse.com><lma@suse.com>>>> xmlXPathFreeContext(ctxt); </lma@suse.com><lma@suse.com>>>> + xmlXPathFreeContext(ctxt_transient_if); </lma@suse.com><lma@suse.com>>>> xmlFreeDoc(xml_doc); </lma@suse.com><lma@suse.com>>>> + xmlFreeDoc(xml_doc_transient_if); </lma@suse.com><lma@suse.com>>>> return ret; </lma@suse.com><lma@suse.com>>>> } </lma@suse.com><lma@suse.com>>>> </lma@suse.com><lma@suse.com>>>> </lma@suse.com><lma@suse.com>>> ACK. I'll merge this tomorrow (unless somebody beats me). </lma@suse.com>> <lma@suse.com>>Please don't push it as is. I think the behavior of iface-unbridge </lma@suse.com><lma@suse.com>>should match whatever is done to net-destroy (if anything). If the </lma@suse.com><lma@suse.com>>change is made, it should be made to both at the same time, and this one </lma@suse.com><lma@suse.com>>should also have a --force option to allow overriding the extra check, </lma@suse.com><lma@suse.com>>as patch 2/3 does for net-destroy. </lma@suse.com><lma@suse.com>As Daniel points out, destroy is The net-destroy already shows to user that it's a forceful operation, So we don't need a --force option, then iface-unbridge either. </lma@suse.com></div></body></html>