[libvirt] [PATCH 06/10] conf: Add support for cputune/cachetune

Pavel Hrdina phrdina at redhat.com
Wed Jan 24 14:04:37 UTC 2018


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 <mkletzan at redhat.com>
> ---
>  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/  / /

> +                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 

> +
> +    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/

Reviewed-by: Pavel Hrdina <phrdina at redhat.com>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20180124/5b87128d/attachment-0001.sig>


More information about the libvir-list mailing list