From mboxrd@z Thu Jan 1 00:00:00 1970 From: Linda Subject: Re: [PATCH v6] libxl: provide libxl_bitmap_{or,and} Date: Wed, 15 Apr 2015 09:13:51 -0600 Message-ID: <552E802F.2050608@jma3.com> References: <1429098311-30180-1-git-send-email-lindaj@jma3.com> <1429101914.15516.262.camel@citrix.com> <20150415131550.GC8128@zion.uk.xensource.com> <1429104550.15516.287.camel@citrix.com> <20150415134147.GD8128@zion.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1YiP0w-0006mV-Ns for xen-devel@lists.xenproject.org; Wed, 15 Apr 2015 15:14:06 +0000 In-Reply-To: <20150415134147.GD8128@zion.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Wei Liu , Ian Campbell Cc: julien.grall@citrix.com, xen-devel@lists.xenproject.org, stefano.stabellini@eu.citrix.com, ian.jackson@citrix.com List-Id: xen-devel@lists.xenproject.org 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) >>>>> {