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

[Yi Sun <yi.y.sun@xxxxxxxxxxxxxxx>]

On 17-08-31 09:40:37, Roger Pau Monn� wrote:
> 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:
> > > >  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.
> 
libxl_psr_cat_get_info is just introduced and not released yet. But 
libxl_psr_cat_get_l3_info has been released for long time. It declares
'int *nr' and it calls 'libxl_psr_cat_get_info' to do the work. So, there
must be a casting anyway.

What do you suggest? Shall I submit a patch to fix 'libxl_psr_cat_get_info'?
Or, a patch to fix both 'libxl_psr_cat_get_info' and 
'libxl_psr_cat_get_l3_info'?

> Thanks, Roger.

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