[libvirt] [PATCH v4] Use-correct-pci-addresses-during-interface-detach

7 Apr 2016

The virsh attach virsh detach interface command fails  when both live and config
are set and when the  interface gets attached to different pci slots
on live and config xml respectively.

When we attach an interface with both --live and --config,
the first time they get the same PCI slots, but the second time
onwards it differs and hence the virsh detach-interface --live
--config command fails. This patch makes sure that when both
--live --config are set , qemuDomainDetachDeviceFlags is called
twice, once with config xml and once with live xml.
---
Change log:
v4:
* Code unchanged, updated with commit message,change log 
  and steps to reproduce the issue..

v3:
* Created function vshDomainDetachInterface and called 
  it once/twice from cmdDetachInterface depending on 
  number of flags set (live/config/both). Passed the 
  correct xml(live/persistent) to it. 

v2:
* Changes only in cmdDetachInterface to pass the right domain xml
  depending on the flag set (live/config/both).

v1:
* Changes only in qemuDomainDetachDeviceFlags to set the right value 
  in dev and dev_copy.

Steps to see the issue:
virsh attach-interface --domain DomainName --type network --source default --mac 52:54:00:4b:76:5f --live --config
virsh detach-interface --domain DomainName --type network --mac 52:54:00:4b:76:5f --live --config
virsh attach-interface --domain DomainName --type network --source default --mac 52:54:00:4b:76:5f --live --config
virsh detach-interface --domain DomainName --type network --mac 52:54:00:4b:76:5f --live --config

 tools/virsh-domain.c | 104 +++++++++++++++++++++++++++++++--------------------
 1 file changed, 64 insertions(+), 40 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 62acecb..a6abaf5 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -10801,40 +10801,21 @@ static const vshCmdOptDef opts_detach_interface[] = {
     {.name = NULL}
 };
 
-static bool
-cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
+static bool 
+vshDomainDetachInterface(char *doc, unsigned int flags, virDomainPtr dom, vshControl *ctl, const vshCmd *cmd)
 {
-    virDomainPtr dom = NULL;
     xmlDocPtr xml = NULL;
     xmlXPathObjectPtr obj = NULL;
     xmlXPathContextPtr ctxt = NULL;
     xmlNodePtr cur = NULL, matchNode = NULL;
-    char *detach_xml = NULL;
     const char *mac = NULL, *type = NULL;
-    char *doc = NULL;
+    char *detach_xml = NULL;
+    bool current = vshCommandOptBool(cmd, "current");
     char buf[64];
     int diff_mac;
     size_t i;
     int ret;
     bool functionReturn = false;
-    unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
-    bool current = vshCommandOptBool(cmd, "current");
-    bool config = vshCommandOptBool(cmd, "config");
-    bool live = vshCommandOptBool(cmd, "live");
-    bool persistent = vshCommandOptBool(cmd, "persistent");
-
-    VSH_EXCLUSIVE_OPTIONS_VAR(persistent, current);
-
-    VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
-    VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
-
-    if (config || persistent)
-        flags |= VIR_DOMAIN_AFFECT_CONFIG;
-    if (live)
-        flags |= VIR_DOMAIN_AFFECT_LIVE;
-
-    if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
-        return false;
 
     if (vshCommandOptStringReq(ctl, cmd, "type", &type) < 0)
         goto cleanup;
@@ -10842,15 +10823,6 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
     if (vshCommandOptStringReq(ctl, cmd, "mac", &mac) < 0)
         goto cleanup;
 
-    if (persistent &&
-        virDomainIsActive(dom) == 1)
-        flags |= VIR_DOMAIN_AFFECT_LIVE;
-
-    if (flags & VIR_DOMAIN_AFFECT_CONFIG)
-        doc = virDomainGetXMLDesc(dom, VIR_DOMAIN_XML_INACTIVE);
-    else
-        doc = virDomainGetXMLDesc(dom, 0);
-
     if (!doc)
         goto cleanup;
 
@@ -10918,23 +10890,75 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
     else
         ret = virDomainDetachDevice(dom, detach_xml);
 
-    if (ret != 0) {
-        vshError(ctl, "%s", _("Failed to detach interface"));
-    } else {
-        vshPrint(ctl, "%s", _("Interface detached successfully\n"));
+    if (ret == 0) 
         functionReturn = true;
-    }
 
  cleanup:
-    VIR_FREE(doc);
     VIR_FREE(detach_xml);
-    virDomainFree(dom);
+    xmlFreeDoc(xml);
     xmlXPathFreeObject(obj);
     xmlXPathFreeContext(ctxt);
-    xmlFreeDoc(xml);
     return functionReturn;
 }
 
+static bool
+cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
+{
+    virDomainPtr dom = NULL;
+    unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
+    char *doc_live = NULL, *doc_config = NULL;
+    bool current = vshCommandOptBool(cmd, "current");
+    bool config = vshCommandOptBool(cmd, "config");
+    bool live = vshCommandOptBool(cmd, "live");
+    bool persistent = vshCommandOptBool(cmd, "persistent");
+    bool functionReturn = false;
+
+    VSH_EXCLUSIVE_OPTIONS_VAR(persistent, current);
+
+    VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
+    VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
+
+    if (config || persistent)
+        flags |= VIR_DOMAIN_AFFECT_CONFIG;
+
+    if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
+        return false;
+    
+    if (persistent &&
+        virDomainIsActive(dom) == 1)
+        flags |= VIR_DOMAIN_AFFECT_LIVE;
+
+    if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
+         doc_config = virDomainGetXMLDesc(dom, VIR_DOMAIN_XML_INACTIVE);
+         if (!(functionReturn = vshDomainDetachInterface(doc_config, flags, dom, ctl, cmd)))
+            goto cleanup;
+    } 
+
+    if (live)
+        flags |= VIR_DOMAIN_AFFECT_LIVE;
+
+    if (flags & VIR_DOMAIN_AFFECT_LIVE) {
+        doc_live = virDomainGetXMLDesc(dom, 0);
+
+       if (flags & VIR_DOMAIN_AFFECT_CONFIG)
+          flags &= (~VIR_DOMAIN_AFFECT_CONFIG); 
+        
+       functionReturn = vshDomainDetachInterface(doc_live, flags, dom, ctl, cmd);
+    }
+
+ cleanup:
+    if (functionReturn == false) {
+        vshError(ctl, "%s", _("Failed to detach interface"));
+    } else {
+        vshPrint(ctl, "%s", _("Interface detached successfully\n"));
+        functionReturn = true;
+    }
+   VIR_FREE(doc_live); 
+   VIR_FREE(doc_config);
+   virDomainFree(dom);
+   return functionReturn;
+}
+
 typedef enum {
     VIRSH_FIND_DISK_NORMAL,
     VIRSH_FIND_DISK_CHANGEABLE,
-- 
1.8.3.1

    

Nitesh Konkar

Cole Robinson

tags

participants (2)