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@xxxxxxxxxx>
> > ---
> > 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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
