Re: [Xen-devel] [PATCH v3 11/14] libxl: get and set soft affinity

[Dario Faggioli <dario.faggioli@xxxxxxxxxx>]

On mar, 2013-11-19 at 15:41 +0000, George Dunlap wrote:
> 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 <dario.faggioli@xxxxxxxxxx>
> 
> 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:
> 
the '3' variant (tries to) accomplish what IanJ explicitly asked: having
a way to set both hard and soft affinity at the same time, and each with
its own value, and only checking for consistency at the very end.

I also wasn't sure whether that would have been actually useful but, I
have to admit, it turned out it is, as it can be seen in the following
patches, when the interface is used to (re)implement both the existing
and the new xl commands and command variants.

Let's see what IanJ thinks, I guess. :-)

> > @@ -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?
> 
Fair enough. I can respin with that added.

> > 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.
> 
Right. Will do.

Thanks and Regards,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

Attachment: signature.asc
Description: This is a digitally signed message part

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