[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v6] libxl: provide libxl_bitmap_{or,and}
BTW who changes the configure file to test for the HAVE_ macros? Thanks. Linda On 4/15/2015 7:41 AM, Wei Liu wrote: On Wed, Apr 15, 2015 at 02:29:10PM +0100, Ian Campbell wrote:On Wed, 2015-04-15 at 14:15 +0100, Wei Liu wrote: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.Sure, but that doesn't remove the need for the commit log to be a standalone justification for the patch in its own right.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.Sure, but the reasoning for why you^WLinda thinks that needs to be in the commit log. In particular because there are no users being added. I could probably guess why you think these should be public, but I shouldn't have to guess and in any case that doesn't help in 6 months when someone asks "why do we have these functions".Linda, can you use something like this in your commit log: Provide libxl_bitmap_{and,or} functions. These functions can be used in vNUMA configuration check function. They are also generally useful so they are made as public API.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...)Weren't they always there and hence don't need it?OK, maybe I'm mistaken. Linda, you need to add one more hunk to libxl.h. #define LIBXL_HAVE_BITMAP_AND_OR 1 Search for other LIBXL_HAVE_ macro to get an idea how it is handled. If you have any questions, just ask. Wei.Ian.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
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |