From: Bing Niu <bing.niu@intel.com>
Resctrl not only supports cache tuning, but also memory bandwidth
tuning. Renaming cachetune to restune(resource tuning) to reflect
that. With restune, all allocation for different resources (cache,
memory bandwidth) are aggregated and represented by a
virResctrlAllocPtr inside virDomainRestuneDef.
Signed-off-by: Bing Niu <bing.niu@intel.com>
---
src/conf/domain_conf.c | 44 ++++++++++++++++++++++----------------------
src/conf/domain_conf.h | 10 +++++-----
src/qemu/qemu_domain.c | 2 +-
src/qemu/qemu_process.c | 18 +++++++++---------
4 files changed, 37 insertions(+), 37 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f259b4c..24fefd1 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2963,14 +2963,14 @@ virDomainLoaderDefFree(virDomainLoaderDefPtr loader)
static void
-virDomainCachetuneDefFree(virDomainCachetuneDefPtr cachetune)
+virDomainRestuneDefFree(virDomainRestuneDefPtr restune)
{
- if (!cachetune)
+ if (!restune)
return;
- virObjectUnref(cachetune->alloc);
- virBitmapFree(cachetune->vcpus);
- VIR_FREE(cachetune);
+ virObjectUnref(restune->alloc);
+ virBitmapFree(restune->vcpus);
+ VIR_FREE(restune);
}
@@ -3160,9 +3160,9 @@ 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);
+ for (i = 0; i < def->nrestunes; i++)
+ virDomainRestuneDefFree(def->restunes[i]);
+ VIR_FREE(def->restunes);
VIR_FREE(def->keywrap);
@@ -18967,7 +18967,7 @@ virDomainCachetuneDefParse(virDomainDefPtr def,
xmlNodePtr *nodes = NULL;
virBitmapPtr vcpus = NULL;
virResctrlAllocPtr alloc = virResctrlAllocNew();
- virDomainCachetuneDefPtr tmp_cachetune = NULL;
+ virDomainRestuneDefPtr tmp_restune = NULL;
char *tmp = NULL;
char *vcpus_str = NULL;
char *alloc_id = NULL;
@@ -18980,7 +18980,7 @@ virDomainCachetuneDefParse(virDomainDefPtr def,
if (!alloc)
goto cleanup;
- if (VIR_ALLOC(tmp_cachetune) < 0)
+ if (VIR_ALLOC(tmp_restune) < 0)
goto cleanup;
vcpus_str = virXMLPropString(node, "vcpus");
@@ -19021,8 +19021,8 @@ virDomainCachetuneDefParse(virDomainDefPtr def,
goto cleanup;
}
- for (i = 0; i < def->ncachetunes; i++) {
- if (virBitmapOverlaps(def->cachetunes[i]->vcpus, vcpus)) {
+ for (i = 0; i < def->nrestunes; i++) {
+ if (virBitmapOverlaps(def->restunes[i]->vcpus, vcpus)) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("Overlapping vcpus in cachetunes"));
goto cleanup;
@@ -19052,16 +19052,16 @@ virDomainCachetuneDefParse(virDomainDefPtr def,
if (virResctrlAllocSetID(alloc, alloc_id) < 0)
goto cleanup;
- VIR_STEAL_PTR(tmp_cachetune->vcpus, vcpus);
- VIR_STEAL_PTR(tmp_cachetune->alloc, alloc);
+ VIR_STEAL_PTR(tmp_restune->vcpus, vcpus);
+ VIR_STEAL_PTR(tmp_restune->alloc, alloc);
- if (VIR_APPEND_ELEMENT(def->cachetunes, def->ncachetunes, tmp_cachetune) < 0)
+ if (VIR_APPEND_ELEMENT(def->restunes, def->nrestunes, tmp_restune) < 0)
goto cleanup;
ret = 0;
cleanup:
ctxt->node = oldnode;
- virDomainCachetuneDefFree(tmp_cachetune);
+ virDomainRestuneDefFree(tmp_restune);
virObjectUnref(alloc);
virBitmapFree(vcpus);
VIR_FREE(alloc_id);
@@ -26913,7 +26913,7 @@ virDomainCachetuneDefFormatHelper(unsigned int level,
static int
virDomainCachetuneDefFormat(virBufferPtr buf,
- virDomainCachetuneDefPtr cachetune,
+ virDomainRestuneDefPtr restune,
unsigned int flags)
{
virBuffer childrenBuf = VIR_BUFFER_INITIALIZER;
@@ -26921,7 +26921,7 @@ virDomainCachetuneDefFormat(virBufferPtr buf,
int ret = -1;
virBufferSetChildIndent(&childrenBuf, buf);
- virResctrlAllocForeachCache(cachetune->alloc,
+ virResctrlAllocForeachCache(restune->alloc,
virDomainCachetuneDefFormatHelper,
&childrenBuf);
@@ -26934,14 +26934,14 @@ virDomainCachetuneDefFormat(virBufferPtr buf,
goto cleanup;
}
- vcpus = virBitmapFormat(cachetune->vcpus);
+ vcpus = virBitmapFormat(restune->vcpus);
if (!vcpus)
goto cleanup;
virBufferAsprintf(buf, "<cachetune vcpus='%s'", vcpus);
if (!(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE)) {
- const char *alloc_id = virResctrlAllocGetID(cachetune->alloc);
+ const char *alloc_id = virResctrlAllocGetID(restune->alloc);
if (!alloc_id)
goto cleanup;
@@ -27062,8 +27062,8 @@ virDomainCputuneDefFormat(virBufferPtr buf,
def->iothreadids[i]->iothread_id);
}
- for (i = 0; i < def->ncachetunes; i++)
- virDomainCachetuneDefFormat(&childrenBuf, def->cachetunes[i], flags);
+ for (i = 0; i < def->nrestunes; i++)
+ virDomainCachetuneDefFormat(&childrenBuf, def->restunes[i], flags);
if (virBufferCheckError(&childrenBuf) < 0)
return -1;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 0f10e24..64920fa 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2229,10 +2229,10 @@ struct _virDomainCputune {
};
-typedef struct _virDomainCachetuneDef virDomainCachetuneDef;
-typedef virDomainCachetuneDef *virDomainCachetuneDefPtr;
+typedef struct _virDomainRestuneDef virDomainRestuneDef;
+typedef virDomainRestuneDef *virDomainRestuneDefPtr;
-struct _virDomainCachetuneDef {
+struct _virDomainRestuneDef {
virBitmapPtr vcpus;
virResctrlAllocPtr alloc;
};
@@ -2410,8 +2410,8 @@ struct _virDomainDef {
virDomainCputune cputune;
- virDomainCachetuneDefPtr *cachetunes;
- size_t ncachetunes;
+ virDomainRestuneDefPtr *restunes;
+ size_t nrestunes;
virDomainNumaPtr numa;
virDomainResourceDefPtr resource;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index ed76495..a9979ba 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3999,7 +3999,7 @@ qemuDomainDefValidate(const virDomainDef *def,
}
}
- if (def->ncachetunes &&
+ if (def->nrestunes &&
def->virtType != VIR_DOMAIN_VIRT_KVM) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("cachetune is only supported for KVM domains"));
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index c903a8e..233ae94 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -2559,7 +2559,7 @@ qemuProcessResctrlCreate(virQEMUDriverPtr driver,
virCapsPtr caps = NULL;
qemuDomainObjPrivatePtr priv = vm->privateData;
- if (!vm->def->ncachetunes)
+ if (!vm->def->nrestunes)
return 0;
/* Force capability refresh since resctrl info can change
@@ -2568,9 +2568,9 @@ qemuProcessResctrlCreate(virQEMUDriverPtr driver,
if (!caps)
return -1;
- for (i = 0; i < vm->def->ncachetunes; i++) {
+ for (i = 0; i < vm->def->nrestunes; i++) {
if (virResctrlAllocCreate(caps->host.resctrl,
- vm->def->cachetunes[i]->alloc,
+ vm->def->restunes[i]->alloc,
priv->machineName) < 0)
goto cleanup;
}
@@ -5376,8 +5376,8 @@ qemuProcessSetupVcpu(virDomainObjPtr vm,
&vcpu->sched) < 0)
return -1;
- for (i = 0; i < vm->def->ncachetunes; i++) {
- virDomainCachetuneDefPtr ct = vm->def->cachetunes[i];
+ for (i = 0; i < vm->def->nrestunes; i++) {
+ virDomainRestuneDefPtr ct = vm->def->restunes[i];
if (virBitmapIsBitSet(ct->vcpus, vcpuid)) {
if (virResctrlAllocAddPID(ct->alloc, vcpupid) < 0)
@@ -7077,8 +7077,8 @@ void qemuProcessStop(virQEMUDriverPtr driver,
/* Remove resctrl allocation after cgroups are cleaned up which makes it
* kind of safer (although removing the allocation should work even with
* pids in tasks file */
- for (i = 0; i < vm->def->ncachetunes; i++)
- virResctrlAllocRemove(vm->def->cachetunes[i]->alloc);
+ for (i = 0; i < vm->def->nrestunes; i++)
+ virResctrlAllocRemove(vm->def->restunes[i]->alloc);
qemuProcessRemoveDomainStatus(driver, vm);
@@ -7801,8 +7801,8 @@ qemuProcessReconnect(void *opaque)
if (qemuConnectAgent(driver, obj) < 0)
goto error;
- for (i = 0; i < obj->def->ncachetunes; i++) {
- if (virResctrlAllocDeterminePath(obj->def->cachetunes[i]->alloc,
+ for (i = 0; i < obj->def->nrestunes; i++) {
+ if (virResctrlAllocDeterminePath(obj->def->restunes[i]->alloc,
priv->machineName) < 0)
goto error;
}
--
2.7.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 07/18/2018 03:57 AM, bing.niu@intel.com wrote:
> From: Bing Niu <bing.niu@intel.com>
>
> Resctrl not only supports cache tuning, but also memory bandwidth
> tuning. Renaming cachetune to restune(resource tuning) to reflect
> that. With restune, all allocation for different resources (cache,
> memory bandwidth) are aggregated and represented by a
> virResctrlAllocPtr inside virDomainRestuneDef.
>
> Signed-off-by: Bing Niu <bing.niu@intel.com>
> ---
> src/conf/domain_conf.c | 44 ++++++++++++++++++++++----------------------
> src/conf/domain_conf.h | 10 +++++-----
> src/qemu/qemu_domain.c | 2 +-
> src/qemu/qemu_process.c | 18 +++++++++---------
> 4 files changed, 37 insertions(+), 37 deletions(-)
>
As I noted previously, not much a fan of Restune instead of Cachetune,
but I understand the logic why you went that way.
I wonder if "virDomainResAllocDef" is any better (resallocs,
nresallocs)? or if that clashes with any other namespace so far? or is
too close to virResctrlAllocPtr.
Or perhaps "virDomainResCtrlDef" w/ resctrls and nresctrls to mimic the
virresctrl.{c,h} naming scheme.
As previously stated, "Naming is hard"... Wish there was more feedback
than just me on this, but in the long run, I'll go with whatever the
Intel team agrees upon as it's not that big a deal. If someone else has
agita after things are pushed and wants to change the name, then they
know how to send patches.
John
[...]
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
> -----Original Message-----
> From: John Ferlan [mailto:jferlan@redhat.com]
> Sent: Friday, July 27, 2018 12:33 AM
> To: Niu, Bing <bing.niu@intel.com>; libvir-list@redhat.com
> Cc: Feng, Shaohe <shaohe.feng@intel.com>; Wang, Huaqiang
> <huaqiang.wang@intel.com>; Ding, Jian-feng <jian-feng.ding@intel.com>;
> rui.zang@yandex.com
> Subject: Re: [libvirt] [PATCH 6/9] conf: Rename cachetune to restune
>
>
>
> On 07/18/2018 03:57 AM, bing.niu@intel.com wrote:
> > From: Bing Niu <bing.niu@intel.com>
> >
> > Resctrl not only supports cache tuning, but also memory bandwidth
> > tuning. Renaming cachetune to restune(resource tuning) to reflect
> > that. With restune, all allocation for different resources (cache,
> > memory bandwidth) are aggregated and represented by a
> > virResctrlAllocPtr inside virDomainRestuneDef.
> >
> > Signed-off-by: Bing Niu <bing.niu@intel.com>
> > ---
> > src/conf/domain_conf.c | 44
> > ++++++++++++++++++++++----------------------
> > src/conf/domain_conf.h | 10 +++++----- src/qemu/qemu_domain.c | 2
> > +- src/qemu/qemu_process.c | 18 +++++++++---------
> > 4 files changed, 37 insertions(+), 37 deletions(-)
> >
>
> As I noted previously, not much a fan of Restune instead of Cachetune, but I
> understand the logic why you went that way.
>
> I wonder if "virDomainResAllocDef" is any better (resallocs, nresallocs)? or if
> that clashes with any other namespace so far? or is too close to
> virResctrlAllocPtr.
>From Huaqiang:
Hi Bing and John, this is Huaqiang and I am working on preparing libvirt patches
to support cache monitoring (CMT) feature and memory bandwidth monitoring
feature(MBM).
I am wondering if we can consider making a further step of renaming of
'virDomainResAllocDef' in the future to accommodate RDT monitoring
feature (CMT and MBM):
Using 'virDomainResDef' or 'virDomainCPUResDef' to substitute
'virDomainResAllocDef'? My considerations are below:
- 'cache tune', 'memory bw tune' and the corresponding monitoring technologies
are integrated in same underlying CPU resource control group (aka: resctrlfs group).
They are sharing interfaces exposed interfaces of same resctrlfs subdirectory.
- If we make a rename to 'virDomainResDef'/'virDomainCPUResDef', this data
struct could be used for CMT/MBM feature in a very straightforward way.
- And I prefer 'virDomainCPUResDef' than the other one which make it clear
that it is dealing with CPU resources other than network or disk resources.
I am also in thinking about raising a refactor patch in the future to rename
'virResctrlAllocPtr' to ' virResctrlPtr'(or virResctrlGroupPtr, still in figuring out
an appropriate name...). Which will make it possible to reuse some of code
existing in file virresctrl.c/h now with similar reasons I talked above.
>
> Or perhaps "virDomainResCtrlDef" w/ resctrls and nresctrls to mimic the
> virresctrl.{c,h} naming scheme.
>
> As previously stated, "Naming is hard"... Wish there was more feedback than
> just me on this, but in the long run, I'll go with whatever the Intel team agrees
> upon as it's not that big a deal. If someone else has agita after things are pushed
> and wants to change the name, then they know how to send patches.
>
> John
>
> [...]
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
> -----Original Message-----
> From: John Ferlan [mailto:jferlan@redhat.com]
> Sent: Friday, July 27, 2018 12:33 AM
> To: Niu, Bing <bing.niu@intel.com>; libvir-list@redhat.com
> Cc: Feng, Shaohe <shaohe.feng@intel.com>; Wang, Huaqiang
> <huaqiang.wang@intel.com>; Ding, Jian-feng <jian-feng.ding@intel.com>;
> rui.zang@yandex.com
> Subject: Re: [libvirt] [PATCH 6/9] conf: Rename cachetune to restune
>
>
>
> On 07/18/2018 03:57 AM, bing.niu@intel.com wrote:
> > From: Bing Niu <bing.niu@intel.com>
> >
> > Resctrl not only supports cache tuning, but also memory bandwidth
> > tuning. Renaming cachetune to restune(resource tuning) to reflect
> > that. With restune, all allocation for different resources (cache,
> > memory bandwidth) are aggregated and represented by a
> > virResctrlAllocPtr inside virDomainRestuneDef.
> >
> > Signed-off-by: Bing Niu <bing.niu@intel.com>
> > ---
> > src/conf/domain_conf.c | 44
> > ++++++++++++++++++++++----------------------
> > src/conf/domain_conf.h | 10 +++++----- src/qemu/qemu_domain.c | 2
> > +- src/qemu/qemu_process.c | 18 +++++++++---------
> > 4 files changed, 37 insertions(+), 37 deletions(-)
> >
>
> As I noted previously, not much a fan of Restune instead of Cachetune, but I
> understand the logic why you went that way.
>
> I wonder if "virDomainResAllocDef" is any better (resallocs, nresallocs)? or if
> that clashes with any other namespace so far? or is too close to
> virResctrlAllocPtr.
>
> Or perhaps "virDomainResCtrlDef" w/ resctrls and nresctrls to mimic the
> virresctrl.{c,h} naming scheme.
>
Hi John,
I have made a comment without reading above line...
Yes! This is what I want to change for the naming. 'virDomainResCtrlDef' looks
Ok for me.
> As previously stated, "Naming is hard"... Wish there was more feedback than
> just me on this, but in the long run, I'll go with whatever the Intel team agrees
> upon as it's not that big a deal. If someone else has agita after things are pushed
> and wants to change the name, then they know how to send patches.
>
> John
>
> [...]
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 2018年07月27日 00:32, John Ferlan wrote:
>
>
> On 07/18/2018 03:57 AM, bing.niu@intel.com wrote:
>> From: Bing Niu <bing.niu@intel.com>
>>
>> Resctrl not only supports cache tuning, but also memory bandwidth
>> tuning. Renaming cachetune to restune(resource tuning) to reflect
>> that. With restune, all allocation for different resources (cache,
>> memory bandwidth) are aggregated and represented by a
>> virResctrlAllocPtr inside virDomainRestuneDef.
>>
>> Signed-off-by: Bing Niu <bing.niu@intel.com>
>> ---
>> src/conf/domain_conf.c | 44 ++++++++++++++++++++++----------------------
>> src/conf/domain_conf.h | 10 +++++-----
>> src/qemu/qemu_domain.c | 2 +-
>> src/qemu/qemu_process.c | 18 +++++++++---------
>> 4 files changed, 37 insertions(+), 37 deletions(-)
>>
>
> As I noted previously, not much a fan of Restune instead of Cachetune,
> but I understand the logic why you went that way.
>
> I wonder if "virDomainResAllocDef" is any better (resallocs,
> nresallocs)? or if that clashes with any other namespace so far? or is
> too close to virResctrlAllocPtr.
>
> Or perhaps "virDomainResCtrlDef" w/ resctrls and nresctrls to mimic the
> virresctrl.{c,h} naming scheme.
virDomainResCtrlDef is better. How about we did one puny adjustment.
virDomainResctrlDef w/ resctrls and nresctrls?
Use little 'c' can align with virresctrl.c function naming. ;)
>
> As previously stated, "Naming is hard"... Wish there was more feedback
> than just me on this, but in the long run, I'll go with whatever the
> Intel team agrees upon as it's not that big a deal. If someone else has
> agita after things are pushed and wants to change the name, then they
> know how to send patches.
>
> John
>
> [...]
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2026 Red Hat, Inc.