9:26 a.m.
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.
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
Nitesh Konkar (2):
virsh: Introduce virshDomainDetachInterface function
virsh: Pass the correct live/config xml to virshDomainDetachInterface
tools/virsh-domain.c | 128 +++++++++++++++++++++++++++++----------------------
1 file changed, 73 insertions(+), 55 deletions(-)
--
1.8.3.1
9:26 a.m.
New subject: [libvirt] [PATCH 1/2] virsh: Introduce virshDomainDetachInterface function
virshDomainDetachInterface handles virsh interface
detach from the specified live/config domain xml.
Signed-off-by: Nitesh Konkar <nitkon12(a)linux.vnet.ibm.com>
---
tools/virsh-domain.c | 120 ++++++++++++++++++++++++++++-----------------------
1 file changed, 67 insertions(+), 53 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 0a6caae..1b4e9f0 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -11199,57 +11199,21 @@ static const vshCmdOptDef opts_detach_interface[] = {
};
static bool
-cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
+virshDomainDetachInterface(char *doc,
+ unsigned int flags,
+ virDomainPtr dom,
+ vshControl *ctl,
+ bool current,
+ const char *type,
+ const char *mac)
{
- 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 buf[64];
- int diff_mac;
+ char *detach_xml = NULL, buf[64];
+ int diff_mac, ret = -1;
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;
-
- 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;
if (!(xml = virXMLParseStringCtxt(doc, _("(domain_definition)"),
&ctxt))) {
vshError(ctl, "%s", _("Failed to get interface
information"));
@@ -11315,21 +11279,71 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
else
ret = virDomainDetachDevice(dom, detach_xml);
- if (ret != 0) {
+ cleanup:
+ VIR_FREE(detach_xml);
+ xmlFreeDoc(xml);
+ xmlXPathFreeObject(obj);
+ xmlXPathFreeContext(ctxt);
+ return ret == 0;
+}
+
+
+static bool
+cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
+{
+ virDomainPtr dom = NULL;
+ char *doc = NULL;
+ const char *mac = NULL, *type = NULL;
+ bool ret = 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 (vshCommandOptStringReq(ctl, cmd, "type", &type) < 0)
+ goto cleanup;
+
+ if (vshCommandOptStringReq(ctl, cmd, "mac", &mac) < 0)
+ goto cleanup;
+
+ if (config || persistent)
+ flags |= VIR_DOMAIN_AFFECT_CONFIG;
+ if (live)
+ flags |= VIR_DOMAIN_AFFECT_LIVE;
+
+ 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 = virDomainGetXMLDesc(dom, VIR_DOMAIN_XML_INACTIVE);
+ else
+ doc = virDomainGetXMLDesc(dom, 0);
+
+ if (!doc)
+ goto cleanup;
+ else
+ ret = virshDomainDetachInterface(doc, flags, dom, ctl, current, type, mac);
+
+ cleanup:
+ if (!ret) {
vshError(ctl, "%s", _("Failed to detach interface"));
} else {
vshPrint(ctl, "%s", _("Interface detached successfully\n"));
- functionReturn = true;
}
- cleanup:
VIR_FREE(doc);
- VIR_FREE(detach_xml);
virDomainFree(dom);
- xmlXPathFreeObject(obj);
- xmlXPathFreeContext(ctxt);
- xmlFreeDoc(xml);
- return functionReturn;
+ return ret;
}
typedef enum {
--
1.8.3.1
10:41 a.m.
New subject: [libvirt] [PATCH 1/2] virsh: Introduce virshDomainDetachInterface function
On 05/04/2016 10:26 AM, Nitesh Konkar wrote:
virshDomainDetachInterface handles virsh interface
detach from the specified live/config domain xml.
Signed-off-by: Nitesh Konkar <nitkon12(a)linux.vnet.ibm.com>
---
tools/virsh-domain.c | 120 ++++++++++++++++++++++++++++-----------------------
1 file changed, 67 insertions(+), 53 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 0a6caae..1b4e9f0 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -11199,57 +11199,21 @@ static const vshCmdOptDef opts_detach_interface[] = {
};
Thanks, I pushed with the following changes:
+
+
+static bool
+cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
+{
+ virDomainPtr dom = NULL;
+ char *doc = NULL;
+ const char *mac = NULL, *type = NULL;
+ bool ret = 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 (vshCommandOptStringReq(ctl, cmd, "type", &type) < 0)
+ goto cleanup;
+
+ if (vshCommandOptStringReq(ctl, cmd, "mac", &mac) < 0)
+ goto cleanup;
+
Moved these two checks below the 'dom' lookup, which is where they were
originally
+ if (config || persistent)
+ flags |= VIR_DOMAIN_AFFECT_CONFIG;
+ if (live)
+ flags |= VIR_DOMAIN_AFFECT_LIVE;
+
+ 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 = virDomainGetXMLDesc(dom, VIR_DOMAIN_XML_INACTIVE);
+ else
+ doc = virDomainGetXMLDesc(dom, 0);
+
+ if (!doc)
+ goto cleanup;
+ else
+ ret = virshDomainDetachInterface(doc, flags, dom, ctl, current, type, mac);
+
Split this long line
Thanks,
Cole
+ cleanup:
+ if (!ret) {
vshError(ctl, "%s", _("Failed to detach interface"));
} else {
vshPrint(ctl, "%s", _("Interface detached
successfully\n"));
- functionReturn = true;
}
- cleanup:
VIR_FREE(doc);
- VIR_FREE(detach_xml);
virDomainFree(dom);
- xmlXPathFreeObject(obj);
- xmlXPathFreeContext(ctxt);
- xmlFreeDoc(xml);
- return functionReturn;
+ return ret;
}
typedef enum {
9:26 a.m.
New subject: [libvirt] [PATCH 2/2] virsh: Pass the correct live/config xml to virshDomainDetachInterface.
cmdDetachInterface function checks for live config
flags and then passes the live/config domain xml
to virshDomainDetachInterface accordingly.
Signed-off-by: Nitesh Konkar <nitkon12(a)linux.vnet.ibm.com>
---
tools/virsh-domain.c | 46 +++++++++++++++++++++++++---------------------
1 file changed, 25 insertions(+), 21 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 1b4e9f0..ff467df 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -11292,10 +11292,10 @@ static bool
cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
{
virDomainPtr dom = NULL;
- char *doc = NULL;
+ char *doc_live = NULL, *doc_config = NULL;
const char *mac = NULL, *type = NULL;
- bool ret = false;
- unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
+ int flags = 0;
+ bool ret = false, affect_config, affect_live;
bool current = vshCommandOptBool(cmd, "current");
bool config = vshCommandOptBool(cmd, "config");
bool live = vshCommandOptBool(cmd, "live");
@@ -11312,27 +11312,31 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptStringReq(ctl, cmd, "mac", &mac) < 0)
goto cleanup;
- if (config || persistent)
- flags |= VIR_DOMAIN_AFFECT_CONFIG;
- if (live)
- flags |= VIR_DOMAIN_AFFECT_LIVE;
-
if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
return false;
- if (persistent &&
- virDomainIsActive(dom) == 1)
- flags |= VIR_DOMAIN_AFFECT_LIVE;
+ affect_config = (config || persistent);
- if (flags & VIR_DOMAIN_AFFECT_CONFIG)
- doc = virDomainGetXMLDesc(dom, VIR_DOMAIN_XML_INACTIVE);
- else
- doc = virDomainGetXMLDesc(dom, 0);
+ if (affect_config) {
+ if (!(doc_config = virDomainGetXMLDesc(dom, VIR_DOMAIN_XML_INACTIVE)))
+ goto cleanup;
+ if (!(ret = virshDomainDetachInterface(doc_config, flags |
VIR_DOMAIN_AFFECT_CONFIG, dom, ctl, current, type, mac)))
+ goto cleanup;
+ }
- if (!doc)
- goto cleanup;
- else
- ret = virshDomainDetachInterface(doc, flags, dom, ctl, current, type, mac);
+ affect_live = (live || (persistent && virDomainIsActive(dom) == 1));
+
+ if (affect_live || !affect_config) {
+ flags = 0;
+
+ if (affect_live)
+ flags |= VIR_DOMAIN_AFFECT_LIVE;
+
+ if (!(doc_live = virDomainGetXMLDesc(dom, 0)))
+ goto cleanup;
+
+ ret = virshDomainDetachInterface(doc_live, flags, dom, ctl, current, type, mac);
+ }
cleanup:
if (!ret) {
@@ -11340,8 +11344,8 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
} else {
vshPrint(ctl, "%s", _("Interface detached successfully\n"));
}
-
- VIR_FREE(doc);
+ VIR_FREE(doc_live);
+ VIR_FREE(doc_config);
virDomainFree(dom);
return ret;
}
--
1.8.3.1
10:41 a.m.
New subject: [libvirt] [PATCH 2/2] virsh: Pass the correct live/config xml to virshDomainDetachInterface.
On 05/04/2016 10:26 AM, Nitesh Konkar wrote:
cmdDetachInterface function checks for live config
flags and then passes the live/config domain xml
to virshDomainDetachInterface accordingly.
Signed-off-by: Nitesh Konkar <nitkon12(a)linux.vnet.ibm.com>
---
tools/virsh-domain.c | 46 +++++++++++++++++++++++++---------------------
1 file changed, 25 insertions(+), 21 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 1b4e9f0..ff467df 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -11292,10 +11292,10 @@ static bool
cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
{
virDomainPtr dom = NULL;
- char *doc = NULL;
+ char *doc_live = NULL, *doc_config = NULL;
const char *mac = NULL, *type = NULL;
- bool ret = false;
- unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
+ int flags = 0;
+ bool ret = false, affect_config, affect_live;
bool current = vshCommandOptBool(cmd, "current");
bool config = vshCommandOptBool(cmd, "config");
bool live = vshCommandOptBool(cmd, "live");
@@ -11312,27 +11312,31 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptStringReq(ctl, cmd, "mac", &mac) < 0)
goto cleanup;
- if (config || persistent)
- flags |= VIR_DOMAIN_AFFECT_CONFIG;
- if (live)
- flags |= VIR_DOMAIN_AFFECT_LIVE;
-
if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
return false;
- if (persistent &&
- virDomainIsActive(dom) == 1)
- flags |= VIR_DOMAIN_AFFECT_LIVE;
+ affect_config = (config || persistent);
- if (flags & VIR_DOMAIN_AFFECT_CONFIG)
- doc = virDomainGetXMLDesc(dom, VIR_DOMAIN_XML_INACTIVE);
- else
- doc = virDomainGetXMLDesc(dom, 0);
+ if (affect_config) {
+ if (!(doc_config = virDomainGetXMLDesc(dom, VIR_DOMAIN_XML_INACTIVE)))
+ goto cleanup;
+ if (!(ret = virshDomainDetachInterface(doc_config, flags |
VIR_DOMAIN_AFFECT_CONFIG, dom, ctl, current, type, mac)))
+ goto cleanup;
+ }
- if (!doc)
- goto cleanup;
- else
- ret = virshDomainDetachInterface(doc, flags, dom, ctl, current, type, mac);
+ affect_live = (live || (persistent && virDomainIsActive(dom) == 1));
+
+ if (affect_live || !affect_config) {
+ flags = 0;
+
+ if (affect_live)
+ flags |= VIR_DOMAIN_AFFECT_LIVE;
+
+ if (!(doc_live = virDomainGetXMLDesc(dom, 0)))
+ goto cleanup;
+
+ ret = virshDomainDetachInterface(doc_live, flags, dom, ctl, current, type,
mac);
+ }
I pushed with the above long lines split
Thanks,
Cole
cleanup:
if (!ret) {
@@ -11340,8 +11344,8 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
} else {
vshPrint(ctl, "%s", _("Interface detached
successfully\n"));
}
-
- VIR_FREE(doc);
+ VIR_FREE(doc_live);
+ VIR_FREE(doc_config);
virDomainFree(dom);
return ret;
}
10:43 a.m.
On 05/04/2016 10:26 AM, Nitesh Konkar wrote:
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.
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
Nitesh Konkar (2):
virsh: Introduce virshDomainDetachInterface function
virsh: Pass the correct live/config xml to virshDomainDetachInterface
tools/virsh-domain.c | 128 +++++++++++++++++++++++++++++----------------------
1 file changed, 73 insertions(+), 55 deletions(-)
Should have mentioned, the subject should have contained [PATCH v3] or
whatever version we were on. You can do 'git format-patch -v3' for that, or
send-email -v3
Similarly, if you are going through multiple versions, list in the cover
letter what changed between versions. Like
v3:
Fixed indentation
Fix memory leak
or similar
Thanks,
Cole
3124
days inactive
3124
days old
5 comments
2 participants
participants (2)
-
Cole Robinson -
Nitesh Konkar