On 24.08.2011 14:53, Michal Privoznik wrote: > From: Michal Prívozník <mprivozn@xxxxxxxxxx> > > 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? -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list