[libvirt] 答复： Re: [PATCH 3/3] virsh: prevent removing a in-used bridge for iface-unbridge

>>> Laine Stump <laine(a)laine.org&gt; 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 >> --- >> 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.