Re: [libvirt] [PATCH 6/9] conf: Rename cachetune to restune
-----Original Message-----
From: John Ferlan [mailto:jferlan@redhat.com]
Sent: Friday, July 27, 2018 12:33 AM
To: Niu, Bing <bing.niu(a)intel.com>; libvir-list(a)redhat.com
Cc: Feng, Shaohe <shaohe.feng(a)intel.com>; Wang, Huaqiang
<huaqiang.wang(a)intel.com>; Ding, Jian-feng <jian-feng.ding(a)intel.com>;
rui.zang(a)yandex.com
Subject: Re: [libvirt] [PATCH 6/9] conf: Rename cachetune to restune
On 07/18/2018 03:57 AM, bing.niu(a)intel.com wrote:
> From: Bing Niu <bing.niu(a)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(a)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
[...]