Re: [v7 PATCH 07/10] libxl: get and set soft affinity

[George Dunlap <george.dunlap@eu.citrix.com>]

On 06/10/2014 01:45 AM, Dario Faggioli wrote:
> Make space for two new cpumap-s, one in vcpu_info (for getting
> soft affinity) and build_info (for setting it) and amend the
> API for setting vCPU affinity.
>
> libxl_set_vcpuaffinity() now takes two cpumaps, one for hard
> and one for soft affinity (LIBXL_API_VERSION is exploited to
> retain source level backword compatibility). Either of the
> two cpumap can be NULL, in which case, only the affinity
> corresponding to the non-NULL cpumap will be affected.
>
> Getting soft affinity happens indirectly, via `xl vcpu-list'
> (as it is already for hard affinity).
>
> This commit also introduces some logic to check whether the
> affinity which will be used by Xen to schedule the vCPU(s)
> does actually match with the cpumaps provided. In fact, we
> want to allow every possible combination of hard and soft
> affinity to be set, but we warn the user upon particularly
> weird situations (e.g., hard and soft being disjoint sets
> of pCPUs).
>
> This very change also update the error handling for calls
> to libxl_set_vcpuaffinity() in xl, as that can now be any
> libxl error code, not just only -1.
>
> Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
> Reviewed-by: George Dunlap <george.dunlap@eu.citrix.com>

>
> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> index 80947c3..e549db8 100644
> --- a/tools/libxl/libxl.h
> +++ b/tools/libxl/libxl.h
> @@ -82,6 +82,15 @@
>   #define LIBXL_HAVE_DOMAIN_NODEAFFINITY 1
>   
>   /*
> + * LIBXL_HAVE_SOFT_AFFINITY indicates that a 'cpumap_soft'
> + * field (of libxl_bitmap type) is present in:
> + *  - libxl_vcpuinfo, containing the soft affinity of a vcpu;
> + *  - libxl_domain_build_info, for setting the soft affinity of
> + *    all vcpus while creating the domain.
> + */
> +#define LIBXL_HAVE_SOFT_AFFINITY 1

Did we say we were going to move these to where the actual change was 
happening (say, down in the libxl_set_vcpuaffinity definitions)?  I'm OK 
either way.

> +
> +/*
>    * LIBXL_HAVE_BUILDINFO_HVM_VENDOR_DEVICE indicates that the
>    * libxl_vendor_device field is present in the hvm sections of
>    * libxl_domain_build_info. This field tells libxl which
> @@ -1097,9 +1106,22 @@ int libxl_userdata_retrieve(libxl_ctx *ctx, uint32_t domid,
>   
>   int libxl_get_physinfo(libxl_ctx *ctx, libxl_physinfo *physinfo);
>   int libxl_set_vcpuaffinity(libxl_ctx *ctx, uint32_t domid, uint32_t vcpuid,
> -                           libxl_bitmap *cpumap);
> +                           const libxl_bitmap *cpumap_hard,
> +                           const libxl_bitmap *cpumap_soft);
>   int libxl_set_vcpuaffinity_all(libxl_ctx *ctx, uint32_t domid,
> -                               unsigned int max_vcpus, libxl_bitmap *cpumap);
> +                               unsigned int max_vcpus,
> +                               const libxl_bitmap *cpumap_hard,
> +                               const libxl_bitmap *cpumap_soft);
> +
> +#if defined (LIBXL_API_VERSION) && LIBXL_API_VERSION < 0x040500
> +
> +#define libxl_set_vcpuaffinity(ctx, domid, vcpuid, map) \
> +    libxl_set_vcpuaffinity((ctx), (domid), (vcpuid), (map), NULL)
> +#define libxl_set_vcpuaffinity_all(ctx, domid, max_vcpus, map) \
> +    libxl_set_vcpuaffinity_all((ctx), (domid), (max_vcpus), (map), NULL)
> +
> +#endif
> +
>   int libxl_domain_set_nodeaffinity(libxl_ctx *ctx, uint32_t domid,
>                                     libxl_bitmap *nodemap);
>   int libxl_domain_get_nodeaffinity(libxl_ctx *ctx, uint32_t domid,
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index d015cf4..49a01a7 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -193,6 +193,12 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
>           libxl_bitmap_set_any(&b_info->cpumap);
>       }
>   
> +    if (!b_info->cpumap_soft.size) {
> +        if (libxl_cpu_bitmap_alloc(CTX, &b_info->cpumap_soft, 0))
> +            return ERROR_FAIL;
> +        libxl_bitmap_set_any(&b_info->cpumap_soft);
> +    }
> +
>       libxl_defbool_setdefault(&b_info->numa_placement, true);
>   
>       if (!b_info->nodemap.size) {
> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> index 661999c..91af7f8 100644
> --- a/tools/libxl/libxl_dom.c
> +++ b/tools/libxl/libxl_dom.c
> @@ -261,7 +261,8 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid,
>               return rc;
>       }
>       libxl_domain_set_nodeaffinity(ctx, domid, &info->nodemap);
> -    libxl_set_vcpuaffinity_all(ctx, domid, info->max_vcpus, &info->cpumap);
> +    libxl_set_vcpuaffinity_all(ctx, domid, info->max_vcpus, &info->cpumap,
> +                               &info->cpumap_soft);
>   
>       if (xc_domain_setmaxmem(ctx->xch, domid, info->target_memkb +
>           LIBXL_MAXMEM_CONSTANT) < 0) {
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index 52f1aa9..15382cc 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -300,6 +300,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
>       ("max_vcpus",       integer),
>       ("avail_vcpus",     libxl_bitmap),
>       ("cpumap",          libxl_bitmap),
> +    ("cpumap_soft",     libxl_bitmap),
>       ("nodemap",         libxl_bitmap),
>       ("numa_placement",  libxl_defbool),
>       ("tsc_mode",        libxl_tsc_mode),
> @@ -516,7 +517,8 @@ libxl_vcpuinfo = Struct("vcpuinfo", [
>       ("blocked", bool),
>       ("running", bool),
>       ("vcpu_time", uint64), # total vcpu time ran (ns)
> -    ("cpumap", libxl_bitmap), # current cpu's affinities
> +    ("cpumap", libxl_bitmap), # current hard cpu affinity
> +    ("cpumap_soft", libxl_bitmap), # current soft cpu affinity
>       ], dir=DIR_OUT)
>   
>   libxl_physinfo = Struct("physinfo", [
> diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
> index e37fb89..014f1f6 100644
> --- a/tools/libxl/libxl_utils.h
> +++ b/tools/libxl/libxl_utils.h
> @@ -98,6 +98,31 @@ 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))
>   
> +/*
> + * Compares two bitmaps bit by bit, up to nr_bits or, if nr_bits is 0, up
> + * to the size of the lagest bitmap. If sizes does not match, bits past the

*largest

Other than that:

Reviewed-by: George Dunlap <george.dunlap@eu.citrix.com>