Re: [Xen-devel] [PATCH v2 10/15] tools: implement the new libxl get hw info interface

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

On Thu, Aug 31, 2017 at 11:16:34AM +0800, Yi Sun wrote:
> On 17-08-30 10:15:22, Roger Pau Monn� wrote:
> > On Thu, Aug 24, 2017 at 09:14:44AM +0800, Yi Sun wrote:
> > > diff --git a/tools/libxl/libxl_psr.c b/tools/libxl/libxl_psr.c
> > > index b183305..d7da7d7 100644
> > > --- a/tools/libxl/libxl_psr.c
> > > +++ b/tools/libxl/libxl_psr.c
> > > @@ -382,56 +382,51 @@ static inline xc_psr_feat_type 
> > > libxl__psr_feat_type_to_libxc_psr_feat_type(
> > >      return xc_type;
> > >  }
> > >  
> > > +static inline int libxl__psr_hw_info_to_libxl_psr_cat_info(
> > 
> > No inline. Maybe you could try to shorter the name?
> > 
> Got it. Will remove the last '_psr'.
> 
> [...]
> 
> > >  int libxl_psr_cat_get_info(libxl_ctx *ctx, libxl_psr_cat_info **info,
> > >                             int *nr, unsigned int lvl)
> > >  {
> > >      GC_INIT(ctx);
> > >      int rc;
> > > -    int i = 0, socketid, nr_sockets;
> > > -    libxl_bitmap socketmap;
> > > +    unsigned int i;
> > > +    libxl_psr_hw_info *hw_info;
> > >      libxl_psr_cat_info *ptr;
> > > -    xc_psr_hw_info hw_info;
> > > -    xc_psr_feat_type xc_type;
> > > -
> > > -    libxl_bitmap_init(&socketmap);
> > > -
> > > -    rc = libxl__count_physical_sockets(gc, &nr_sockets);
> > > -    if (rc) {
> > > -        LOGE(ERROR, "failed to get system socket count");
> > > -        goto out;
> > > -    }
> > >  
> > > -    libxl_socket_bitmap_alloc(ctx, &socketmap, nr_sockets);
> > > -    rc = libxl_get_online_socketmap(ctx, &socketmap);
> > > -    if (rc < 0) {
> > > -        LOGE(ERROR, "failed to get available sockets");
> > > +    rc = libxl_psr_get_hw_info(ctx, &hw_info, (unsigned int *)nr,
> > 
> > Is there any reason nr is int instead of unsigned int?
> > 
> > I would rather avoid casting things. Since this interface has not been
> > present in a release yet, could you please send a separate patch to
> > fix this if nr has no reason to be signed?
> > 
> This is a historical issue.
> 
> The first version of PSR introduced 'libxl_psr_cat_get_l3_info'. The input
> parameter is 'int *nr'.
> 
> I think we cannot change the interface which has been merged and used by
> others. Right?

Has libxl_psr_cat_get_info been present in any Xen release? (I don't
think so) If it hasn't then you can change the interface without
issues.

Thanks, Roger.

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