4:02 a.m.
On 2018年11月06日 01:26, John Ferlan wrote:
On 10/22/18 4:01 AM, Wang Huaqiang wrote:
> Introducing <monitor> element under <cachetune> to represent
> a cache monitor.
>
> Signed-off-by: Wang Huaqiang <huaqiang.wang(a)intel.com>
> ---
> docs/formatdomain.html.in | 26 +++
> docs/schemas/domaincommon.rng | 10 +
> src/conf/domain_conf.c | 234 ++++++++++++++++++++-
> src/conf/domain_conf.h | 11 +
> tests/genericxml2xmlindata/cachetune-cdp.xml | 3 +
> .../cachetune-colliding-monitor.xml | 30 +++
> tests/genericxml2xmlindata/cachetune-small.xml | 7 +
> tests/genericxml2xmltest.c | 2 +
> 8 files changed, 322 insertions(+), 1 deletion(-)
> create mode 100644 tests/genericxml2xmlindata/cachetune-colliding-monitor.xml
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index b1651e3..2fd665c 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -759,6 +759,12 @@
> <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'/>
> + <monitor level='3' vcpus='1'/>
> + <monitor level='3' vcpus='0-3'/>
> + </cachetune>
> + <cachetune vcpus='4-5'>
> + <monitor level='3' vcpus='4'/>
> + <monitor level='3' vcpus='5'/>
> </cachetune>
> <memorytune vcpus='0-3'>
> <node id='0' bandwidth='60'/>
> @@ -978,6 +984,26 @@
> </dd>
> </dl>
> </dd>
> + <dt><code>monitor</code></dt>
> + <dd>
> + The optional element <code>monitor</code> creates the cache
> + monitor(s) for current cache allocation and has the following
> + required attributes:
> + <dl>
> + <dt><code>level</code></dt>
> + <dd>
> + Host cache level the monitor belongs to.
> + </dd>
> + <dt><code>vcpus</code></dt>
> + <dd>
> + vCPU list the monitor applies to. A monitor's vCPU list
> + can only be the member(s) of the vCPU list of associating
the associated
Thanks.
> + allocation. The default monitor has the same vCPU list as the
> + associating allocation. For non-default monitors, there
associated
Thanks.
> + are no vCPU overlap permitted.
For non-default monitors, overlapping vCPUs are not permitted.
Thanks.
> + </dd>
> + </dl>
> + </dd>
> </dl>
> </dd>
>
[...]
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index a068d4d..01f795f 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -2955,13 +2955,31 @@ virDomainLoaderDefFree(virDomainLoaderDefPtr loader)
>
>
[...]
> @@ -19026,7 +19215,14 @@ virDomainCachetuneDefParse(virDomainDefPtr def,
> if (!resctrl)
> goto cleanup;
>
> - if (virResctrlAllocIsEmpty(alloc)) {
> + if (virDomainResctrlMonDefParse(def, ctxt, node,
> + VIR_RESCTRL_MONITOR_TYPE_CACHE,
> + resctrl) < 0)
> + goto cleanup;
> +
> + /* If no <cache> element or <monitor> element in <cachetune>,
do not
> + * append any resctrl element */
> + if (virResctrlAllocIsEmpty(alloc) && !resctrl->nmonitors) {
Swap order since *IsEmpty is more compute intensive, also change to:
if (resctrl->nmonitors == 0 &&
Agree.
> ret = 0;
> goto cleanup;
> }
> @@ -27063,12 +27259,41 @@ virDomainCachetuneDefFormatHelper(unsigned int level,
>
>
> static int
> +virDomainResctrlMonDefFormatHelper(virDomainResctrlMonDefPtr domresmon,
> + virResctrlMonitorType tag,
> + virBufferPtr buf)
> +{
> + char *vcpus = NULL;
> +
> + if (domresmon->tag != tag)
> + return 0;
> +
> + virBufferAddLit(buf, "<monitor ");
> +
> + if (tag == VIR_RESCTRL_MONITOR_TYPE_CACHE) {
> + virBufferAsprintf(buf, "level='%u' ",
> + VIR_DOMAIN_RESCTRL_MONITOR_CACHELEVEL);
> + }
So is this because when <memtune> is introduced it won't use a cache
level, right? Just trying to recall (and perhaps add a comment).
Yes. 'level' is just for cachetune monitor. Planned to add the comment when
extending this function to support memtune monitor, mentioning the memtune
monitor ( tag == VIR_RESCTRL_MONITOR_TYPE_BW) here will cause confusing.
> +
> + vcpus = virBitmapFormat(domresmon->vcpus);
> + if (!vcpus)
> + return -1;
> +
> + virBufferAsprintf(buf, "vcpus='%s'/>\n", vcpus);
> +
> + VIR_FREE(vcpus);
> + return 0;
> +}
> +
> +
[...]
I can fix the minor nits, just ack them...
Thanks please help fix.
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
Thanks for fixing and the review, I will make changes in my code.
Huaqiang