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

[Nitesh Konkar]

Hello All,

Any comments for the above patch.

Warm Regards,
Nitesh Konkar.


On Tue, Mar 8, 2016 at 10:54 AM, Nitesh Konkar <
niteshkonkar.libvirt at gmail.com> 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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20160322/11c2a6a4/attachment-0001.htm>