On 12/13/2017 10:39 AM, 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@redhat.com> --- docs/formatdomain.html.in | 54 +++++ docs/schemas/domaincommon.rng | 32 +++ src/conf/domain_conf.c | 251 +++++++++++++++++++++ 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, 550 insertions(+) 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 01db83e60820..623860cc0e95 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 3.10.0</span></dt>
At least 4.0.0 if not 4.1.0
+ <dd> + Optional <code>cachetune</code> element can control allocations for CPU + caches using the resctrlfs on the host. Whether or not is this supported
s/resctrlfs/Resource Control File System/ Although this defaults to /sys/fs/resctrl, I'm not sure if we want to state the default or not. It *is* the path we will use though and I don't believe we have a way to alter that default (yet).
+ can be gathered from capabilities where some limitations like minimum + size and required granularity are reported as well. The required + 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.
Oh - should have read this first ;-)... Maybe rename @id internally to hostCacheID... Shall I assume someone setting this up would know how to determine how to get these values from the host?
+ </dd> + <dt><code>type</code></dt> + <dd> + Type of allocation. Can be <code>code</code> for code + (instructions), <code>data</code> for data or <code>both</code> + for both code and data (unified). Currently the allocation can + 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/src/conf/domain_conf.c b/src/conf/domain_conf.c index 66e21c4bdbee..ec8d760c7971 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2883,6 +2883,17 @@ virDomainLoaderDefFree(virDomainLoaderDefPtr loader) VIR_FREE(loader); }
+static void +virDomainCachetuneDefFree(virDomainCachetuneDefPtr cachetune) +{ + if (!cachetune) + return; + + virObjectUnref(cachetune->alloc); + virBitmapFree(cachetune->vcpus); + VIR_FREE(cachetune); +} +
Two empty lines before and after
void virDomainDefFree(virDomainDefPtr def) { size_t i;
[...]
+struct virCachetuneHelperData { + virBufferPtr buf; + size_t vcpu_id; +}; + +static int +virDomainCachetuneDefFormatHelper(unsigned int level, + virCacheType type, + unsigned int cache, + unsigned long long size, + void *opaque) +{ + const char *unit; + virBufferPtr buf = opaque; + unsigned long long short_size = virFormatIntPretty(size, &unit); + + virBufferAsprintf(buf, + "<cache id='%u' level='%u' type='%s' " + "size='%llu' unit='%s'/>\n", + cache, level, virCacheTypeToString(type), + short_size, unit);
A nit, but if the default is "B" and we don't require for parse, then do we want to Format the output? IOW: why print unit='B'
+ + return 0; +} + +
[...]
diff --git a/tests/genericxml2xmlindata/cachetune-cdp.xml b/tests/genericxml2xmlindata/cachetune-cdp.xml new file mode 100644 index 000000000000..1331aad06e54 --- /dev/null +++ b/tests/genericxml2xmlindata/cachetune-cdp.xml @@ -0,0 +1,36 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>2</vcpu> + <cputune> + <cachetune vcpus='0-1'> + <cache id='0' level='3' type='code' size='7680' unit='KiB'/> + <cache id='1' level='3' type='data' size='3840' unit='KiB'/> + </cachetune> + <cachetune vcpus='2'> + <cache id='1' level='3' type='code' size='6' unit='MiB'/> + </cachetune> + <cachetune vcpus='3'> + <cache id='1' level='3' type='data' size='6912' unit='KiB'/> + </cachetune>
But there's only 2 vcpu's on this guest?
+ </cputune> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='virtio'/> + </devices> +</domain>
[...]
diff --git a/tests/genericxml2xmlindata/cachetune.xml b/tests/genericxml2xmlindata/cachetune.xml new file mode 100644 index 000000000000..0051410aec32 --- /dev/null +++ b/tests/genericxml2xmlindata/cachetune.xml @@ -0,0 +1,33 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>2</vcpu> + <cputune> + <cachetune vcpus='0-1'> + <cache id='0' level='3' type='both' size='3' unit='MiB'/> + <cache id='1' level='3' type='both' size='3' unit='MiB'/> + </cachetune> + <cachetune vcpus='3'> + <cache id='0' level='3' type='both' size='3' unit='MiB'/> + </cachetune>
Another one with vcpusid > nvcpus defined above.... I'll give you my: Reviewed-by: John Ferlan <jferlan@redhat.com> Since adjustments are simple going forward. I'm not sure how you should handle the vcpusid value > nvcpus if that's an issue or not... John
+ </cputune> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='virtio'/> + </devices> +</domain>