On Wed, Jan 24, 2018 at 10:32:07PM +0100, Martin Kletzander wrote: > On Wed, Jan 24, 2018 at 03:04:37PM +0100, Pavel Hrdina wrote: > > On Tue, Jan 23, 2018 at 07:05:15PM +0100, Martin Kletzander wrote: > > > More info in the documentation, this is basically the XML > > > parsing/formatting > > > support, schemas, tests and documentation for the new cputune/cachetune > > > element > > > that will get used by following patches. > > > > > > Signed-off-by: Martin Kletzander <[email protected]> > > > --- > > > docs/formatdomain.html.in | 54 ++++ > > > docs/schemas/domaincommon.rng | 32 +++ > > > src/conf/domain_conf.c | 295 > > > ++++++++++++++++++++- > > > src/conf/domain_conf.h | 13 + > > > tests/genericxml2xmlindata/cachetune-cdp.xml | 36 +++ > > > .../cachetune-colliding-allocs.xml | 30 +++ > > > .../cachetune-colliding-tunes.xml | 32 +++ > > > .../cachetune-colliding-types.xml | 30 +++ > > > tests/genericxml2xmlindata/cachetune-small.xml | 29 ++ > > > tests/genericxml2xmlindata/cachetune.xml | 33 +++ > > > tests/genericxml2xmltest.c | 10 + > > > 11 files changed, 592 insertions(+), 2 deletions(-) > > > create mode 100644 tests/genericxml2xmlindata/cachetune-cdp.xml > > > create mode 100644 > > > tests/genericxml2xmlindata/cachetune-colliding-allocs.xml > > > create mode 100644 > > > tests/genericxml2xmlindata/cachetune-colliding-tunes.xml > > > create mode 100644 > > > tests/genericxml2xmlindata/cachetune-colliding-types.xml > > > create mode 100644 tests/genericxml2xmlindata/cachetune-small.xml > > > create mode 100644 tests/genericxml2xmlindata/cachetune.xml > > > > > > diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in > > > index d272cc1ba698..7b4d9051a551 100644 > > > --- a/docs/formatdomain.html.in > > > +++ b/docs/formatdomain.html.in > > > @@ -689,6 +689,10 @@ > > > <iothread_quota>-1</iothread_quota> > > > <vcpusched vcpus='0-4,^3' scheduler='fifo' priority='1'/> > > > <iothreadsched iothreads='2' scheduler='batch'/> > > > + <cachetune vcpus='0-3'> > > > + <cache id='0' level='3' type='both' size='3' unit='MiB'/> > > > + <cache id='1' level='3' type='both' size='3' unit='MiB'/> > > > + </cachetune> > > > </cputune> > > > ... > > > </domain> > > > @@ -834,6 +838,56 @@ > > > <span class="since">Since 1.2.13</span> > > > </dd> > > > > > > + <dt><code>cachetune</code><span class="since">Since > > > 4.1.0</span></dt> > > > + <dd> > > > + Optional <code>cachetune</code> element can control allocations > > > for CPU > > > + caches using the resctrl on the host. Whether or not is this > > > supported > > > + can be gathered from capabilities where some limitations like > > > minimum > > > + size and required granularity are reported as well. The required > > > > s/ / / > > > > > + attribute <code>vcpus</code> specifies to which vCPUs this > > > allocation > > > + applies. A vCPU can only be member of one <code>cachetune</code> > > > element > > > + allocations. Supported subelements are: > > > + <dl> > > > + <dt><code>cache</code></dt> > > > + <dd> > > > + This element controls the allocation of CPU cache and has the > > > + following attributes: > > > + <dl> > > > + <dt><code>level</code></dt> > > > + <dd> > > > + Host cache level from which to allocate. > > > + </dd> > > > + <dt><code>id</code></dt> > > > + <dd> > > > + Host cache id from which to allocate. > > > + </dd> > > > + <dt><code>type</code></dt> > > > + <dd> > > > + Type of allocation. Can be <code>code</code> for code > > > > s/ / / > > > > > + (instructions), <code>data</code> for data or > > > <code>both</code> > > > + for both code and data (unified). Currently the > > > allocation can > > > > s/ / / > > > > Fixed all three. > > /me wonders why people still insist on this. Is someone editing these > files with proportional font? Do we have any other output than > HTML? I guess this will be yet another thing I have to get used to > and surrender.
I personally don't care about the double space or single space, this
was just for consistency.
> > > + be done only with the same type as the host supports,
> > > meaning
> > > + you cannot request <code>both</code> for host with CDP
> > > + (code/data prioritization) enabled.
> > > + </dd>
> > > + <dt><code>size</code></dt>
> > > + <dd>
> > > + The size of the region to allocate. The value by default
> > > is in
> > > + bytes, but the <code>unit</code> attribute can be used
> > > to scale
> > > + the value.
> > > + </dd>
> > > + <dt><code>unit</code> (optional)</dt>
> > > + <dd>
> > > + If specified it is the unit such as KiB, MiB, GiB, or TiB
> > > + (described in the <code>memory</code> element
> > > + for <a href="#elementsMemoryAllocation">Memory
> > > Allocation</a>)
> > > + in which <code>size</code> is specified, defaults to
> > > bytes.
> > > + </dd>
> > > + </dl>
> > > + </dd>
> > > + </dl>
> > > +
> > > + </dd>
> > > </dl>
> > >
> > >
> > > diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> > > index f22c932f6c09..252f58f4379c 100644
> > > --- a/docs/schemas/domaincommon.rng
> > > +++ b/docs/schemas/domaincommon.rng
> > > @@ -900,6 +900,38 @@
> > > <ref name="schedparam"/>
> > > </element>
> > > </zeroOrMore>
> > > + <zeroOrMore>
> > > + <element name="cachetune">
> > > + <attribute name="vcpus">
> > > + <ref name='cpuset'/>
> > > + </attribute>
> > > + <oneOrMore>
> > > + <element name="cache">
> > > + <attribute name="id">
> > > + <ref name='unsignedInt'/>
> > > + </attribute>
> > > + <attribute name="level">
> > > + <ref name='unsignedInt'/>
> > > + </attribute>
> > > + <attribute name="type">
> > > + <choice>
> > > + <value>both</value>
> > > + <value>code</value>
> > > + <value>data</value>
> > > + </choice>
> > > + </attribute>
> > > + <attribute name="size">
> > > + <ref name='unsignedLong'/>
> > > + </attribute>
> > > + <optional>
> > > + <attribute name='unit'>
> > > + <ref name='unit'/>
> > > + </attribute>
> > > + </optional>
> > > + </element>
> > > + </oneOrMore>
> > > + </element>
> > > + </zeroOrMore>
> > > </interleave>
> > > </element>
> > > </define>
> > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > > index a1c25060f9e9..27665d0372a7 100644
> > > --- a/src/conf/domain_conf.c
> > > +++ b/src/conf/domain_conf.c
> > > @@ -2883,6 +2883,19 @@ virDomainLoaderDefFree(virDomainLoaderDefPtr
> > > loader)
> > > VIR_FREE(loader);
> > > }
> > >
> > > +
> > > +static void
> > > +virDomainCachetuneDefFree(virDomainCachetuneDefPtr cachetune)
> > > +{
> > > + if (!cachetune)
> > > + return;
> > > +
> > > + virObjectUnref(cachetune->alloc);
> > > + virBitmapFree(cachetune->vcpus);
> > > + VIR_FREE(cachetune);
> > > +}
> > > +
> > > +
> > > void virDomainDefFree(virDomainDefPtr def)
> > > {
> > > size_t i;
> > > @@ -3055,6 +3068,10 @@ void virDomainDefFree(virDomainDefPtr def)
> > > virDomainShmemDefFree(def->shmems[i]);
> > > VIR_FREE(def->shmems);
> > >
> > > + for (i = 0; i < def->ncachetunes; i++)
> > > + virDomainCachetuneDefFree(def->cachetunes[i]);
> > > + VIR_FREE(def->cachetunes);
> > > +
> > > VIR_FREE(def->keywrap);
> > >
> > > if (def->namespaceData && def->ns.free)
> > > @@ -18247,6 +18264,194 @@ virDomainDefParseBootOptions(virDomainDefPtr
> > > def,
> > > }
> > >
> > >
> > > +static int
> > > +virDomainCachetuneDefParseCache(xmlXPathContextPtr ctxt,
> > > + xmlNodePtr node,
> > > + virResctrlAllocPtr alloc)
> > > +{
> > > + xmlNodePtr oldnode = ctxt->node;
> > > + unsigned int level;
> > > + unsigned int cache;
> > > + int type;
> > > + unsigned long long size;
> > > + char *tmp = NULL;
> > > + int ret = -1;
> > > +
> > > + ctxt->node = node;
> > > +
> > > + tmp = virXMLPropString(node, "id");
> > > + if (!tmp) {
> > > + virReportError(VIR_ERR_XML_ERROR, "%s",
> > > + _("Missing cachetune attribute 'id'"));
> > > + goto cleanup;
> > > + }
> > > + if (virStrToLong_uip(tmp, NULL, 10, &cache) < 0) {
> > > + virReportError(VIR_ERR_XML_ERROR,
> > > + _("Invalid cachetune attribute 'id' value '%s'"),
> > > + tmp);
> > > + goto cleanup;
> > > + }
> > > + VIR_FREE(tmp);
> > > +
> > > + tmp = virXMLPropString(node, "level");
> > > + if (!tmp) {
> > > + virReportError(VIR_ERR_XML_ERROR, "%s",
> > > + _("Missing cachetune attribute 'level'"));
> > > + goto cleanup;
> > > + }
> > > + if (virStrToLong_uip(tmp, NULL, 10, &level) < 0) {
> > > + virReportError(VIR_ERR_XML_ERROR,
> > > + _("Invalid cachetune attribute 'level' value
> > > '%s'"),
> > > + tmp);
> > > + goto cleanup;
> > > + }
> > > + VIR_FREE(tmp);
> > > +
> > > + tmp = virXMLPropString(node, "type");
> > > + if (!tmp) {
> > > + virReportError(VIR_ERR_XML_ERROR, "%s",
> > > + _("Missing cachetune attribute 'type'"));
> > > + goto cleanup;
> > > + }
> > > + type = virCacheTypeFromString(tmp);
> > > + if (type < 0) {
> > > + virReportError(VIR_ERR_XML_ERROR,
> > > + _("Invalid cachetune attribute 'type' value
> > > '%s'"),
> > > + tmp);
> > > + goto cleanup;
> > > + }
> > > + VIR_FREE(tmp);
> > > +
> > > + if (virDomainParseScaledValue("./@size", "./@unit",
> > > + ctxt, &size, 1024,
> > > + ULLONG_MAX, true) < 0)
> > > + goto cleanup;
> > > +
> > > + if (virResctrlAllocSetSize(alloc, level, type, cache, size) < 0)
> > > + goto cleanup;
> > > +
> > > + ret = 0;
> > > + cleanup:
> > > + ctxt->node = oldnode;
> > > + VIR_FREE(tmp);
> > > + return ret;
> > > +}
> > > +
> > > +
> > > +static int
> > > +virDomainCachetuneDefParse(virDomainDefPtr def,
> > > + xmlXPathContextPtr ctxt,
> > > + xmlNodePtr node,
> > > + unsigned int flags)
> > > +{
> > > + xmlNodePtr oldnode = ctxt->node;
> > > + xmlNodePtr *nodes = NULL;
> > > + virBitmapPtr vcpus = NULL;
> > > + virResctrlAllocPtr alloc = virResctrlAllocNew();
> > > + virDomainCachetuneDefPtr tmp_cachetune = NULL;
> > > + char *tmp = NULL;
> > > + char *vcpus_str = NULL;
> > > + char *alloc_id = NULL;
> > > + ssize_t i = 0;
> > > + int n;
> > > + int ret = -1;
> > > +
> > > + ctxt->node = node;
> > > +
> > > + if (!alloc)
> > > + goto cleanup;
> > > +
> > > + if (VIR_ALLOC(tmp_cachetune) < 0)
> > > + goto cleanup;
> > > +
> > > + vcpus_str = virXMLPropString(node, "vcpus");
> > > + if (!vcpus_str) {
> > > + virReportError(VIR_ERR_XML_ERROR, "%s",
> > > + _("Missing cachetune attribute 'vcpus'"));
> > > + goto cleanup;
> > > + }
> > > + if (virBitmapParse(vcpus_str, &vcpus, VIR_DOMAIN_CPUMASK_LEN) < 0) {
> > > + virReportError(VIR_ERR_XML_ERROR,
> > > + _("Invalid cachetune attribute 'vcpus' value
> > > '%s'"),
> > > + vcpus_str);
> > > + goto cleanup;
> > > + }
> > > +
> > > + /* We need to limit the bitmap to number of vCPUs. If there's
> > > nothing left,
> > > + * then we can just clean up and return 0 immediately */
> > > + virBitmapShrink(vcpus, def->maxvcpus);
> > > + if (virBitmapIsAllClear(vcpus)) {
> > > + ret = 0;
> > > + goto cleanup;
> > > + }
> > > +
> > > + if ((n = virXPathNodeSet("./cache", ctxt, &nodes)) < 0) {
> > > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > > + _("Cannot extract cache nodes under cachetune"));
> > > + goto cleanup;
> > > + }
> > > +
> > > + for (i = 0; i < n; i++) {
> > > + if (virDomainCachetuneDefParseCache(ctxt, nodes[i], alloc) < 0)
> > > + goto cleanup;
> > > + }
> > > +
> > > + if (virResctrlAllocIsEmpty(alloc)) {
> > > + ret = 0;
> > > + goto cleanup;
> > > + }
> >
> > Can this ever happen? The
> >
>
> Sure, for example:
>
> <cachetune/>
Right, I figure that out and I thought that I removed this comment.
> > > +
> > > + for (i = 0; i < def->ncachetunes; i++) {
> > > + if (virBitmapOverlaps(def->cachetunes[i]->vcpus, vcpus)) {
> > > + virReportError(VIR_ERR_XML_ERROR, "%s",
> > > + _("Overlapping vcpus in cachetunes"));
> > > + goto cleanup;
> > > + }
> > > + }
> > > +
> > > + /* We need to format it back because we need to be consistent in the
> > > naming
> > > + * even when users specify some "sub-optimal" string there. */
> > > + VIR_FREE(vcpus_str);
> > > + vcpus_str = virBitmapFormat(vcpus);
> > > + if (!vcpus_str)
> > > + goto cleanup;
> > > +
> > > + if (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE))
> > > + alloc_id = virXMLPropString(node, "id");
> > > +
> > > + if (!alloc_id) {
> > > + /* The number of allocatios is limited and the directory
> > > structure is flat,
> >
> > s/allocatios/allocations/
> >
>
> Fixed.
>
> > Reviewed-by: Pavel Hrdina <[email protected]>
>
>
signature.asc
Description: PGP signature
-- libvir-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
