4:11 a.m.
The virsh attach/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 | 107 +++++++++++++++++++++++++++++++--------------------
1 file changed, 65 insertions(+), 42 deletions(-)
--
1.8.3.1
4:11 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 | 99 ++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 99 insertions(+)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 36d0353..7cb042b 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -11118,6 +11118,105 @@ static const vshCmdOptDef opts_detach_interface[] = {
};
static bool
+virshDomainDetachInterface(char *doc, unsigned int flags, virDomainPtr dom, vshControl
*ctl, const vshCmd *cmd)
+{
+ xmlDocPtr xml = NULL;
+ xmlXPathObjectPtr obj = NULL;
+ xmlXPathContextPtr ctxt = NULL;
+ xmlNodePtr cur = NULL, matchNode = NULL;
+ const char *mac = NULL, *type = NULL;
+ char *detach_xml = NULL, buf[64];
+ bool current = vshCommandOptBool(cmd, "current");
+ int diff_mac,ret;
+ size_t i;
+ bool functionReturn = false;
+
+ if (vshCommandOptStringReq(ctl, cmd, "type", &type) < 0)
+ goto cleanup;
+
+ if (vshCommandOptStringReq(ctl, cmd, "mac", &mac) < 0)
+ goto cleanup;
+
+ if (!doc)
+ goto cleanup;
+
+ if (!(xml = virXMLParseStringCtxt(doc, _("(domain_definition)"),
&ctxt))) {
+ vshError(ctl, "%s", _("Failed to get interface
information"));
+ goto cleanup;
+ }
+
+ snprintf(buf, sizeof(buf), "/domain/devices/interface[@type='%s']",
type);
+ obj = xmlXPathEval(BAD_CAST buf, ctxt);
+ if (obj == NULL || obj->type != XPATH_NODESET ||
+ obj->nodesetval == NULL || obj->nodesetval->nodeNr == 0) {
+ vshError(ctl, _("No interface found whose type is %s"), type);
+ goto cleanup;
+ }
+
+ if (!mac && obj->nodesetval->nodeNr > 1) {
+ vshError(ctl, _("Domain has %d interfaces. Please specify which one "
+ "to detach using --mac"),
obj->nodesetval->nodeNr);
+ goto cleanup;
+ }
+
+ if (!mac) {
+ matchNode = obj->nodesetval->nodeTab[0];
+ goto hit;
+ }
+
+ /* multiple possibilities, so search for matching mac */
+ for (i = 0; i < obj->nodesetval->nodeNr; i++) {
+ cur = obj->nodesetval->nodeTab[i]->children;
+ while (cur != NULL) {
+ if (cur->type == XML_ELEMENT_NODE &&
+ xmlStrEqual(cur->name, BAD_CAST "mac")) {
+ char *tmp_mac = virXMLPropString(cur, "address");
+ diff_mac = virMacAddrCompare(tmp_mac, mac);
+ VIR_FREE(tmp_mac);
+ if (!diff_mac) {
+ if (matchNode) {
+ /* this is the 2nd match, so it's ambiguous */
+ vshError(ctl, _("Domain has multiple interfaces matching
"
+ "MAC address %s. You must use detach-device
and "
+ "specify the device pci address to remove
it."),
+ mac);
+ goto cleanup;
+ }
+ matchNode = obj->nodesetval->nodeTab[i];
+ }
+ }
+ cur = cur->next;
+ }
+ }
+ if (!matchNode) {
+ vshError(ctl, _("No interface with MAC address %s was found"), mac);
+ goto cleanup;
+ }
+
+ hit:
+ if (!(detach_xml = virXMLNodeToString(xml, matchNode))) {
+ vshSaveLibvirtError();
+ goto cleanup;
+ }
+
+ if (flags != 0 || current)
+ ret = virDomainDetachDeviceFlags(dom, detach_xml, flags);
+ else
+ ret = virDomainDetachDevice(dom, detach_xml);
+
+ 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;
--
1.8.3.1
7:56 a.m.
New subject: [libvirt] [PATCH 1/2] virsh: Introduce virshDomainDetachInterface function.
On 04/28/2016 05:11 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 | 99 ++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 99 insertions(+)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 36d0353..7cb042b 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -11118,6 +11118,105 @@ static const vshCmdOptDef opts_detach_interface[] = {
};
I meant that this patch should also be removing the functionality
cmdDetachInterface at the same time. So patch #1: break out the old
functionality to virshDomainDetachInterface, and use it in cmdDetachInterface.
The change should be a functional no-op
Then patch #2 changes the behavior. The point is that you isolate the
functional changes in the smallest patch possible, since that is the harder
stuff to review, but a smaller patch will make it easier to review.
static bool
+ virshDomainDetachInterface(char *doc, unsigned int flags, virDomainPtr dom, vshControl
*ctl, const vshCmd *cmd)
+{
This line is too long. Split it like:
static bool
virshDomainDetachInterface(char *doc,
unsigned int flags,
...
+ xmlDocPtr xml = NULL;
+ xmlXPathObjectPtr obj = NULL;
+ xmlXPathContextPtr ctxt = NULL;
+ xmlNodePtr cur = NULL, matchNode = NULL;
+ const char *mac = NULL, *type = NULL;
+ char *detach_xml = NULL, buf[64];
+ bool current = vshCommandOptBool(cmd, "current");
+ int diff_mac,ret;
The style is wrong here, I see you fixed it in patch 2, but the change should
have been in this patch. Make sure every patch is syntax-check clean
Please fix those issues and repost
Thanks,
Cole
4:11 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 | 114 +++++++++------------------------------------------
1 file changed, 19 insertions(+), 95 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 7cb042b..271e229 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -11127,10 +11127,10 @@ virshDomainDetachInterface(char *doc, unsigned int flags,
virDomainPtr dom, vshC
const char *mac = NULL, *type = NULL;
char *detach_xml = NULL, buf[64];
bool current = vshCommandOptBool(cmd, "current");
- int diff_mac,ret;
+ int diff_mac, ret;
size_t i;
bool functionReturn = false;
-
+
if (vshCommandOptStringReq(ctl, cmd, "type", &type) < 0)
goto cleanup;
@@ -11220,23 +11220,13 @@ static bool
cmdDetachInterface(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;
- 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");
+ char *doc_live = NULL, *doc_config = NULL;
+ bool functionReturn = false;
VSH_EXCLUSIVE_OPTIONS_VAR(persistent, current);
@@ -11245,108 +11235,42 @@ 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;
- 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"));
- goto cleanup;
- }
-
- snprintf(buf, sizeof(buf), "/domain/devices/interface[@type='%s']",
type);
- obj = xmlXPathEval(BAD_CAST buf, ctxt);
- if (obj == NULL || obj->type != XPATH_NODESET ||
- obj->nodesetval == NULL || obj->nodesetval->nodeNr == 0) {
- vshError(ctl, _("No interface found whose type is %s"), type);
- goto cleanup;
+ if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
+ doc_config = virDomainGetXMLDesc(dom, VIR_DOMAIN_XML_INACTIVE);
+ if (!(functionReturn = virshDomainDetachInterface(doc_config, flags, dom, ctl,
cmd)))
+ goto cleanup;
}
- if (!mac && obj->nodesetval->nodeNr > 1) {
- vshError(ctl, _("Domain has %d interfaces. Please specify which one "
- "to detach using --mac"),
obj->nodesetval->nodeNr);
- goto cleanup;
- }
+ if (live || (!live && !config))
+ flags |= VIR_DOMAIN_AFFECT_LIVE;
- if (!mac) {
- matchNode = obj->nodesetval->nodeTab[0];
- goto hit;
- }
+ if (flags & VIR_DOMAIN_AFFECT_LIVE) {
+ doc_live = virDomainGetXMLDesc(dom, 0);
- /* multiple possibilities, so search for matching mac */
- for (i = 0; i < obj->nodesetval->nodeNr; i++) {
- cur = obj->nodesetval->nodeTab[i]->children;
- while (cur != NULL) {
- if (cur->type == XML_ELEMENT_NODE &&
- xmlStrEqual(cur->name, BAD_CAST "mac")) {
- char *tmp_mac = virXMLPropString(cur, "address");
- diff_mac = virMacAddrCompare(tmp_mac, mac);
- VIR_FREE(tmp_mac);
- if (!diff_mac) {
- if (matchNode) {
- /* this is the 2nd match, so it's ambiguous */
- vshError(ctl, _("Domain has multiple interfaces matching
"
- "MAC address %s. You must use detach-device
and "
- "specify the device pci address to remove
it."),
- mac);
- goto cleanup;
- }
- matchNode = obj->nodesetval->nodeTab[i];
- }
- }
- cur = cur->next;
- }
- }
- if (!matchNode) {
- vshError(ctl, _("No interface with MAC address %s was found"), mac);
- goto cleanup;
- }
+ if (flags & VIR_DOMAIN_AFFECT_CONFIG)
+ flags &= (~VIR_DOMAIN_AFFECT_CONFIG);
- hit:
- if (!(detach_xml = virXMLNodeToString(xml, matchNode))) {
- vshSaveLibvirtError();
- goto cleanup;
+ functionReturn = virshDomainDetachInterface(doc_live, flags, dom, ctl, cmd);
}
- if (flags != 0 || current)
- ret = virDomainDetachDeviceFlags(dom, detach_xml, flags);
- else
- ret = virDomainDetachDevice(dom, detach_xml);
-
- if (ret != 0) {
+ 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);
+ VIR_FREE(doc_live);
+ VIR_FREE(doc_config);
virDomainFree(dom);
- xmlXPathFreeObject(obj);
- xmlXPathFreeContext(ctxt);
- xmlFreeDoc(xml);
return functionReturn;
}
--
1.8.3.1
3163
days inactive
3164
days old
3 comments
2 participants
participants (2)
-
Cole Robinson -
Nitesh Konkar