8:59 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 corect live/config xml to virshDomainDetachInterface.
tools/virsh-domain.c | 114 +++++++++++++++++++++++++++++++--------------------
1 file changed, 69 insertions(+), 45 deletions(-)
--
1.8.3.1
8:59 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 | 104 ++++++++++++++++++++++++++++++---------------------
1 file changed, 61 insertions(+), 43 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 0a6caae..d9fde4d 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -11199,39 +11199,22 @@ static const vshCmdOptDef opts_detach_interface[] = {
};
static bool
-cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
+virshDomainDetachInterface(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 buf[64];
- int diff_mac;
+ char *detach_xml = NULL, buf[64];
+ bool current = vshCommandOptBool(cmd, "current");
+ int diff_mac, ret;
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;
@@ -11239,18 +11222,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;
-
if (!(xml = virXMLParseStringCtxt(doc, _("(domain_definition)"),
&ctxt))) {
vshError(ctl, "%s", _("Failed to get interface
information"));
goto cleanup;
@@ -11315,20 +11286,67 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
else
ret = virDomainDetachDevice(dom, detach_xml);
- if (ret != 0) {
+ if (ret == 0)
+ functionReturn = true;
+
+ cleanup:
+ VIR_FREE(detach_xml);
+ xmlFreeDoc(xml);
+ xmlXPathFreeObject(obj);
+ xmlXPathFreeContext(ctxt);
+ return functionReturn;
+}
+
+
+static bool
+cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
+{
+ virDomainPtr dom = NULL;
+ char *doc = NULL;
+ 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 (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
+ functionReturn = virshDomainDetachInterface(doc, 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;
}
- cleanup:
VIR_FREE(doc);
- VIR_FREE(detach_xml);
virDomainFree(dom);
- xmlXPathFreeObject(obj);
- xmlXPathFreeContext(ctxt);
- xmlFreeDoc(xml);
return functionReturn;
}
--
1.8.3.1
6:31 p.m.
New subject: [libvirt] [PATCH 1/2] virsh: Introduce virshDomainDetachInterface function
On 05/02/2016 09:59 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 | 104 ++++++++++++++++++++++++++++++---------------------
1 file changed, 61 insertions(+), 43 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 0a6caae..d9fde4d 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -11199,39 +11199,22 @@ static const vshCmdOptDef opts_detach_interface[] = {
};
static bool
-cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
+virshDomainDetachInterface(char *doc,
+ unsigned int flags,
+ virDomainPtr dom,
+ vshControl *ctl,
+ const vshCmd *cmd)
{
The split is a bit wrong IMO. I don't think virshDomainDetachInterface should
be doing any command line processing. I think the args should be:
virshDomainDetachInterface(vshControl *ctl,
virDomainPtr dom,
unsigned int flags,
bool current,
const char *type,
const char *mac);
One other comment below:
- 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];
+ bool current = vshCommandOptBool(cmd, "current");
+ int diff_mac, ret;
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;
@@ -11239,18 +11222,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;
-
if (!(xml = virXMLParseStringCtxt(doc, _("(domain_definition)"),
&ctxt))) {
vshError(ctl, "%s", _("Failed to get interface
information"));
goto cleanup;
@@ -11315,20 +11286,67 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
else
ret = virDomainDetachDevice(dom, detach_xml);
- if (ret != 0) {
+ if (ret == 0)
+ functionReturn = true;
+
+ cleanup:
+ VIR_FREE(detach_xml);
+ xmlFreeDoc(xml);
+ xmlXPathFreeObject(obj);
+ xmlXPathFreeContext(ctxt);
+ return functionReturn;
+}
+
+
+static bool
+cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
+{
+ virDomainPtr dom = NULL;
+ char *doc = NULL;
+ 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 (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
+ functionReturn = virshDomainDetachInterface(doc, 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;
}
- cleanup:
VIR_FREE(doc);
- VIR_FREE(detach_xml);
virDomainFree(dom);
- xmlXPathFreeObject(obj);
- xmlXPathFreeContext(ctxt);
- xmlFreeDoc(xml);
return functionReturn;
Drop functionReturn entirely here. set ret=-1 at init time, then return ret == 0;
Otherwise it looks good.
Thanks,
Cole
8:59 a.m.
New subject: [libvirt] [PATCH 2/2] virsh: Pass the corect 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 | 32 +++++++++++++++++++-------------
1 file changed, 19 insertions(+), 13 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index d9fde4d..5cfd6a3 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -11302,7 +11302,7 @@ static bool
cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
{
virDomainPtr dom = NULL;
- char *doc = NULL;
+ char *doc_live = NULL, *doc_config = NULL;
bool functionReturn = false;
unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
bool current = vshCommandOptBool(cmd, "current");
@@ -11317,8 +11317,6 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
if (config || persistent)
flags |= VIR_DOMAIN_AFFECT_CONFIG;
- if (live)
- flags |= VIR_DOMAIN_AFFECT_LIVE;
if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
return false;
@@ -11327,15 +11325,23 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
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 (flags & VIR_DOMAIN_AFFECT_CONFIG) {
+ if (!(doc_config = virDomainGetXMLDesc(dom, VIR_DOMAIN_XML_INACTIVE)))
+ goto cleanup;
+ if (!(functionReturn = virshDomainDetachInterface(doc_config, flags, dom, ctl,
cmd)))
+ goto cleanup;
+ }
- if (!doc)
- goto cleanup;
- else
- functionReturn = virshDomainDetachInterface(doc, flags, dom, ctl, cmd);
+ flags = 0;
+
+ if (live || (!live && !config))
+ flags |= VIR_DOMAIN_AFFECT_LIVE;
+
+ if (flags & VIR_DOMAIN_AFFECT_LIVE) {
+ if (!(doc_live = virDomainGetXMLDesc(dom, 0)))
+ goto cleanup;
+ functionReturn = virshDomainDetachInterface(doc_live, flags, dom, ctl, cmd);
+ }
cleanup:
if (functionReturn == false) {
@@ -11344,8 +11350,8 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
vshPrint(ctl, "%s", _("Interface detached successfully\n"));
functionReturn = true;
}
-
- VIR_FREE(doc);
+ VIR_FREE(doc_live);
+ VIR_FREE(doc_config);
virDomainFree(dom);
return functionReturn;
}
--
1.8.3.1
7:08 p.m.
New subject: [libvirt] [PATCH 2/2] virsh: Pass the corect live/config xml to virshDomainDetachInterface.
On 05/02/2016 09:59 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 | 32 +++++++++++++++++++-------------
1 file changed, 19 insertions(+), 13 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index d9fde4d..5cfd6a3 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -11302,7 +11302,7 @@ static bool
cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
{
virDomainPtr dom = NULL;
- char *doc = NULL;
+ char *doc_live = NULL, *doc_config = NULL;
bool functionReturn = false;
unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
bool current = vshCommandOptBool(cmd, "current");
@@ -11317,8 +11317,6 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
Hmm, I'm having trouble following the logic here with the flag manipulation. I
think there's an error here with --persistent handling at least. I suggest to
drop the 'flags' variable entirely. So...
if (config || persistent)
flags |= VIR_DOMAIN_AFFECT_CONFIG;
Here I'd change to
bool affect_config;
...
affect_config = (config || persistent);
- if (live)
- flags |= VIR_DOMAIN_AFFECT_LIVE;
if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
return false;
@@ -11327,15 +11325,23 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
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 (flags & VIR_DOMAIN_AFFECT_CONFIG) {
This will just be (affect_config)
+ if (!(doc_config = virDomainGetXMLDesc(dom,
VIR_DOMAIN_XML_INACTIVE)))
+ goto cleanup;
+ if (!(functionReturn = virshDomainDetachInterface(doc_config, flags, dom, ctl,
cmd)))
+ goto cleanup;
+ }
- if (!doc)
- goto cleanup;
- else
- functionReturn = virshDomainDetachInterface(doc, flags, dom, ctl, cmd);
+ flags = 0;
+
+ if (live || (!live && !config))
+ flags |= VIR_DOMAIN_AFFECT_LIVE;
+
+ if (flags & VIR_DOMAIN_AFFECT_LIVE) {
This will be:
bool affect_live;
...
affect_live = (live || (persistent && virDomainIsActive(dom) == 1));
if (affect_live || !affect_config)
int flags = 0;
if (affect_live)
flags |= VIR_DOMAIN_AFFECT_LIVE;
...
The last bit with the 'int flags' ensures we still use flags=0 and old style
virDomainDetachDevice() if the user doesn't specify any command line flags. We
want to preserve that behavior for compat with older libvirt
- Cole
3161
days inactive
3162
days old
4 comments
2 participants
participants (2)
-
Cole Robinson -
Nitesh Konkar