<div dir="ltr"><div><div><div>Hello All,<br><br></div>Any comments for the above patch.<br><br></div>Warm Regards,<br></div>Nitesh Konkar.<br><br></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Mar 8, 2016 at 10:54 AM, Nitesh Konkar <span dir="ltr"><<a href="mailto:niteshkonkar.libvirt@gmail.com" target="_blank">niteshkonkar.libvirt@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">The virsh attach virsh detach interface command fails when both live and config<br>
are set and when the interface gets attached to different pci slots<br>
on live and config xml respectively.<br>
<br>
When we attach an interface with both --live and --config,<br>
the first time they get the same PCI slots, but the second time<br>
onwards it differs and hence the virsh detach-interface --live<br>
--config command fails. This patch makes sure that when both<br>
--live --config are set , qemuDomainDetachDeviceFlags is called<br>
twice, once with config xml and once with live xml.<br>
---<br>
Change log:<br>
v4:<br>
* Code unchanged, updated with commit message,change log<br>
and steps to reproduce the issue..<br>
<br>
v3:<br>
* Created function vshDomainDetachInterface and called<br>
it once/twice from cmdDetachInterface depending on<br>
number of flags set (live/config/both). Passed the<br>
correct xml(live/persistent) to it.<br>
<br>
v2:<br>
* Changes only in cmdDetachInterface to pass the right domain xml<br>
depending on the flag set (live/config/both).<br>
<br>
v1:<br>
* Changes only in qemuDomainDetachDeviceFlags to set the right value<br>
in dev and dev_copy.<br>
<br>
Steps to see the issue:<br>
virsh attach-interface --domain DomainName --type network --source default --mac 52:54:00:4b:76:5f --live --config<br>
virsh detach-interface --domain DomainName --type network --mac 52:54:00:4b:76:5f --live --config<br>
virsh attach-interface --domain DomainName --type network --source default --mac 52:54:00:4b:76:5f --live --config<br>
virsh detach-interface --domain DomainName --type network --mac 52:54:00:4b:76:5f --live --config<br>
<br>
tools/virsh-domain.c | 104 +++++++++++++++++++++++++++++++--------------------<br>
1 file changed, 64 insertions(+), 40 deletions(-)<br>
<br>
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c<br>
index 62acecb..a6abaf5 100644<br>
--- a/tools/virsh-domain.c<br>
+++ b/tools/virsh-domain.c<br>
@@ -10801,40 +10801,21 @@ static const vshCmdOptDef opts_detach_interface[] = {<br>
{.name = NULL}<br>
};<br>
<br>
-static bool<br>
-cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)<br>
+static bool<br>
+vshDomainDetachInterface(char *doc, unsigned int flags, virDomainPtr dom, vshControl *ctl, const vshCmd *cmd)<br>
{<br>
- virDomainPtr dom = NULL;<br>
xmlDocPtr xml = NULL;<br>
xmlXPathObjectPtr obj = NULL;<br>
xmlXPathContextPtr ctxt = NULL;<br>
xmlNodePtr cur = NULL, matchNode = NULL;<br>
- char *detach_xml = NULL;<br>
const char *mac = NULL, *type = NULL;<br>
- char *doc = NULL;<br>
+ char *detach_xml = NULL;<br>
+ bool current = vshCommandOptBool(cmd, "current");<br>
char buf[64];<br>
int diff_mac;<br>
size_t i;<br>
int ret;<br>
bool functionReturn = false;<br>
- unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;<br>
- bool current = vshCommandOptBool(cmd, "current");<br>
- bool config = vshCommandOptBool(cmd, "config");<br>
- bool live = vshCommandOptBool(cmd, "live");<br>
- bool persistent = vshCommandOptBool(cmd, "persistent");<br>
-<br>
- VSH_EXCLUSIVE_OPTIONS_VAR(persistent, current);<br>
-<br>
- VSH_EXCLUSIVE_OPTIONS_VAR(current, live);<br>
- VSH_EXCLUSIVE_OPTIONS_VAR(current, config);<br>
-<br>
- if (config || persistent)<br>
- flags |= VIR_DOMAIN_AFFECT_CONFIG;<br>
- if (live)<br>
- flags |= VIR_DOMAIN_AFFECT_LIVE;<br>
-<br>
- if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))<br>
- return false;<br>
<br>
if (vshCommandOptStringReq(ctl, cmd, "type", &type) < 0)<br>
goto cleanup;<br>
@@ -10842,15 +10823,6 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)<br>
if (vshCommandOptStringReq(ctl, cmd, "mac", &mac) < 0)<br>
goto cleanup;<br>
<br>
- if (persistent &&<br>
- virDomainIsActive(dom) == 1)<br>
- flags |= VIR_DOMAIN_AFFECT_LIVE;<br>
-<br>
- if (flags & VIR_DOMAIN_AFFECT_CONFIG)<br>
- doc = virDomainGetXMLDesc(dom, VIR_DOMAIN_XML_INACTIVE);<br>
- else<br>
- doc = virDomainGetXMLDesc(dom, 0);<br>
-<br>
if (!doc)<br>
goto cleanup;<br>
<br>
@@ -10918,23 +10890,75 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)<br>
else<br>
ret = virDomainDetachDevice(dom, detach_xml);<br>
<br>
- if (ret != 0) {<br>
- vshError(ctl, "%s", _("Failed to detach interface"));<br>
- } else {<br>
- vshPrint(ctl, "%s", _("Interface detached successfully\n"));<br>
+ if (ret == 0)<br>
functionReturn = true;<br>
- }<br>
<br>
cleanup:<br>
- VIR_FREE(doc);<br>
VIR_FREE(detach_xml);<br>
- virDomainFree(dom);<br>
+ xmlFreeDoc(xml);<br>
xmlXPathFreeObject(obj);<br>
xmlXPathFreeContext(ctxt);<br>
- xmlFreeDoc(xml);<br>
return functionReturn;<br>
}<br>
<br>
+static bool<br>
+cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)<br>
+{<br>
+ virDomainPtr dom = NULL;<br>
+ unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;<br>
+ char *doc_live = NULL, *doc_config = NULL;<br>
+ bool current = vshCommandOptBool(cmd, "current");<br>
+ bool config = vshCommandOptBool(cmd, "config");<br>
+ bool live = vshCommandOptBool(cmd, "live");<br>
+ bool persistent = vshCommandOptBool(cmd, "persistent");<br>
+ bool functionReturn = false;<br>
+<br>
+ VSH_EXCLUSIVE_OPTIONS_VAR(persistent, current);<br>
+<br>
+ VSH_EXCLUSIVE_OPTIONS_VAR(current, live);<br>
+ VSH_EXCLUSIVE_OPTIONS_VAR(current, config);<br>
+<br>
+ if (config || persistent)<br>
+ flags |= VIR_DOMAIN_AFFECT_CONFIG;<br>
+<br>
+ if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))<br>
+ return false;<br>
+<br>
+ if (persistent &&<br>
+ virDomainIsActive(dom) == 1)<br>
+ flags |= VIR_DOMAIN_AFFECT_LIVE;<br>
+<br>
+ if (flags & VIR_DOMAIN_AFFECT_CONFIG) {<br>
+ doc_config = virDomainGetXMLDesc(dom, VIR_DOMAIN_XML_INACTIVE);<br>
+ if (!(functionReturn = vshDomainDetachInterface(doc_config, flags, dom, ctl, cmd)))<br>
+ goto cleanup;<br>
+ }<br>
+<br>
+ if (live)<br>
+ flags |= VIR_DOMAIN_AFFECT_LIVE;<br>
+<br>
+ if (flags & VIR_DOMAIN_AFFECT_LIVE) {<br>
+ doc_live = virDomainGetXMLDesc(dom, 0);<br>
+<br>
+ if (flags & VIR_DOMAIN_AFFECT_CONFIG)<br>
+ flags &= (~VIR_DOMAIN_AFFECT_CONFIG);<br>
+<br>
+ functionReturn = vshDomainDetachInterface(doc_live, flags, dom, ctl, cmd);<br>
+ }<br>
+<br>
+ cleanup:<br>
+ if (functionReturn == false) {<br>
+ vshError(ctl, "%s", _("Failed to detach interface"));<br>
+ } else {<br>
+ vshPrint(ctl, "%s", _("Interface detached successfully\n"));<br>
+ functionReturn = true;<br>
+ }<br>
+ VIR_FREE(doc_live);<br>
+ VIR_FREE(doc_config);<br>
+ virDomainFree(dom);<br>
+ return functionReturn;<br>
+}<br>
+<br>
typedef enum {<br>
VIRSH_FIND_DISK_NORMAL,<br>
VIRSH_FIND_DISK_CHANGEABLE,<br>
<span class="HOEnZb"><font color="#888888">--<br>
1.8.3.1<br>
<br>
</font></span></blockquote></div><br></div>