[libvirt] [PATCH 06/10] conf: Add support for cputune/cachetune
Martin Kletzander
mkletzan at redhat.com
Wed Jan 24 21:32:07 UTC 2018
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 <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/ / /
>
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.
>> + 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/>
>> +
>> + 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 <phrdina at redhat.com>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20180124/60a8b614/attachment-0001.sig>
More information about the libvir-list
mailing list