* [PATCH v6] libxl: provide libxl_bitmap_{or,and} @ 2015-04-15 11:45 Linda Jacobson 2015-04-15 12:45 ` Ian Campbell 0 siblings, 1 reply; 14+ messages in thread From: Linda Jacobson @ 2015-04-15 11:45 UTC (permalink / raw) To: xen-devel Cc: wei.liu2, ian.campbell, stefano.stabellini, julien.grall, ian.jackson, lindaj There are new functions to provide logical and and or of two bitmaps. Signed-off-by: Linda Jacobson <lindaj@jma3.com> --- v.1 The new functions were added. v.2 The comments and format were corrected. v.3 The bitmap functions were rewritten to manipulate bytes not bits. v.4 Several non-modified parameters, and local variables were changed to const Also the code formatting was fixed. v.5 The commit subject line now has versions and is simpler. v.6 All descriptions in the commit history are now complete sentences. Extraneous blank lines are gone. --- tools/libxl/libxl_utils.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++ tools/libxl/libxl_utils.h | 7 +++++ 2 files changed, 77 insertions(+) diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c index 9053b27..f552700 100644 --- a/tools/libxl/libxl_utils.c +++ b/tools/libxl/libxl_utils.c @@ -691,6 +691,76 @@ void libxl_bitmap_reset(libxl_bitmap *bitmap, int bit) bitmap->map[bit / 8] &= ~(1 << (bit & 7)); } +int libxl_bitmap_or(libxl_ctx *ctx, libxl_bitmap *or_map, + const libxl_bitmap *map1, const libxl_bitmap *map2) +{ + GC_INIT(ctx); + int rc; + uint32_t i; + const libxl_bitmap *large_map; + const libxl_bitmap *small_map; + + if (map1->size > map2->size) { + large_map = map1; + small_map = map2; + } else { + large_map = map2; + small_map = map1; + } + + rc = libxl_bitmap_alloc(ctx, or_map, large_map->size * 8); + if (rc) + goto out; + + /* + * If bitmaps aren't the same size, their union (logical or) will + * be size of larger bit map. Any bit past the end of the + * smaller bit map, will match the larger one. + */ + for (i = 0; i < small_map->size; i++) + or_map->map[i] = (small_map->map[i] | large_map->map[i]); + + for (i = small_map->size; i < large_map->size; i++) + or_map->map[i] = large_map->map[i]; + +out: + GC_FREE; + return rc; +} + +int libxl_bitmap_and(libxl_ctx *ctx, libxl_bitmap *and_map, + const libxl_bitmap *map1, const libxl_bitmap *map2) +{ + GC_INIT(ctx); + int rc; + uint32_t i; + const libxl_bitmap *large_map; + const libxl_bitmap *small_map; + + if (map1->size > map2->size) { + large_map = map1; + small_map = map2; + } else { + large_map = map2; + small_map = map1; + } + + rc = libxl_bitmap_alloc(ctx, and_map, small_map->size * 8); + if (rc) + goto out; + + /* + * If bitmaps aren't same size, their 'and' will be size of + * smaller bit map + */ + for (i = 0; i < and_map->size; i++) + and_map->map[i] = (large_map->map[i] & small_map->map[i]); + +out: + GC_FREE; + return rc; +} + int libxl_bitmap_count_set(const libxl_bitmap *bitmap) { int i, nr_set_bits = 0; 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 */ +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) { -- 1.9.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v6] libxl: provide libxl_bitmap_{or,and} 2015-04-15 11:45 [PATCH v6] libxl: provide libxl_bitmap_{or,and} Linda Jacobson @ 2015-04-15 12:45 ` Ian Campbell 2015-04-15 13:15 ` Wei Liu 0 siblings, 1 reply; 14+ messages in thread From: Ian Campbell @ 2015-04-15 12:45 UTC (permalink / raw) To: Linda Jacobson Cc: julien.grall, xen-devel, stefano.stabellini, wei.liu2, ian.jackson 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. 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. 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). 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) > { ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v6] libxl: provide libxl_bitmap_{or,and} 2015-04-15 12:45 ` Ian Campbell @ 2015-04-15 13:15 ` Wei Liu 2015-04-15 13:29 ` Ian Campbell 0 siblings, 1 reply; 14+ messages in thread From: Wei Liu @ 2015-04-15 13:15 UTC (permalink / raw) To: Ian Campbell Cc: wei.liu2, stefano.stabellini, julien.grall, ian.jackson, xen-devel, Linda Jacobson 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) > > { > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v6] libxl: provide libxl_bitmap_{or,and} 2015-04-15 13:15 ` Wei Liu @ 2015-04-15 13:29 ` Ian Campbell 2015-04-15 13:41 ` Wei Liu 0 siblings, 1 reply; 14+ messages in thread From: Ian Campbell @ 2015-04-15 13:29 UTC (permalink / raw) To: Wei Liu Cc: julien.grall, xen-devel, ian.jackson, stefano.stabellini, Linda Jacobson 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". > > 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? 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) > > > { > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v6] libxl: provide libxl_bitmap_{or,and} 2015-04-15 13:29 ` Ian Campbell @ 2015-04-15 13:41 ` Wei Liu 2015-04-15 13:52 ` Linda Jacobson ` (2 more replies) 0 siblings, 3 replies; 14+ messages in thread From: Wei Liu @ 2015-04-15 13:41 UTC (permalink / raw) To: Ian Campbell Cc: Wei Liu, stefano.stabellini, julien.grall, ian.jackson, xen-devel, Linda Jacobson 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) > > > > { > > > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v6] libxl: provide libxl_bitmap_{or,and} 2015-04-15 13:41 ` Wei Liu @ 2015-04-15 13:52 ` Linda Jacobson 2015-04-15 15:13 ` Linda 2015-04-15 16:38 ` Linda 2 siblings, 0 replies; 14+ messages in thread From: Linda Jacobson @ 2015-04-15 13:52 UTC (permalink / raw) Cc: Wei Liu, Ian Campbell, <stefano.stabellini@eu.citrix.com>, <julien.grall@citrix.com>, <ian.jackson@citrix.com>, <xen-devel@lists.xenproject.org> Sent from my iPhone > On Apr 15, 2015, at 7:41 AM, Wei Liu <wei.liu2@citrix.com> 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. Of course L > >>>> 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) >>>>> { >> ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v6] libxl: provide libxl_bitmap_{or,and} 2015-04-15 13:41 ` Wei Liu 2015-04-15 13:52 ` Linda Jacobson @ 2015-04-15 15:13 ` Linda 2015-04-15 15:50 ` Wei Liu 2015-04-15 16:38 ` Linda 2 siblings, 1 reply; 14+ messages in thread From: Linda @ 2015-04-15 15:13 UTC (permalink / raw) To: Wei Liu, Ian Campbell Cc: julien.grall, xen-devel, stefano.stabellini, ian.jackson When adding the HAVE macro, is there a protocol on where in libxl.h, this one should be placed? They appear to be fairly spread out through the file. 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) >>>>> { ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v6] libxl: provide libxl_bitmap_{or,and} 2015-04-15 15:13 ` Linda @ 2015-04-15 15:50 ` Wei Liu 2015-04-15 15:54 ` Ian Campbell 0 siblings, 1 reply; 14+ messages in thread From: Wei Liu @ 2015-04-15 15:50 UTC (permalink / raw) To: Linda Cc: Wei Liu, Ian Campbell, stefano.stabellini, julien.grall, ian.jackson, xen-devel On Wed, Apr 15, 2015 at 09:13:51AM -0600, Linda wrote: > When adding the HAVE macro, is there a protocol on where in libxl.h, this > one should be placed? They appear to be fairly spread out through the file. > You can place it near the top. Wei. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v6] libxl: provide libxl_bitmap_{or,and} 2015-04-15 15:50 ` Wei Liu @ 2015-04-15 15:54 ` Ian Campbell 2015-04-15 16:26 ` Linda 0 siblings, 1 reply; 14+ messages in thread From: Ian Campbell @ 2015-04-15 15:54 UTC (permalink / raw) To: Wei Liu; +Cc: julien.grall, xen-devel, ian.jackson, stefano.stabellini, Linda On Wed, 2015-04-15 at 16:50 +0100, Wei Liu wrote: > On Wed, Apr 15, 2015 at 09:13:51AM -0600, Linda wrote: > > When adding the HAVE macro, is there a protocol on where in libxl.h, this > > one should be placed? They appear to be fairly spread out through the file. > > > > You can place it near the top. But not right at the top! Just stick it next to an arbitrary existing one. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v6] libxl: provide libxl_bitmap_{or,and} 2015-04-15 15:54 ` Ian Campbell @ 2015-04-15 16:26 ` Linda 0 siblings, 0 replies; 14+ messages in thread From: Linda @ 2015-04-15 16:26 UTC (permalink / raw) To: Ian Campbell, Wei Liu Cc: julien.grall, xen-devel, stefano.stabellini, ian.jackson On 4/15/2015 9:54 AM, Ian Campbell wrote: > On Wed, 2015-04-15 at 16:50 +0100, Wei Liu wrote: >> On Wed, Apr 15, 2015 at 09:13:51AM -0600, Linda wrote: >>> When adding the HAVE macro, is there a protocol on where in libxl.h, this >>> one should be placed? They appear to be fairly spread out through the file. >>> >> You can place it near the top. > But not right at the top! Just stick it next to an arbitrary existing > one. Will do. Linda Jacobson > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v6] libxl: provide libxl_bitmap_{or,and} 2015-04-15 13:41 ` Wei Liu 2015-04-15 13:52 ` Linda Jacobson 2015-04-15 15:13 ` Linda @ 2015-04-15 16:38 ` Linda 2015-04-15 16:43 ` Wei Liu 2015-04-15 16:44 ` Ian Campbell 2 siblings, 2 replies; 14+ messages in thread From: Linda @ 2015-04-15 16:38 UTC (permalink / raw) To: Wei Liu, Ian Campbell Cc: julien.grall, xen-devel, stefano.stabellini, ian.jackson 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) >>>>> { ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v6] libxl: provide libxl_bitmap_{or,and} 2015-04-15 16:38 ` Linda @ 2015-04-15 16:43 ` Wei Liu 2015-04-15 16:48 ` Linda 2015-04-15 16:44 ` Ian Campbell 1 sibling, 1 reply; 14+ messages in thread From: Wei Liu @ 2015-04-15 16:43 UTC (permalink / raw) To: Linda Cc: Wei Liu, Ian Campbell, stefano.stabellini, julien.grall, ian.jackson, xen-devel On Wed, Apr 15, 2015 at 10:38:19AM -0600, Linda wrote: > BTW who changes the configure file to test for the HAVE_ macros? > The user (application) of libxl should test that. We don't need to worry about the test. Wei. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v6] libxl: provide libxl_bitmap_{or,and} 2015-04-15 16:43 ` Wei Liu @ 2015-04-15 16:48 ` Linda 0 siblings, 0 replies; 14+ messages in thread From: Linda @ 2015-04-15 16:48 UTC (permalink / raw) To: Wei Liu Cc: julien.grall, xen-devel, stefano.stabellini, Ian Campbell, ian.jackson On 4/15/2015 10:43 AM, Wei Liu wrote: > On Wed, Apr 15, 2015 at 10:38:19AM -0600, Linda wrote: >> BTW who changes the configure file to test for the HAVE_ macros? >> > The user (application) of libxl should test that. We don't need to worry > about the test. Thanks. Linda > > Wei. > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v6] libxl: provide libxl_bitmap_{or,and} 2015-04-15 16:38 ` Linda 2015-04-15 16:43 ` Wei Liu @ 2015-04-15 16:44 ` Ian Campbell 1 sibling, 0 replies; 14+ messages in thread From: Ian Campbell @ 2015-04-15 16:44 UTC (permalink / raw) To: Linda; +Cc: julien.grall, xen-devel, ian.jackson, Wei Liu, stefano.stabellini On Wed, 2015-04-15 at 10:38 -0600, Linda wrote: Please stop top posting. > BTW who changes the configure file to test for the HAVE_ macros? Nobody, this is purely for the benefit of external callers of the library. No code in xen.git should ever check this #define (with a few exceptions which do not apply here). Ian. ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2015-04-15 16:48 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-04-15 11:45 [PATCH v6] libxl: provide libxl_bitmap_{or,and} Linda Jacobson 2015-04-15 12:45 ` Ian Campbell 2015-04-15 13:15 ` Wei Liu 2015-04-15 13:29 ` Ian Campbell 2015-04-15 13:41 ` Wei Liu 2015-04-15 13:52 ` Linda Jacobson 2015-04-15 15:13 ` Linda 2015-04-15 15:50 ` Wei Liu 2015-04-15 15:54 ` Ian Campbell 2015-04-15 16:26 ` Linda 2015-04-15 16:38 ` Linda 2015-04-15 16:43 ` Wei Liu 2015-04-15 16:48 ` Linda 2015-04-15 16:44 ` Ian Campbell
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.