[libvirt] [PATCH v3] virsh: Increase device-detach intelligence
Michal Privoznik
mprivozn at redhat.com
Thu Sep 8 14:48:04 UTC 2011
On 24.08.2011 14:53, Michal Privoznik wrote:
> From: Michal Prívozník <mprivozn at 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?
More information about the libvir-list
mailing list