Hello All, Any comments for the above patch.
Warm Regards, Nitesh Konkar. On Tue, Mar 8, 2016 at 10:54 AM, Nitesh Konkar < [email protected]> 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. > --- > 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 > >
-- libvir-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
