Re: [libvirt][RFC PATCH] add a new 'default' option for attribute mode in numatune

On Mon, Nov 23, 2020 at 05:02:31PM +0800, Zhong, Luyao wrote: > > > On 11/20/2020 6:08 PM, Martin Kletzander wrote: >> On Fri, Nov 20, 2020 at 01:17:29PM +0800, Zhong, Luyao wrote: >>> On 11/18/2020 8:42 PM, Martin Kletzander wrote: >>>> So let's finish this sooner rather than later.  Let's remove the >>>> "migratable/movable" attribute and add another mode for the way memory >>>> allocations are going to be treated.  But let's name it something >>>> else than >>>> "default" and let's explain the name properly in man pages and >>>> documentation. >>>> Since naming is hard and names I come up with are usually bad I can >>>> only >>>> suggest >>>> a lowest bottom fallback if we can't come up with anything else =) >>>> Let's >>>> say >>>> something like "restrictive". >>>> >>> I have no better name than yours. So "restrictive" is good I think, I >>> could use it and send out the patch first, then other reviewers might >>> come up with new name otherwise we keep it. >>> >> >> I actually thought of another way yesterday.  What if not specifying >> any >> mode, >> but specifying nodeset means we'll use cpuset.mems, but don't tell >> QEMU?  That >> sounds to me like something that could make sense for both of us and >> could be >> easily explainable in the documentation to your users. >> > > "not specifying any mode" will be treated as and formatted to "strict" > mode in libvirt since the enumeration type is initialed to the first > value: "strict".[1] > > [1]https://github.com/libvirt/libvirt/blob/master/include/libvirt/libvirt-... > > > And I need a place(a variable with new value) to hold my config, right? > Otherwise in libvirt what value can indicates this config? That's one of > the reasons why I have to introduce a new option for mode. > Ah, that enum is exposed in the API, so we need a name for it. And we alse explicitly default to "strict" in the parsing code. Oh well, I guess we have to go with a new name then. I'm not against that, I just wanted to make it more straight-forward. Feel free to go with whatever name, it can be adjusted before pushing it if there are better ideas. Thanks for noticing and letting me know.

> > >> Basically just like your patch, but "no mode" means "default mode" and >> without >> any "migratable" or anything.  We might need to figure out how to >> deal with >> virsh parameters, but that should be easy to do. >> >> Thanks for your patience ;) >