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@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
> 
> [...]