Both cpu-compare and cpu-baseline commands accept more that just CPU
definition XML(s). For users' convenience they are able to extract the
CPU definition(s) even from domain XML or capabilities XML. The main
differences between the two commands is in the number of CPU definitions
they expect: cpu-compare wants only one CPU definition while
cpu-baseline expects one or more CPUs.
The extracted code forms a new vshExtractCPUDefXML function.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
---
tools/virsh-host.c | 160 +++++++++++++++++++++------------------------
1 file changed, 75 insertions(+), 85 deletions(-)
diff --git a/tools/virsh-host.c b/tools/virsh-host.c
index 6d6e3cfc85..51497db385 100644
--- a/tools/virsh-host.c
+++ b/tools/virsh-host.c
@@ -1106,6 +1106,72 @@ cmdURI(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
return true;
}
+
+/* Extracts the CPU definition XML strings from a file which may contain either
+ * - just the CPU definitions,
+ * - domain XMLs, or
+ * - capabilities XMLs.
+ *
+ * Returns NULL terminated string list.
+ */
+static char **
+vshExtractCPUDefXMLs(vshControl *ctl,
+ const char *xmlFile)
+{
+ char **cpus = NULL;
+ char *buffer = NULL;
+ char *xmlStr = NULL;
+ xmlDocPtr xml = NULL;
+ xmlXPathContextPtr ctxt = NULL;
+ xmlNodePtr *nodes = NULL;
+ size_t i;
+ int n;
+
+ if (virFileReadAll(xmlFile, VSH_MAX_XML_FILE, &buffer) < 0)
+ goto error;
+
+ if (virAsprintf(&xmlStr, "<container>%s</container>", buffer) < 0)
+ goto error;
+
+ if (!(xml = virXMLParseStringCtxt(xmlStr, xmlFile, &ctxt)))
+ goto error;
+
+ n = virXPathNodeSet("/container/cpu|"
+ "/container/domain/cpu|"
+ "/container/capabilities/host/cpu",
+ ctxt, &nodes);
+ if (n < 0)
+ goto error;
+
+ if (n == 0) {
+ vshError(ctl, _("File '%s' does not contain any <cpu> element or "
+ "valid domain or capabilities XML"), xmlFile);
+ goto error;
+ }
+
+ cpus = vshCalloc(ctl, n + 1, sizeof(const char *));
+
+ for (i = 0; i < n; i++) {
+ if (!(cpus[i] = virXMLNodeToString(xml, nodes[i]))) {
+ vshSaveLibvirtError();
+ goto error;
+ }
+ }
+
+ cleanup:
+ VIR_FREE(buffer);
+ VIR_FREE(xmlStr);
+ xmlFreeDoc(xml);
+ xmlXPathFreeContext(ctxt);
+ VIR_FREE(nodes);
+ return cpus;
+
+ error:
+ virStringListFree(cpus);
+ goto cleanup;
+}
+
+
/*
* "cpu-compare" command
*/
@@ -1133,13 +1199,9 @@ cmdCPUCompare(vshControl *ctl, const vshCmd *cmd)
{
const char *from = NULL;
bool ret = false;
- char *buffer;
int result;
- char *snippet = NULL;
+ char **cpus = NULL;
unsigned int flags = 0;
- xmlDocPtr xml = NULL;
- xmlXPathContextPtr ctxt = NULL;
- xmlNodePtr node;
virshControlPtr priv = ctl->privData;
if (vshCommandOptBool(cmd, "error"))
@@ -1148,27 +1210,10 @@ cmdCPUCompare(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0)
return false;
- if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0)
+ if (!(cpus = vshExtractCPUDefXMLs(ctl, from)))
return false;
- /* try to extract the CPU element from as it would appear in a domain XML*/
- if (!(xml = virXMLParseStringCtxt(buffer, from, &ctxt)))
- goto cleanup;
-
- if ((node = virXPathNode("/cpu|"
- "/domain/cpu|"
- "/capabilities/host/cpu", ctxt))) {
- if (!(snippet = virXMLNodeToString(xml, node))) {
- vshSaveLibvirtError();
- goto cleanup;
- }
- } else {
- vshError(ctl, _("File '%s' does not contain a <cpu> element or is not "
- "a valid domain or capabilities XML"), from);
- goto cleanup;
- }
-
- result = virConnectCompareCPU(priv->conn, snippet, flags);
+ result = virConnectCompareCPU(priv->conn, cpus[0], flags);
switch (result) {
case VIR_CPU_COMPARE_INCOMPATIBLE:
@@ -1196,10 +1241,7 @@ cmdCPUCompare(vshControl *ctl, const vshCmd *cmd)
ret = true;
cleanup:
- VIR_FREE(buffer);
- VIR_FREE(snippet);
- xmlXPathFreeContext(ctxt);
- xmlFreeDoc(xml);
+ virStringListFree(cpus);
return ret;
}
@@ -1235,17 +1277,9 @@ cmdCPUBaseline(vshControl *ctl, const vshCmd *cmd)
{
const char *from = NULL;
bool ret = false;
- char *buffer;
char *result = NULL;
char **list = NULL;
unsigned int flags = 0;
- int count = 0;
-
- xmlDocPtr xml = NULL;
- xmlNodePtr *node_list = NULL;
- xmlXPathContextPtr ctxt = NULL;
- virBuffer buf = VIR_BUFFER_INITIALIZER;
- size_t i;
virshControlPtr priv = ctl->privData;
if (vshCommandOptBool(cmd, "features"))
@@ -1256,65 +1290,21 @@ cmdCPUBaseline(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0)
return false;
- if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0)
+ if (!(list = vshExtractCPUDefXMLs(ctl, from)))
return false;
- /* add a separate container around the xml */
- virBufferStrcat(&buf, "<container>", buffer, "</container>", NULL);
- if (virBufferError(&buf))
- goto no_memory;
-
- 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;
- }
-
- list = vshCalloc(ctl, count, sizeof(const char *));
-
- for (i = 0; i < count; i++) {
- if (!(list[i] = virXMLNodeToString(xml, node_list[i]))) {
- vshSaveLibvirtError();
- goto cleanup;
- }
- }
-
- result = virConnectBaselineCPU(priv->conn,
- (const char **)list, count, flags);
+ result = virConnectBaselineCPU(priv->conn, (const char **)list,
+ virStringListLength((const char **)list),
+ flags);
if (result) {
vshPrint(ctl, "%s", result);
ret = true;
}
- cleanup:
- xmlXPathFreeContext(ctxt);
- xmlFreeDoc(xml);
VIR_FREE(result);
- if (list != NULL && count > 0) {
- for (i = 0; i < count; i++)
- VIR_FREE(list[i]);
- }
- VIR_FREE(list);
- VIR_FREE(buffer);
- VIR_FREE(node_list);
-
+ virStringListFree(list);
return ret;
-
- no_memory:
- vshError(ctl, "%s", _("Out of memory"));
- ret = false;
- goto cleanup;
}
/*
--
2.17.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 05/16/2018 04:39 AM, Jiri Denemark wrote:
> Both cpu-compare and cpu-baseline commands accept more that just CPU
> definition XML(s). For users' convenience they are able to extract the
> CPU definition(s) even from domain XML or capabilities XML. The main
> differences between the two commands is in the number of CPU definitions
> they expect: cpu-compare wants only one CPU definition while
> cpu-baseline expects one or more CPUs.
>
> The extracted code forms a new vshExtractCPUDefXML function.
>
> Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
> ---
> tools/virsh-host.c | 160 +++++++++++++++++++++------------------------
> 1 file changed, 75 insertions(+), 85 deletions(-)
>
> diff --git a/tools/virsh-host.c b/tools/virsh-host.c
> index 6d6e3cfc85..51497db385 100644
> --- a/tools/virsh-host.c
> +++ b/tools/virsh-host.c
> @@ -1106,6 +1106,72 @@ cmdURI(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
> return true;
> }
>
> +
> +/* Extracts the CPU definition XML strings from a file which may contain either
> + * - just the CPU definitions,
> + * - domain XMLs, or
> + * - capabilities XMLs.
> + *
> + * Returns NULL terminated string list.
> + */
> +static char **
> +vshExtractCPUDefXMLs(vshControl *ctl,
> + const char *xmlFile)
> +{
> + char **cpus = NULL;
> + char *buffer = NULL;
> + char *xmlStr = NULL;
> + xmlDocPtr xml = NULL;
> + xmlXPathContextPtr ctxt = NULL;
> + xmlNodePtr *nodes = NULL;
> + size_t i;
> + int n;
> +
> + if (virFileReadAll(xmlFile, VSH_MAX_XML_FILE, &buffer) < 0)
> + goto error;
> +
> + if (virAsprintf(&xmlStr, "<container>%s</container>", buffer) < 0)
> + goto error;
> +
Why wrap the xml in the <container> tags?
> + if (!(xml = virXMLParseStringCtxt(xmlStr, xmlFile, &ctxt)))
> + goto error;
> +
> + n = virXPathNodeSet("/container/cpu|"
> + "/container/domain/cpu|"
> + "/container/capabilities/host/cpu",
> + ctxt, &nodes);
> + if (n < 0)
> + goto error;
> +
> + if (n == 0) {
> + vshError(ctl, _("File '%s' does not contain any <cpu> element or "
> + "valid domain or capabilities XML"), xmlFile);
> + goto error;
> + }
> +
> + cpus = vshCalloc(ctl, n + 1, sizeof(const char *));
> +
> + for (i = 0; i < n; i++) {
> + if (!(cpus[i] = virXMLNodeToString(xml, nodes[i]))) {
> + vshSaveLibvirtError();
> + goto error;
> + }
> + }
> +
> + cleanup:
> + VIR_FREE(buffer);
> + VIR_FREE(xmlStr);
> + xmlFreeDoc(xml);
> + xmlXPathFreeContext(ctxt);
> + VIR_FREE(nodes);
> + return cpus;
> +
> + error:
> + virStringListFree(cpus);
> + goto cleanup;
> +}
> +
> +
> /*
> * "cpu-compare" command
> */
> @@ -1133,13 +1199,9 @@ cmdCPUCompare(vshControl *ctl, const vshCmd *cmd)
> {
> const char *from = NULL;
> bool ret = false;
> - char *buffer;
> int result;
> - char *snippet = NULL;
> + char **cpus = NULL;
> unsigned int flags = 0;
> - xmlDocPtr xml = NULL;
> - xmlXPathContextPtr ctxt = NULL;
> - xmlNodePtr node;
> virshControlPtr priv = ctl->privData;
>
> if (vshCommandOptBool(cmd, "error"))
> @@ -1148,27 +1210,10 @@ cmdCPUCompare(vshControl *ctl, const vshCmd *cmd)
> if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0)
> return false;
>
> - if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0)
> + if (!(cpus = vshExtractCPUDefXMLs(ctl, from)))
> return false;
>
> - /* try to extract the CPU element from as it would appear in a domain XML*/
> - if (!(xml = virXMLParseStringCtxt(buffer, from, &ctxt)))
> - goto cleanup;
> -
> - if ((node = virXPathNode("/cpu|"
> - "/domain/cpu|"
> - "/capabilities/host/cpu", ctxt))) {
> - if (!(snippet = virXMLNodeToString(xml, node))) {
> - vshSaveLibvirtError();
> - goto cleanup;
> - }
> - } else {
> - vshError(ctl, _("File '%s' does not contain a <cpu> element or is not "
> - "a valid domain or capabilities XML"), from);
> - goto cleanup;
> - }
> -
> - result = virConnectCompareCPU(priv->conn, snippet, flags);
> + result = virConnectCompareCPU(priv->conn, cpus[0], flags);
I wonder if it's worth commenting here or adding to the cpu compare docs that comparison only
compares the host CPU with the _first_ cpu found in the XML file?
>
> switch (result) {
> case VIR_CPU_COMPARE_INCOMPATIBLE:
> @@ -1196,10 +1241,7 @@ cmdCPUCompare(vshControl *ctl, const vshCmd *cmd)
> ret = true;
>
> cleanup:
> - VIR_FREE(buffer);
> - VIR_FREE(snippet);
> - xmlXPathFreeContext(ctxt);
> - xmlFreeDoc(xml);
> + virStringListFree(cpus);
>
> return ret;
> }
[...]
The rest of this patch looks good... I'd like to take another look tomorrow in case I missed
anything subtle.
--
Respectfully,
- Collin Walling
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, May 22, 2018 at 17:33:14 -0400, Collin Walling wrote:
> On 05/16/2018 04:39 AM, Jiri Denemark wrote:
> > Both cpu-compare and cpu-baseline commands accept more that just CPU
> > definition XML(s). For users' convenience they are able to extract the
> > CPU definition(s) even from domain XML or capabilities XML. The main
> > differences between the two commands is in the number of CPU definitions
> > they expect: cpu-compare wants only one CPU definition while
> > cpu-baseline expects one or more CPUs.
> >
> > The extracted code forms a new vshExtractCPUDefXML function.
> >
> > Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
> > ---
> > tools/virsh-host.c | 160 +++++++++++++++++++++------------------------
> > 1 file changed, 75 insertions(+), 85 deletions(-)
> >
> > diff --git a/tools/virsh-host.c b/tools/virsh-host.c
> > index 6d6e3cfc85..51497db385 100644
> > --- a/tools/virsh-host.c
> > +++ b/tools/virsh-host.c
> > @@ -1106,6 +1106,72 @@ cmdURI(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
> > return true;
> > }
> >
> > +
> > +/* Extracts the CPU definition XML strings from a file which may contain either
> > + * - just the CPU definitions,
> > + * - domain XMLs, or
> > + * - capabilities XMLs.
> > + *
> > + * Returns NULL terminated string list.
> > + */
> > +static char **
> > +vshExtractCPUDefXMLs(vshControl *ctl,
> > + const char *xmlFile)
> > +{
> > + char **cpus = NULL;
> > + char *buffer = NULL;
> > + char *xmlStr = NULL;
> > + xmlDocPtr xml = NULL;
> > + xmlXPathContextPtr ctxt = NULL;
> > + xmlNodePtr *nodes = NULL;
> > + size_t i;
> > + int n;
> > +
> > + if (virFileReadAll(xmlFile, VSH_MAX_XML_FILE, &buffer) < 0)
> > + goto error;
> > +
> > + if (virAsprintf(&xmlStr, "<container>%s</container>", buffer) < 0)
> > + goto error;
> > +
>
> Why wrap the xml in the <container> tags?
Because you can only have one root element in an XML document. Thus to
be able to parse a file with several root elements, we need to
encapsulate the content into a container.
>
> > + if (!(xml = virXMLParseStringCtxt(xmlStr, xmlFile, &ctxt)))
> > + goto error;
> > +
> > + n = virXPathNodeSet("/container/cpu|"
> > + "/container/domain/cpu|"
> > + "/container/capabilities/host/cpu",
> > + ctxt, &nodes);
> > + if (n < 0)
> > + goto error;
> > +
> > + if (n == 0) {
> > + vshError(ctl, _("File '%s' does not contain any <cpu> element or "
> > + "valid domain or capabilities XML"), xmlFile);
> > + goto error;
> > + }
> > +
> > + cpus = vshCalloc(ctl, n + 1, sizeof(const char *));
> > +
> > + for (i = 0; i < n; i++) {
> > + if (!(cpus[i] = virXMLNodeToString(xml, nodes[i]))) {
> > + vshSaveLibvirtError();
> > + goto error;
> > + }
> > + }
> > +
> > + cleanup:
> > + VIR_FREE(buffer);
> > + VIR_FREE(xmlStr);
> > + xmlFreeDoc(xml);
> > + xmlXPathFreeContext(ctxt);
> > + VIR_FREE(nodes);
> > + return cpus;
> > +
> > + error:
> > + virStringListFree(cpus);
> > + goto cleanup;
> > +}
> > +
> > +
> > /*
> > * "cpu-compare" command
> > */
> > @@ -1133,13 +1199,9 @@ cmdCPUCompare(vshControl *ctl, const vshCmd *cmd)
> > {
> > const char *from = NULL;
> > bool ret = false;
> > - char *buffer;
> > int result;
> > - char *snippet = NULL;
> > + char **cpus = NULL;
> > unsigned int flags = 0;
> > - xmlDocPtr xml = NULL;
> > - xmlXPathContextPtr ctxt = NULL;
> > - xmlNodePtr node;
> > virshControlPtr priv = ctl->privData;
> >
> > if (vshCommandOptBool(cmd, "error"))
> > @@ -1148,27 +1210,10 @@ cmdCPUCompare(vshControl *ctl, const vshCmd *cmd)
> > if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0)
> > return false;
> >
> > - if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0)
> > + if (!(cpus = vshExtractCPUDefXMLs(ctl, from)))
> > return false;
> >
> > - /* try to extract the CPU element from as it would appear in a domain XML*/
> > - if (!(xml = virXMLParseStringCtxt(buffer, from, &ctxt)))
> > - goto cleanup;
> > -
> > - if ((node = virXPathNode("/cpu|"
> > - "/domain/cpu|"
> > - "/capabilities/host/cpu", ctxt))) {
> > - if (!(snippet = virXMLNodeToString(xml, node))) {
> > - vshSaveLibvirtError();
> > - goto cleanup;
> > - }
> > - } else {
> > - vshError(ctl, _("File '%s' does not contain a <cpu> element or is not "
> > - "a valid domain or capabilities XML"), from);
> > - goto cleanup;
> > - }
> > -
> > - result = virConnectCompareCPU(priv->conn, snippet, flags);
> > + result = virConnectCompareCPU(priv->conn, cpus[0], flags);
>
> I wonder if it's worth commenting here or adding to the cpu compare docs that comparison only
> compares the host CPU with the _first_ cpu found in the XML file?
The cpu-compare command is already documented this way. It accepts a CPU
definition and compares it to host CPU, while cpu-baseline accepts a set
of CPU definitions.
Jirka
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, May 16, 2018 at 10:39:21AM +0200, Jiri Denemark wrote: >Both cpu-compare and cpu-baseline commands accept more that just CPU >definition XML(s). For users' convenience they are able to extract the >CPU definition(s) even from domain XML or capabilities XML. The main >differences between the two commands is in the number of CPU definitions >they expect: cpu-compare wants only one CPU definition while >cpu-baseline expects one or more CPUs. > >The extracted code forms a new vshExtractCPUDefXML function. > >Signed-off-by: Jiri Denemark <jdenemar@redhat.com> >--- > tools/virsh-host.c | 160 +++++++++++++++++++++------------------------ > 1 file changed, 75 insertions(+), 85 deletions(-) > Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.