Re: [Xen-devel] [PATCH v2 08/15] tools: create general interfaces to support psr allocation features

[Roger Pau Monn� <roger.pau@xxxxxxxxxx>]

On Thu, Aug 31, 2017 at 10:38:46AM +0800, Yi Sun wrote:
> On 17-08-30 09:42:56, Roger Pau Monn� wrote:
> > On Thu, Aug 24, 2017 at 09:14:42AM +0800, Yi Sun wrote:
> > > This patch creates general interfaces in libxl to support all psr
> > > allocation features.
> > > 
> > > Add 'LIBXL_HAVE_PSR_MBA' to indicate interface change.
> > 
> > I'm not sure this is enough to cover the changes you are doing here:
> > you are introducing some MBA stuff, plus a kind of generic interface
> > for PSR.
> > 
> > I think this should be split into two patches, the first one adding
> > the generic interface, and the second one adding the MBA stuff.
> > 
> This patch only introduces the generic interfaces without any MBA stuff.
> 
> The 'LIBXL_HAVE_PSR_MBA' is used to indicate the interfaces change here.
> Per my understand, we should add a macro to indicate libxl interfaces
> change, right?

So if there's no MBA specific interfaces introduced, why name the
define PSR_MBA? Maybe PSR_GENERIC or whatever you find more suitable.

> > > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> > > index 6e80d36..ab847f8 100644
> > > --- a/tools/libxl/libxl_types.idl
> > > +++ b/tools/libxl/libxl_types.idl
> > > @@ -977,6 +977,7 @@ libxl_psr_cbm_type = Enumeration("psr_cbm_type", [
> > >      (2, "L3_CBM_CODE"),
> > >      (3, "L3_CBM_DATA"),
> > >      (4, "L2_CBM"),
> > > +    (5, "MBA_THRTL"),
> > 
> > Is this really a CBM type?
> > 
> This is not CBM type. The 'libxl_psr_cbm_type' name is not good enough. But I
> have to introduce a new generic interface here if we want to make the name be
> generic. I think it is not so valuable. So, I reuse the 'libxl_psr_cbm_type'
> to cover MBA. How do you think?

Maybe you could introduce a new typedef, so that old code call still
work, ie:

typedef enum libxl_psr_cbm_type libxl_psr_type;

(Or whatever name you find suitable).

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel