[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v3] virsh: Increase device-detach intelligence



On 24.08.2011 14:53, Michal Privoznik wrote:
> From: Michal Prívozník <mprivozn redhat com>
> 
> Up to now users have to give a full XML description on input when
> device-detaching. If they omitted something it lead to unclear
> error messages (like generated MAC wasn't found, etc.).
> With this patch users can specify only those information which
> specify one device sufficiently precise. Remaining information is
> completed from domain.
> ---
> diff to v2:
> -rebase to current HEAD
> 
> diff to v1:
> -rebase to current HEAD
> -add a little bit comments
> 
>  tools/virsh.c |  266 +++++++++++++++++++++++++++++++++++++++++++++++++++++----
>  1 files changed, 250 insertions(+), 16 deletions(-)
> 
> diff --git a/tools/virsh.c b/tools/virsh.c
> index 1ad84a2..aae8e4e 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -10351,6 +10351,226 @@ cmdAttachDevice(vshControl *ctl, const vshCmd *cmd)
>      return true;
>  }
>  
> +/**
> + * Check if n1 is superset of n2, meaning n1 contains all elements and
> + * attributes as n2 at lest. Including children.
> + * @n1 first node
> + * @n2 second node
> + * return 1 in case n1 covers n2, 0 otherwise.
> + */
> +static int
> +vshNodeIsSuperset(xmlNodePtr n1, xmlNodePtr n2) {
> +    xmlNodePtr child1, child2;
> +    xmlAttrPtr attr1, attr2;
> +    int found;
> +
> +    if (!n1 && !n2)
> +        return 1;
> +
> +    if (!n1 || !n2)
> +        return 0;
> +
> +    if (!xmlStrEqual(n1->name, n2->name))
> +        return 0;
> +
> +    /* Iterate over n2 attributes and check if n1 contains them*/
> +    attr2 = n2->properties;
> +    while (attr2) {
> +        if (attr2->type == XML_ATTRIBUTE_NODE) {
> +            attr1 = n1->properties;
> +            found = 0;
> +            while (attr1) {
> +                if (xmlStrEqual(attr1->name, attr2->name)) {
> +                    found = 1;
> +                    break;
> +                }
> +                attr1 = attr1->next;
> +            }
> +            if (!found)
> +                return 0;
> +            if (!xmlStrEqual(BAD_CAST virXMLPropString(n1, (const char *) attr1->name),
> +                             BAD_CAST virXMLPropString(n2, (const char *) attr2->name)))
> +                return 0;
> +        }
> +        attr2 = attr2->next;
> +    }
> +
> +    /* and now iterate over n2 children */
> +    child2 = n2->children;
> +    while (child2) {
> +        if (child2->type == XML_ELEMENT_NODE) {
> +            child1 = n1->children;
> +            found = 0;
> +            while (child1) {
> +                if (child1->type == XML_ELEMENT_NODE &&
> +                    xmlStrEqual(child1->name, child2->name)) {
> +                    found = 1;
> +                    break;
> +                }
> +                child1 = child1->next;
> +            }
> +            if (!found)
> +                return 0;
> +            if (!vshNodeIsSuperset(child1, child2))
> +                return 0;
> +        }
> +        child2 = child2->next;
> +    }
> +
> +    return 1;
> +}
> +
> +/**
> + * To given domain and (probably incomplete) device XML specification try to
> + * find such device in domain and complete missing parts. This is however
> + * possible when given device XML is sufficiently precise so it addresses only
> + * one device.
> + * @ctl vshControl for error messages printing
> + * @dom domain
> + * @oldXML device XML before
> + * @newXML and after completion
> + * Returns -2 when no such device exists in domain, -3 when given XML selects many
> + *          (is too ambiguous), 0 in case of success. Otherwise returns -1. @newXML
> + *          is touched only in case of success.
> + */
> +static int
> +vshCompleteXMLFromDomain(vshControl *ctl, virDomainPtr dom, char *oldXML,
> +                         char **newXML) {
> +    int funcRet = -1;
> +    char *domXML = NULL;
> +    xmlDocPtr domDoc = NULL, devDoc = NULL;
> +    xmlNodePtr node = NULL;
> +    xmlXPathContextPtr domCtxt = NULL, devCtxt = NULL;
> +    xmlNodePtr *devices = NULL;
> +    xmlSaveCtxtPtr sctxt = NULL;
> +    int devices_size;
> +    char *xpath;
> +    xmlBufferPtr buf = NULL;
> +
> +    if (!(domXML = virDomainGetXMLDesc(dom, 0))) {
> +        vshError(ctl, _("couldn't get XML description of domain %s"),
> +                 virDomainGetName(dom));
> +        goto cleanup;
> +    }
> +
> +    if (!(domDoc = xmlReadDoc(BAD_CAST domXML, "domain.xml", NULL,
> +                              XML_PARSE_NOENT | XML_PARSE_NONET |
> +                              XML_PARSE_NOERROR | XML_PARSE_NOWARNING))) {
> +        vshError(ctl, "%s", _("could not parse domain XML"));
> +        goto cleanup;
> +    }
> +
> +    if (!(devDoc = xmlReadDoc(BAD_CAST oldXML, "device.xml", NULL,
> +                              XML_PARSE_NOENT | XML_PARSE_NONET |
> +                              XML_PARSE_NOERROR | XML_PARSE_NOWARNING))) {
> +        vshError(ctl, "%s", _("could not parse device XML"));
> +        goto cleanup;
> +    }
> +
> +    node = xmlDocGetRootElement(domDoc);
> +    if (!node) {
> +        vshError(ctl, "%s", _("failed to get domain root element"));
> +        goto cleanup;
> +    }
> +
> +    domCtxt = xmlXPathNewContext(domDoc);
> +    if (!domCtxt) {
> +        vshError(ctl, "%s", _("failed to create context on domain XML"));
> +        goto cleanup;
> +    }
> +    domCtxt->node = node;
> +
> +    node = xmlDocGetRootElement(devDoc);
> +    if (!node) {
> +        vshError(ctl, "%s", _("failed to get device root element"));
> +        goto cleanup;
> +    }
> +
> +    devCtxt = xmlXPathNewContext(devDoc);
> +    if (!devCtxt) {
> +        vshError(ctl, "%s", _("failed to create context on device XML"));
> +        goto cleanup;
> +    }
> +    devCtxt->node = node;
> +
> +    buf = xmlBufferCreate();
> +    if (!buf) {
> +        vshError(ctl, "%s", _("out of memory"));
> +        goto cleanup;
> +    }
> +
> +    xmlBufferCat(buf, BAD_CAST "/domain/devices/");
> +    xmlBufferCat(buf, node->name);
> +    xpath = (char *) xmlBufferContent(buf);
> +    /* Get all possible devices */
> +    devices_size = virXPathNodeSet(xpath, domCtxt, &devices);
> +    xmlBufferEmpty(buf);
> +
> +    if (devices_size < 0) {
> +        /* error */
> +        vshError(ctl, "%s", _("error when selecting nodes"));
> +        goto cleanup;
> +    } else if (devices_size == 0) {
> +        /* no such device */
> +        funcRet = -2;
> +        goto cleanup;
> +    }
> +
> +    /* and refine */
> +    int i = 0;
> +    while (i < devices_size) {
> +        if (!vshNodeIsSuperset(devices[i], node)) {
> +            if (devices_size == 1) {
> +                VIR_FREE(devices);
> +                devices_size = 0;
> +            } else {
> +                memmove(devices + i, devices + i + 1,
> +                        sizeof(*devices) * (devices_size-i-1));
> +                devices_size--;
> +                if (VIR_REALLOC_N(devices, devices_size) < 0) {
> +                    /* ignore, harmless */
> +                }
> +            }
> +        } else {
> +            i++;
> +        }
> +    }
> +
> +    if (!devices_size) {
> +        /* no such device */
> +        funcRet = -2;
> +        goto cleanup;
> +    } else if (devices_size > 1) {
> +        /* ambiguous */
> +        funcRet = -3;
> +        goto cleanup;
> +    }
> +
> +    if (newXML) {
> +        sctxt = xmlSaveToBuffer(buf, NULL, 0);
> +        if (!sctxt) {
> +            vshError(ctl, "%s", _("failed to create document saving context"));
> +            goto cleanup;
> +        }
> +
> +        xmlSaveTree(sctxt, devices[0]);
> +        xmlSaveClose(sctxt);
> +        *newXML = (char *) xmlBufferContent(buf);
> +        buf->content = NULL;
> +    }
> +
> +    funcRet = 0;
> +
> +cleanup:
> +    xmlBufferFree(buf);
> +    VIR_FREE(devices);
> +    xmlXPathFreeContext(devCtxt);
> +    xmlXPathFreeContext(domCtxt);
> +    xmlFreeDoc(devDoc);
> +    xmlFreeDoc(domDoc);
> +    VIR_FREE(domXML);
> +    return funcRet;
> +}
>  
>  /*
>   * "detach-device" command
> @@ -10371,10 +10591,11 @@ static const vshCmdOptDef opts_detach_device[] = {
>  static bool
>  cmdDetachDevice(vshControl *ctl, const vshCmd *cmd)
>  {
> -    virDomainPtr dom;
> +    virDomainPtr dom = NULL;
>      const char *from = NULL;
> -    char *buffer;
> +    char *buffer = NULL, *new_buffer = NULL;
>      int ret;
> +    bool funcRet = false;
>      unsigned int flags;
>  
>      if (!vshConnectionUsability(ctl, ctl->conn))
> @@ -10383,37 +10604,50 @@ cmdDetachDevice(vshControl *ctl, const vshCmd *cmd)
>      if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
>          return false;
>  
> -    if (vshCommandOptString(cmd, "file", &from) <= 0) {
> -        virDomainFree(dom);
> -        return false;
> -    }
> +    if (vshCommandOptString(cmd, "file", &from) <= 0)
> +        goto cleanup;
>  
>      if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) {
>          virshReportError(ctl);
> -        virDomainFree(dom);
> -        return false;
> +        goto cleanup;
> +    }
> +
> +    ret = vshCompleteXMLFromDomain(ctl, dom, buffer, &new_buffer);
> +    if (ret < 0) {
> +        if (ret == -2) {
> +            vshError(ctl, _("no such device in %s"), virDomainGetName(dom));
> +        } else if (ret == -3) {
> +            vshError(ctl, "%s", _("given XML selects too many devices. "
> +                                  "Please, be more specific"));
> +        } else {
> +            /* vshCompleteXMLFromDomain() already printed error message,
> +             * so nothing to do here. */
> +        }
> +        goto cleanup;
>      }
>  
>      if (vshCommandOptBool(cmd, "persistent")) {
>          flags = VIR_DOMAIN_AFFECT_CONFIG;
>          if (virDomainIsActive(dom) == 1)
>             flags |= VIR_DOMAIN_AFFECT_LIVE;
> -        ret = virDomainDetachDeviceFlags(dom, buffer, flags);
> +        ret = virDomainDetachDeviceFlags(dom, new_buffer, flags);
>      } else {
> -        ret = virDomainDetachDevice(dom, buffer);
> +        ret = virDomainDetachDevice(dom, new_buffer);
>      }
> -    VIR_FREE(buffer);
>  
>      if (ret < 0) {
>          vshError(ctl, _("Failed to detach device from %s"), from);
> -        virDomainFree(dom);
> -        return false;
> -    } else {
> -        vshPrint(ctl, "%s", _("Device detached successfully\n"));
> +        goto cleanup;
>      }
>  
> +    vshPrint(ctl, "%s", _("Device detached successfully\n"));
> +    funcRet = true;
> +
> +cleanup:
> +    VIR_FREE(new_buffer);
> +    VIR_FREE(buffer);
>      virDomainFree(dom);
> -    return true;
> +    return funcRet;
>  }
>  
>  
Ping?


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]