[libvirt] [PATCH] virsh: Allow using complete <capabilities> elements with cpu-baseline
This patch cleans the cpu baseline function using new libvirt helper functions and fixes XPath expression that selects <cpu> elements from the source file, that can contain concatenated <capabilities> XMLs, domain XMLs and bare <cpu> elements. The fixed XPath expression ensures not to select NUMA <cpu id=... elements. https://bugzilla.redhat.com/show_bug.cgi?id=731645 --- tools/virsh.c | 88 ++++++++++++++++++++++++-------------------------------- 1 files changed, 38 insertions(+), 50 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 3c6e65a..7ea5e1b 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -11908,18 +11908,18 @@ static bool cmdCPUBaseline(vshControl *ctl, const vshCmd *cmd) { const char *from = NULL; - bool ret = true; + bool ret = false; char *buffer; char *result = NULL; const char **list = NULL; - unsigned int count = 0; - xmlDocPtr doc = NULL; - xmlNodePtr node_list; + int count = 0; + + xmlDocPtr xml = NULL; + xmlNodePtr *node_list = NULL; xmlXPathContextPtr ctxt = NULL; - xmlSaveCtxtPtr sctxt = NULL; - xmlBufferPtr buf = NULL; - xmlXPathObjectPtr obj = NULL; - int res, i; + xmlBufferPtr xml_buf = NULL; + virBuffer buf = VIR_BUFFER_INITIALIZER; + int i; if (!vshConnectionUsability(ctl, ctl->conn)) return false; @@ -11930,69 +11930,57 @@ cmdCPUBaseline(vshControl *ctl, const vshCmd *cmd) if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) return false; - doc = xmlNewDoc(NULL); - if (doc == NULL) + /* add an separate container around the xml */ + virBufferStrcat(&buf, "<container>", buffer, "</container>", NULL); + if (virBufferError(&buf)) goto no_memory; - res = xmlParseBalancedChunkMemory(doc, NULL, NULL, 0, - (const xmlChar *)buffer, &node_list); - if (res != 0) { - vshError(ctl, _("Failed to parse XML fragment %s"), from); - ret = false; + VIR_FREE(buffer); + buffer = virBufferContentAndReset(&buf); + + + if (!(xml = virXMLParseStringCtxt(buffer, from, &ctxt))) + goto cleanup; + + if ((count = virXPathNodeSet("//cpu[not(ancestor::cpus)]", + ctxt, &node_list)) == -1) + goto cleanup; + + if (count == 0) { + vshError(ctl, _("No host CPU specified in '%s'"), from); goto cleanup; } - xmlAddChildList((xmlNodePtr) doc, node_list); + list = vshCalloc(ctl, count, sizeof(const char *)); - ctxt = xmlXPathNewContext(doc); - if (!ctxt) + if (!(xml_buf = xmlBufferCreate())) goto no_memory; - obj = xmlXPathEval(BAD_CAST "//cpu[not(ancestor::cpu)]", ctxt); - if ((obj == NULL) || (obj->nodesetval == NULL) || - (obj->nodesetval->nodeTab == NULL)) - goto cleanup; + for (i = 0; i < count; i++) { + xmlBufferEmpty(xml_buf); - for (i = 0;i < obj->nodesetval->nodeNr;i++) { - buf = xmlBufferCreate(); - if (buf == NULL) - goto no_memory; - sctxt = xmlSaveToBuffer(buf, NULL, 0); - if (sctxt == NULL) { - xmlBufferFree(buf); - goto no_memory; + if (xmlNodeDump(xml_buf, xml, node_list[i], 0, 0) < 0) { + vshError(ctl, _("Failed to extract <cpu> element")); + goto cleanup; } - xmlSaveTree(sctxt, obj->nodesetval->nodeTab[i]); - xmlSaveClose(sctxt); - - list = vshRealloc(ctl, list, sizeof(char *) * (count + 1)); - list[count++] = (char *) buf->content; - buf->content = NULL; - xmlBufferFree(buf); - buf = NULL; - } - - if (count == 0) { - vshError(ctl, _("No host CPU specified in '%s'"), from); - ret = false; - goto cleanup; + list[i] = vshStrdup(ctl, (const char *)xmlBufferContent(xml_buf)); } result = virConnectBaselineCPU(ctl->conn, list, count, 0); - if (result) + if (result) { vshPrint(ctl, "%s", result); - else - ret = false; + ret = true; + } cleanup: - xmlXPathFreeObject(obj); xmlXPathFreeContext(ctxt); - xmlFreeDoc(doc); + xmlFreeDoc(xml); + xmlBufferFree(xml_buf); VIR_FREE(result); if ((list != NULL) && (count > 0)) { - for (i = 0;i < count;i++) + for (i = 0; i < count; i++) VIR_FREE(list[i]); } VIR_FREE(list); -- 1.7.3.4
On 09/15/2011 02:05 PM, Peter Krempa wrote:
This patch cleans the cpu baseline function using new libvirt helper functions and fixes XPath expression that selects<cpu> elements from the source file, that can contain concatenated<capabilities> XMLs, domain XMLs and bare<cpu> elements. The fixed XPath expression ensures not to select NUMA<cpu id=... elements.
https://bugzilla.redhat.com/show_bug.cgi?id=731645 --- tools/virsh.c | 88 ++++++++++++++++++++++++-------------------------------- 1 files changed, 38 insertions(+), 50 deletions(-)
Ping? (This patch still applies cleanly to current HEAD). Peter
On 09/15/2011 06:05 AM, Peter Krempa wrote:
This patch cleans the cpu baseline function using new libvirt helper functions and fixes XPath expression that selects<cpu> elements from the source file, that can contain concatenated<capabilities> XMLs, domain XMLs and bare<cpu> elements. The fixed XPath expression ensures not to select NUMA<cpu id=... elements.
https://bugzilla.redhat.com/show_bug.cgi?id=731645 --- tools/virsh.c | 88 ++++++++++++++++++++++++-------------------------------- 1 files changed, 38 insertions(+), 50 deletions(-)
Fixing bugs and more compact at the same time! I didn't see anything obviously wrong with what you've done, but the patch is incomplete, as I now get a compiler warning - you removed the last use of vshRealloc, so you have even more lines of code to remove! virsh.c:418:1: error: '_vshRealloc' defined but not used [-Wunused-function] ACK if you squash this in: diff --git i/tools/virsh.c w/tools/virsh.c index 3e05ce9..1863db3 100644 --- i/tools/virsh.c +++ w/tools/virsh.c @@ -384,9 +384,6 @@ static void *_vshMalloc(vshControl *ctl, size_t sz, const char *filename, int li static void *_vshCalloc(vshControl *ctl, size_t nmemb, size_t sz, const char *filename, int line); #define vshCalloc(_ctl, _nmemb, _sz) _vshCalloc(_ctl, _nmemb, _sz, __FILE__, __LINE__) -static void *_vshRealloc(vshControl *ctl, void *ptr, size_t sz, const char *filename, int line); -#define vshRealloc(_ctl, _ptr, _sz) _vshRealloc(_ctl, _ptr, _sz, __FILE__, __LINE__) - static char *_vshStrdup(vshControl *ctl, const char *s, const char *filename, int line); #define vshStrdup(_ctl, _s) _vshStrdup(_ctl, _s, __FILE__, __LINE__) @@ -414,19 +411,6 @@ _vshCalloc(vshControl *ctl, size_t nmemb, size_t size, const char *filename, int exit(EXIT_FAILURE); } -static void * -_vshRealloc(vshControl *ctl, void *ptr, size_t size, const char *filename, int line) -{ - void *x; - - if ((x = realloc(ptr, size))) - return x; - VIR_FREE(ptr); - vshError(ctl, _("%s: %d: failed to allocate %d bytes"), - filename, line, (int) size); - exit(EXIT_FAILURE); -} - static char * _vshStrdup(vshControl *ctl, const char *s, const char *filename, int line) { -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Dňa 27.9.2011 17:26, Eric Blake wrote / napísal(a):
ACK if you squash this in:
diff --git i/tools/virsh.c w/tools/virsh.c index 3e05ce9..1863db3 100644 --- i/tools/virsh.c +++ w/tools/virsh.c @@ -384,9 +384,6 @@ static void *_vshMalloc(vshControl *ctl, size_t sz, const char *filename, int li static void *_vshCalloc(vshControl *ctl, size_t nmemb, size_t sz, const char *filename, int line); #define vshCalloc(_ctl, _nmemb, _sz) _vshCalloc(_ctl, _nmemb, _sz, __FILE__, __LINE__)
Uh, thanks for noticing that. I somehow forgot to fix that even after I saw gcc complaining :( Thanks, pushed.
participants (2)
-
Eric Blake -
Peter Krempa