Re: [Xen-devel] [PATCH v6] libxl: provide libxl_bitmap_{or,and}

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Wed, Apr 15, 2015 at 01:45:14PM +0100, Ian Campbell wrote:
> On Wed, 2015-04-15 at 05:45 -0600, Linda Jacobson wrote:
> > There are new functions to provide logical and and or of two bitmaps.
> 
> Please could you add a sentence or two on the intended use of these
> functions, since there are no callers being added here.
> 

Linda is our Outreachy applicant. This is a small task that Julien and I
assigned to her.

One user I can think of is in some of the vNUMA validation functions
that operate on bitmaps. But to keep this task small and simple I didn't
ask her to actually use the functions she introduce.

> In particular without that I can't tell if these need to be part of the
> public API or if they are going to be used by something internal.
> 

I think these functions should be public functions.

> If the former then I think a LIBXL_HAVE_... #define is needed in libxl.h
> (as described by the comments in there, and there are many examples).
> 

We're a bit lax on these functions (there is no LIBXL_HAVE_BITMAP...) so
I didn't ask her to add one. We can add LIBXL_HAVE_BITMAP_AND_OR in
libxl.h if you think it is necessary.

Wei.

> If the latter then the prototype should differ. (I'll explain that
> if/when this turns out to be the case).
> 
> > diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
> > index acacdd9..a128a7c 100644
> > --- a/tools/libxl/libxl_utils.h
> > +++ b/tools/libxl/libxl_utils.h
> > @@ -90,6 +90,13 @@ int libxl_bitmap_test(const libxl_bitmap *bitmap, int 
> > bit);
> >  void libxl_bitmap_set(libxl_bitmap *bitmap, int bit);
> >  void libxl_bitmap_reset(libxl_bitmap *bitmap, int bit);
> >  int libxl_bitmap_count_set(const libxl_bitmap *cpumap);
> > +/* Or and and functions for two bitmaps */
> 
> This comment doesn't say anything which isn't already apparent from the
> names and prototypes of the functions in question. Just leave it out.
> 
> 
> > +int libxl_bitmap_or(libxl_ctx *ctx, libxl_bitmap *or_map,
> > +                    const libxl_bitmap *map1, 
> > +                    const libxl_bitmap *map2); 
> > +int libxl_bitmap_and(libxl_ctx *ctx, libxl_bitmap *and_map,
> > +                     const libxl_bitmap *map1, 
> > +                     const libxl_bitmap *map2);
> >  char *libxl_bitmap_to_hex_string(libxl_ctx *ctx, const libxl_bitmap 
> > *cpumap);
> >  static inline void libxl_bitmap_set_any(libxl_bitmap *bitmap)
> >  {
> 

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