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(a)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(a)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>