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@xxxxxxxx> >> --- >> tools/virsh-interface.c | 25 ++++++++++++++++++++++--- >> 1 file changed, 22 insertions(+), 3 deletions(-) > Technically, this is a v2 to a previous patch (I mildly recall seeing > something like this in the past). It looks to be the same patch, just with reference to a private bug report removed, and preceded with the check for net-destroy (since I had said in my response to the original patch that the behavior of iface-unbridge was made to be similar to net-destroy, and that my opinion was that either neither should be changed, or both). > >> diff --git a/tools/virsh-interface.c b/tools/virsh-interface.c >> index 5f848b6..ff40be0 100644 >> --- a/tools/virsh-interface.c >> +++ b/tools/virsh-interface.c >> @@ -1040,11 +1040,11 @@ cmdInterfaceUnbridge(vshControl *ctl, const vshCmd *cmd) >> const char *br_name; >> char *if_type = NULL, *if_name = NULL; >> bool nostart = false; >> - char *br_xml = NULL; >> + char *br_xml = NULL, *br_xml_transient_if = NULL; >> xmlChar *if_xml = NULL; >> int if_xml_size; >> - xmlDocPtr xml_doc = NULL; >> - xmlXPathContextPtr ctxt = NULL; >> + xmlDocPtr xml_doc = NULL, xml_doc_transient_if = NULL; >> + xmlXPathContextPtr ctxt = NULL, ctxt_transient_if = NULL; >> xmlNodePtr top_node, if_node, cur; >> >> /* Get a handle to the original device */ >> @@ -1103,6 +1103,22 @@ 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; >> + } >> + >> /* Change the type and name of the outer/master interface to >> * the type/name of the attached slave interface. >> */ >> @@ -1198,10 +1214,13 @@ cmdInterfaceUnbridge(vshControl *ctl, const vshCmd *cmd) >> virInterfaceFree(br_handle); >> VIR_FREE(if_xml); >> VIR_FREE(br_xml); >> + VIR_FREE(br_xml_transient_if); >> VIR_FREE(if_type); >> VIR_FREE(if_name); >> xmlXPathFreeContext(ctxt); >> + xmlXPathFreeContext(ctxt_transient_if); >> xmlFreeDoc(xml_doc); >> + xmlFreeDoc(xml_doc_transient_if); >> return ret; >> } >> >> > ACK. I'll merge this tomorrow (unless somebody beats me). Please don't push it as is. I think the behavior of iface-unbridge should match whatever is done to net-destroy (if anything). If the change is made, it should be made to both at the same time, and this one should also have a --force option to allow overriding the extra check, as patch 2/3 does for net-destroy. (BTW, I'm still curious about Suse's use of netcf, as there have been no updates to the Suse driver in netcf since the initial port was imported several years ago. Are there downstream changes that can be sent upstream?) -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list