From mboxrd@z Thu Jan 1 00:00:00 1970 From: George Dunlap Subject: Re: [PATCH v3 11/14] libxl: get and set soft affinity Date: Tue, 19 Nov 2013 15:41:20 +0000 Message-ID: <528B86A0.5000905@eu.citrix.com> References: <20131118175544.31002.79574.stgit@Solace> <20131118181813.31002.61195.stgit@Solace> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20131118181813.31002.61195.stgit@Solace> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Dario Faggioli Cc: Marcus Granado , Keir Fraser , Ian Campbell , Li Yechen , Andrew Cooper , Juergen Gross , Ian Jackson , xen-devel@lists.xen.org, Jan Beulich , Justin Weaver , Matt Wilson , Elena Ufimtseva List-Id: xen-devel@lists.xenproject.org On 11/18/2013 06:18 PM, Dario Faggioli wrote: > Make space for two new cpumap-s, one in vcpu_info (for getting > soft affinity) and build_info (for setting it). Provide two > new API calls: > > * libxl_set_vcpuaffinity2, taking a cpumap and setting either > hard, soft or both affinity to it, depending on 'flags'; > * libxl_set_vcpuaffinity3, taking two cpumap, one for hard > and one for soft affinity. > > The bheavior of the existing libxl_set_vcpuaffinity is left > unchanged, i.e., it only set hard affinity. > > Getting soft affinity happens indirectly, via `xl vcpu-list' > (as it is already for hard affinity). > > The new calls include logic to check whether the affinity which > will be used by Xen to schedule the vCPU(s) does actually match > with the cpumap provided. In fact, we want to allow every possible > combination of hard and soft affinities to be set, but we warn > the user upon particularly weird combinations (e.g., hard and > soft being disjoint sets of pCPUs). > > Also, this is the first change breaking the libxl ABI, so it > bumps the MAJOR. > > Signed-off-by: Dario Faggioli The interface is fine with me (I would probably just have 2 and not 3, but I'm OK with both). Just a few minor comments: > @@ -973,6 +987,22 @@ int libxl_set_vcpuaffinity(libxl_ctx *ctx, uint32_t domid, uint32_t vcpuid, > libxl_bitmap *cpumap); > int libxl_set_vcpuaffinity_all(libxl_ctx *ctx, uint32_t domid, > unsigned int max_vcpus, libxl_bitmap *cpumap); > +int libxl_set_vcpuaffinity2(libxl_ctx *ctx, uint32_t domid, uint32_t vcpuid, > + const libxl_bitmap *cpumap, int flags); > +int libxl_set_vcpuaffinity_all2(libxl_ctx *ctx, uint32_t domid, > + unsigned int max_vcpus, > + const libxl_bitmap *cpumap, int flags); Should we have a bit more documentation about the behavior of "flags" somewhere? > diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h > index b11cf28..fc3afee 100644 > --- a/tools/libxl/libxl_utils.h > +++ b/tools/libxl/libxl_utils.h > @@ -98,6 +98,21 @@ static inline int libxl_bitmap_cpu_valid(libxl_bitmap *bitmap, int bit) > #define libxl_for_each_set_bit(v, m) for (v = 0; v < (m).size * 8; v++) \ > if (libxl_bitmap_test(&(m), v)) > > +static inline int libxl_bitmap_equal(const libxl_bitmap *ba, > + const libxl_bitmap *bb, > + int nr_bits) > +{ > + int i; > + > + /* Only check nr_bits (all bits if <= 0) */ > + nr_bits = nr_bits <=0 ? ba->size * 8 : nr_bits; The conditional should really have parenthesis around it, I'm pretty sure; and the spacing is inconsistent. -George