|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v3 11/14] libxl: get and set soft affinity
On Mon, 2013-11-18 at 19:18 +0100, 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>
> ---
> Changes from v2:
> * interface completely redesigned, as discussed during
> review.
> ---
> tools/libxl/Makefile | 2 -
> tools/libxl/libxl.c | 131
> +++++++++++++++++++++++++++++++++++++++++++
> tools/libxl/libxl.h | 30 ++++++++++
> tools/libxl/libxl_create.c | 6 ++
> tools/libxl/libxl_types.idl | 4 +
> tools/libxl/libxl_utils.h | 15 +++++
> 6 files changed, 186 insertions(+), 2 deletions(-)
>
> diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
> index cf214bb..cba32d5 100644
> --- a/tools/libxl/Makefile
> +++ b/tools/libxl/Makefile
> @@ -5,7 +5,7 @@
> XEN_ROOT = $(CURDIR)/../..
> include $(XEN_ROOT)/tools/Rules.mk
>
> -MAJOR = 4.3
> +MAJOR = 4.4
> MINOR = 0
>
> XLUMAJOR = 4.3
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index d0db3f0..1122360 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -4204,6 +4204,8 @@ libxl_vcpuinfo *libxl_list_vcpu(libxl_ctx *ctx,
> uint32_t domid,
> for (*nb_vcpu = 0; *nb_vcpu <= domaininfo.max_vcpu_id; ++*nb_vcpu,
> ++ptr) {
> if (libxl_cpu_bitmap_alloc(ctx, &ptr->cpumap, 0))
> return NULL;
> + if (libxl_cpu_bitmap_alloc(ctx, &ptr->cpumap_soft, 0))
> + return NULL;
> if (xc_vcpu_getinfo(ctx->xch, domid, *nb_vcpu, &vcpuinfo) == -1) {
> LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting vcpu info");
> return NULL;
> @@ -4214,6 +4216,12 @@ libxl_vcpuinfo *libxl_list_vcpu(libxl_ctx *ctx,
> uint32_t domid,
> LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting vcpu affinity");
> return NULL;
> }
> + if (xc_vcpu_getaffinity(ctx->xch, domid, *nb_vcpu,
> + XEN_VCPUAFFINITY_SOFT,
> + ptr->cpumap_soft.map) == -1) {
> + LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting vcpu affinity");
> + return NULL;
> + }
> ptr->vcpuid = *nb_vcpu;
> ptr->cpu = vcpuinfo.cpu;
> ptr->online = !!vcpuinfo.online;
> @@ -4250,6 +4258,129 @@ int libxl_set_vcpuaffinity_all(libxl_ctx *ctx,
> uint32_t domid,
> return rc;
> }
>
> +int libxl_set_vcpuaffinity2(libxl_ctx *ctx, uint32_t domid, uint32_t vcpuid,
> + const libxl_bitmap *cpumap, int flags)
I think if we are going to duplicate hte API in this way then we should
still combine the implementation, either with an internal private helper
or by making the old API a wrapper around the new one.
The internals of ...2 and ...3 should be shared as far as possible too.
> +{
> + libxl_cputopology *topology;
> + libxl_bitmap ecpumap;
> + int nr_cpus = 0, rc;
> +
> + topology = libxl_get_cpu_topology(ctx, &nr_cpus);
> + if (!topology) {
> + LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "failed to retrieve CPU topology");
It's not consistent within the file but I think for new functions we
should use the LOG macro variants.
> + return ERROR_FAIL;
> + }
> + libxl_cputopology_list_free(topology, nr_cpus);
Why are you retrieving this only to immediately throw it away?
> +
> + rc = libxl_cpu_bitmap_alloc(ctx, &ecpumap, 0);
> + if (rc)
> + return rc;
> +
> + if (xc_vcpu_setaffinity(ctx->xch, domid, vcpuid, cpumap->map,
> + flags, ecpumap.map)) {
> + LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "setting vcpu affinity");
> + rc = ERROR_FAIL;
> + goto out;
> + }
> +
> + if (!libxl_bitmap_equal(cpumap, &ecpumap, nr_cpus))
> + LIBXL__LOG(ctx, LIBXL__LOG_DEBUG,
> + "New affinity for vcpu %d contains unreachable cpus",
> + vcpuid);
> + if (libxl_bitmap_is_empty(&ecpumap))
> + LIBXL__LOG(ctx, LIBXL__LOG_WARNING,
> + "New affinity for vcpu %d has only unreachabel cpus. "
"unreachable"
> + "Only hard affinity will be considered for scheduling",
> + vcpuid);
> +
> + rc = 0;
> + out:
> + libxl_bitmap_dispose(&ecpumap);
> + return 0;
> +}
> +int libxl_set_vcpuaffinity3(libxl_ctx *ctx, uint32_t domid, uint32_t vcpuid,
> + const libxl_bitmap *cpumap_hard,
> + const libxl_bitmap *cpumap_soft)
Insert the same comments as ...2, because AFAICT it is mostly the same
function.
> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> index c7dceda..504c57b 100644
> --- a/tools/libxl/libxl.h
> +++ b/tools/libxl/libxl.h
> @@ -82,6 +82,20 @@
> #define LIBXL_HAVE_DOMAIN_NODEAFFINITY 1
>
> /*
> + * LIBXL_HAVE_VCPUINFO_SOFTAFFINITY indicates that a 'cpumap_soft'
> + * field (of libxl_bitmap type) is present in libxl_vcpuinfo,
> + * containing the soft affinity for the vcpu.
> + */
> +#define LIBXL_HAVE_VCPUINFO_SOFTAFFINITY 1
> +
> +/*
> + * LIBXL_HAVE_BUILDINFO_SOFTAFFINITY indicates that a 'cpumap_soft'
> + * field (of libxl_bitmap type) is present in libxl_domain_build_info,
> + * containing the soft affinity for the vcpu.
> + */
> +#define LIBXL_HAVE_BUILDINFO_SOFTAFFINITY 1
Given that they arrive can we just use HAVE_SOFTRAFFINITY?
> +
> +/*
> * 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
> @@ -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);
> +int libxl_set_vcpuaffinity3(libxl_ctx *ctx, uint32_t domid, uint32_t vcpuid,
> + const libxl_bitmap *cpumap_hard,
> + const libxl_bitmap *cpumap_soft);
> +int libxl_set_vcpuaffinity_all3(libxl_ctx *ctx, uint32_t domid,
> + unsigned int max_vcpus,
> + const libxl_bitmap *cpumap_hard,
> + const libxl_bitmap *cpumap_soft);
> +/* Flags, consistent with domctl.h */
> +#define LIBXL_VCPUAFFINITY_HARD 1
> +#define LIBXL_VCPUAFFINITY_SOFT 2
Can these be an enum in the idl?
> +
> 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
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |