Re: [libvirt] [PATCHv7 12/18] conf: Introduce cache monitor element in cachetune
On 2018年11月06日 01:26, John Ferlan wrote:
On 10/22/18 4:01 AM, Wang Huaqiang wrote:
Introducing element under to represent
a cache monitor.
Signed-off-by: Wang Huaqiang
---
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 @@
+ monitor
+
+The optional element monitor creates the cache
+monitor(s) for current cache allocation and has the following
+required attributes:
+
+ level
+
+Host cache level the monitor belongs to.
+
+ vcpus
+
+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.
+
+
+
[...]
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 element or element in , 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, "
So is this because when 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
John
Thanks for fixing and the review, I will make changes in my code.
Huaqiang
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv7 12/18] conf: Introduce cache monitor element in cachetune
On 10/22/18 4:01 AM, Wang Huaqiang wrote:
> Introducing element under to represent
> a cache monitor.
>
> Signed-off-by: Wang Huaqiang
> ---
> 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 @@
>
>
>
> + monitor
> +
> +The optional element monitor creates the cache
> +monitor(s) for current cache allocation and has the following
> +required attributes:
> +
> + level
> +
> +Host cache level the monitor belongs to.
> +
> + vcpus
> +
> +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
> +allocation. The default monitor has the same vCPU list as the
> +associating allocation. For non-default monitors, there
associated
> +are no vCPU overlap permitted.
For non-default monitors, overlapping vCPUs are not permitted.
> +
> +
> +
>
>
>
[...]
> 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 element or element in , 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 &&
> 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, " +
> +if (tag == VIR_RESCTRL_MONITOR_TYPE_CACHE) {
> +virBufferAsprintf(buf, "level='%u' ",
> + VIR_DOMAIN_RESCTRL_MONITOR_CACHELEVEL);
> +}
So is this because when is introduced it won't use a cache
level, right? Just trying to recall (and perhaps add a comment).
> +
> +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...
Reviewed-by: John Ferlan
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv7 12/18] conf: Introduce cache monitor element in cachetune
Introducing element under to represent
a cache monitor.
Signed-off-by: Wang Huaqiang
---
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 @@
+ monitor
+
+The optional element monitor creates the cache
+monitor(s) for current cache allocation and has the following
+required attributes:
+
+ level
+
+Host cache level the monitor belongs to.
+
+ vcpus
+
+vCPU list the monitor applies to. A monitor's vCPU list
+can only be the member(s) of the vCPU list of associating
+allocation. The default monitor has the same vCPU list as the
+associating allocation. For non-default monitors, there
+are no vCPU overlap permitted.
+
+
+
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 5c533d6..7ce49d3 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -981,6 +981,16 @@
+
+
+
+
+
+
+
+
+
+
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)
static void
+virDomainResctrlMonDefFree(virDomainResctrlMonDefPtr domresmon)
+{
+if (!domresmon)
+return;
+
+virBitmapFree(domresmon->vcpus);
+virObjectUnref(domresmon->instance);
+VIR_FREE(domresmon);
+}
+
+
+static void
virDomainResctrlDefFree(virDomainResctrlDefPtr resctrl)
{
+size_t i = 0;
+
if (!resctrl)
return;
+for (i = 0; i < resctrl->nmonitors; i++)
+virDomainResctrlMonDefFree(resctrl->monitors[i]);
+
virObjectUnref(resctrl->alloc);
virBitmapFree(resctrl->vcpus);
+VIR_FREE(resctrl->monitors);
VIR_FREE(resctrl);
}
@@ -18920,6 +18938,177 @@ virDomainCachetuneDefParseCache(xmlXPathContextPtr
ctxt,
}
+/* Checking if the monitor's vcpus is conflicted with existing allocation
+ * and monitors.
+ *
+ * Returns 1 if @vcpus equals to @resctrl->vcpus, then the monitor will
+ * share the underlying resctrl group with @resctrl->alloc. Returns - 1
+ * if any conflict found. Returns 0 if no conflict and @vcpus is not equal
+ * to @resctrl->vcpus.
+ */
+static int
+virDomainResctrlMonValidateVcpus(virDomainResctrlDefPtr resctrl,
+virBitmapPtr vcpus)
+{
+size_t i = 0;
+int vcpu = -1;
+size_t mons_same_alloc_vcpus = 0;
+
+if (virBitmapIsAllClear(vcpus)) {
+virReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("vcpus is empty"));
+return -1;
+}
+
+while ((vcpu = virBitmapNextSetBit(vcpus, vcpu)) >= 0) {
+if (!virBitmapIsBitSet(resctrl->vcpus, vcpu)) {
+virReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("Monitor vcpus conflicts with allocation"));
+return -1;
+}
+}
+
+if (virBitmapEqual(vcpus, resctrl->vcpus))
+return 1;
+
+for (i = 0; i < resctrl->nmonitors; i++) {
+if (virBitmapEqual(resctrl->vcpus, resctrl->monitors[i]->vcpus)) {
+mons_same_alloc_vcpus++;
+continue;
+}
+
+if (virBitmapOverlaps(vcpus, resctrl->monitors[i]->vcpus)) {
+virReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("Monitor
