Implement vcpu soft affinity for credit1

as well as both error handling and logging in libxl_cpu_bitmap_alloc and libxl_node_bitmap_alloc.

Now libxl_get_max_{cpus,nodes} either return a positive number, or a libxl error code. Thanks to that, it is possible to fix loggig for the two bitmap allocation functions, which now happens _inside_ the functions themselves, and report what happens more accurately.

Signed-off-by Dario Faggioli <[email protected]>

tools/libxl/libxl_utils.c +9 -0

+        goto out;
+    }
+    if (max_cpus == 0)
+        max_cpus = libxl_get_max_cpus(ctx);
+    if (max_cpus <= 0) {
+        LOG(ERROR, "failed to retrieve the maximum number of cpus");
+        rc = ERROR_FAIL;
+        goto out;
+    }

tools/libxl/libxl_utils.c +24 -0

+{
+    GC_INIT(ctx);
+    int rc = 0;
+
+    if (max_nodes < 0) {
+        rc = ERROR_INVAL;
+        goto out;
+    }
+
+    if (max_nodes == 0)
+        max_nodes = libxl_get_max_nodes(ctx);
+    if (max_nodes <= 0) {
+        LOG(ERROR, "failed to retrieve the maximum number of nodes");
+        rc = ERROR_FAIL;
+        goto out;
+    }
+    /* This can't fail: no need to check and log */
+    libxl_bitmap_alloc(ctx, nodemap, max_nodes);
+
+ out:
+    GC_FREE;
+    return rc;
+}
+
 int libxl_nodemap_to_cpumap(libxl_ctx *ctx,
                             const libxl_bitmap *nodemap,
                             libxl_bitmap *cpumap)

Quoted 10 lines · Dario Faggioli

 
     cpumap->map = xc_cpupool_freeinfo(ctx->xch);

tools/libxl/libxl_utils.c +0 -0

index 9f5f589..1815422 100644
@@ -651,6 +651,56 @@ char *libxl_bitmap_to_hex_string(libxl_ctx *ctx, const libxl_bitmap *bitmap)
     return q;
 }
 

tools/libxl/libxl_utils.c char *libxl_bitmap_to_hex_string(libxl_ctx *ctx, const libxl_bitmap *bitmap) · line 654

Quoted 1 line

tools/libxl/libxl_utils.c +1 -0

+int libxl_cpu_bitmap_alloc(libxl_ctx *ctx, libxl_bitmap *cpumap, int max_cpus)

You seem to have combined code motion with actual changes here. Please don't do that.

Quoted 11 lines · Ian Campbell

Quoted 10 lines · Dario Faggioli

 
     cpumap->map = xc_cpupool_freeinfo(ctx->xch);

tools/libxl/libxl_utils.c +0 -0

index 9f5f589..1815422 100644
@@ -651,6 +651,56 @@ char *libxl_bitmap_to_hex_string(libxl_ctx *ctx, const libxl_bitmap *bitmap)
     return q;
 }
 

Quoted 5 lines

tools/libxl/libxl_utils.c char *libxl_bitmap_to_hex_string(libxl_ctx *ctx, const libxl_bitmap *bitmap) · line 654

Quoted 1 line

tools/libxl/libxl_utils.c +1 -0

+int libxl_cpu_bitmap_alloc(libxl_ctx *ctx, libxl_bitmap *cpumap, int max_cpus)

You seem to have combined code motion with actual changes here. Please don't do that.

As I explained in the commit message for this patch in v5, I actually am both moving and (slightly) changing the functions, and that's mainly because the changes I'm doing require the move (e.g., GC_INIT/GC_FREE not being available in the original place).

I feel like it's more natural to do like this, rather than having a pre-patch just moving the code for no apparent reason... Isn't it?

Or was it something else that you were pointing out?

(Anyway, feel free to look at v5 and reply there).

Quoted 2 lines · Dario Faggioli

I feel like it's more natural to do like this, rather than having a pre-patch just moving the code for no apparent reason... Isn't it?

Certainly making changes which are necessarily required by the code motion is fine to do in a single step, although even then if you can arrange to make the changes either before or after the move it is better.

But is that what is happening here?

tools/libxl/libxl_utils.h +0 -12

-static inline int libxl_cpu_bitmap_alloc(libxl_ctx *ctx, libxl_bitmap *cpumap,
-                                         int max_cpus)
-{
-    if (max_cpus < 0)
-        return ERROR_INVAL;
-    if (max_cpus == 0)
-        max_cpus = libxl_get_max_cpus(ctx);
-    if (max_cpus == 0)
-        return ERROR_FAIL;
-
-    return libxl_bitmap_alloc(ctx, cpumap, max_cpus);
-}

is becoming:

tools/libxl/libxl_utils.c +23 -0

+int libxl_cpu_bitmap_alloc(libxl_ctx *ctx, libxl_bitmap *cpumap, int max_cpus)
+{
+    GC_INIT(ctx);
+    int rc = 0;
+
+    if (max_cpus < 0) {
+        rc = ERROR_INVAL;
+        goto out;
+    }
+    if (max_cpus == 0)
+        max_cpus = libxl_get_max_cpus(ctx);
+    if (max_cpus <= 0) {
+        LOG(ERROR, "failed to retrieve the maximum number of cpus");
+        rc = ERROR_FAIL;
+        goto out;
+    }
+    /* This can't fail: no need to check and log */
+    libxl_bitmap_alloc(ctx, cpumap, max_cpus);
+
+ out:
+    GC_FREE;
+    return rc;
+}

which involves a lot more reworking than simply adding the GC.

In any
case I don't see why the original function couldn't be moved as is and then have the GC-ification added in the new location, I don't think the move requires the change to using the GC In any way.

Quoted 1 line

(Anyway, feel free to look at v5 and reply there).

In general it would be better to reply to vN either up front or as you do the rework, so we can resolve any misunderstanding rather than waiting until after you've posted vN+1 and perhaps requiring a vN+2.

So, my view here is: if I modify the function, adding GC and the LOG()s, then it should also be moved, if not, it can stay where it is. What you seem to be suggesting is that I (either in this or a previous patch) move the function as is, for no apparent reason,

Quoted 4 lines · Dario Faggioli

So, my view here is: if I modify the function, adding GC and the LOG()s, then it should also be moved, if not, it can stay where it is. What you seem to be suggesting is that I (either in this or a previous patch) move the function as is, for no apparent reason,

It's not for "no apparent reason", it is to separate code motion from reworking the function, which you would explain in your commit message.

If that were considered no apparent reason then it wouldn't be possible to split the code motion from the functional changes in the majority of cases.

Thanks for the explanation, and sorry for not getting what you mean/want sooner. It just differs from the way I've done things up to now, so I really wasn't sure I was understanding, rather than not wanting to do it. :-)

to retrieve the actual number of pCPUs on the host.

Signed-off-by Dario Faggioli <[email protected]>

---
 tools/libxl/libxl.h       |    3 +++
 tools/libxl/libxl_utils.c |   18 ++++++++++++++++++
 2 files changed, 21 insertions(+)

tools/libxl/libxl.h +0 -0

index a9663e4..2fab5ba 100644

tools/libxl/libxl_utils.c +14 -0

+    int rc = 0;
+
+    if (xc_physinfo(ctx->xch, &physinfo)) {
+        LOGE(ERROR, "xc_physinfo failed.");
+        rc = ERROR_FAIL;
+        goto out;
+    }
+    rc = physinfo.nr_cpus;
+
+ out:
+    GC_FREE;
+    return rc;
+}
+
 int libxl_get_max_nodes(libxl_ctx *ctx)
 {
     int max_nodes = xc_get_max_nodes(ctx->xch);

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

to retrieve the actual number of pCPUs on the host.

Signed-off-by Dario Faggioli <[email protected]>

---
 tools/libxl/libxl.h       |    3 +++
 tools/libxl/libxl_utils.c |   18 ++++++++++++++++++
 2 files changed, 21 insertions(+)

tools/libxl/libxl.h +0 -0

index a9663e4..2fab5ba 100644

Dario Faggioli writes ("[PATCH v4 03/15] libxl: introduce libxl_get_nr_cpus()"):

Quoted 16 lines

to retrieve the actual number of pCPUs on the host.

Signed-off-by Dario Faggioli <[email protected]>

---
 tools/libxl/libxl.h       |    3 +++
 tools/libxl/libxl_utils.c |   18 ++++++++++++++++++
 2 files changed, 21 insertions(+)

tools/libxl/libxl.h +0 -0

index a9663e4..2fab5ba 100644
@@ -652,6 +652,9 @@ int libxl_domain_preserve(libxl_ctx *ctx, uint32_t domid, libxl_domain_create_in
 /* get max. number of cpus supported by hypervisor */
 int libxl_get_max_cpus(libxl_ctx *ctx);
 

tools/libxl/libxl.h int libxl_domain_preserve(libxl_ctx *ctx, uint32_t domid, libxl_domain_create_in · line 656

Quoted 2 lines

tools/libxl/libxl.h +2 -0

+/* get the actual number of online cpus on the host */
+int libxl_get_nr_cpus(libxl_ctx *ctx);

This number might be out of date as soon as it is read, won't it ?

Quoted 1 line

This number might be out of date as soon as it is read, won't it ?

Quite possible, yes.

So, are you suggesting that we shouldn't even allow the user to read it? Or that I should mention that in the comment? (Or something else?)

Dario Faggioli writes ("Re: [PATCH v4 03/15] libxl: introduce libxl_get_nr_cpus()"):

Quoted 3 lines · Ian Jackson

Quoted 1 line

This number might be out of date as soon as it is read, won't it ?

Quoted 4 lines

Quite possible, yes.

So, are you suggesting that we shouldn't even allow the user to read it? Or that I should mention that in the comment? (Or something else?)

Perhaps I didn't explain my concerns clearly enough.

I wonder what is it for ? Isn't it difficult to use correctly ?

Quoted 4 lines · Ian Jackson

Dario Faggioli writes ("Re: [PATCH v4 03/15] libxl: introduce libxl_get_nr_cpus()"):

Quoted 3 lines · Ian Jackson

Quoted 1 line

This number might be out of date as soon as it is read, won't it ?

Quoted 8 lines

Quoted 4 lines

Quite possible, yes.

So, are you suggesting that we shouldn't even allow the user to read it? Or that I should mention that in the comment? (Or something else?)

Perhaps I didn't explain my concerns clearly enough.

I wonder what is it for ? Isn't it difficult to use correctly ?

Dario uses it in the new version of libxl_vcpu_set_affinity() to limit what was considered an "unreachable cpu" in its warning. (v5 14/17) That is, if you set the affinity to 1111111111111111111111..., and there are only 4 pcpus, it will return 111100000..... These don't match, and yet there are no unreachable cpus. So it asks nr_cpus first, then only compares bits 1..[nr_cpus-1].

I'm not sure how often that would change, but if there was a race, I think it would just be a spurious error message (or lack thereof).

-George

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

Quoted 14 lines · George Dunlap

Quoted 4 lines · Ian Jackson

Dario Faggioli writes ("Re: [PATCH v4 03/15] libxl: introduce libxl_get_nr_cpus()"):

Quoted 3 lines · Ian Jackson

Quoted 1 line

This number might be out of date as soon as it is read, won't it ?

Quoted 8 lines

Quoted 4 lines

Quite possible, yes.

So, are you suggesting that we shouldn't even allow the user to read it? Or that I should mention that in the comment? (Or something else?)

Perhaps I didn't explain my concerns clearly enough.

I wonder what is it for ? Isn't it difficult to use correctly ?

Quoted 7 lines

Dario uses it in the new version of libxl_vcpu_set_affinity() to limit what was considered an "unreachable cpu" in its warning. (v5 14/17) That is, if you set the affinity to 1111111111111111111111..., and there are only 4 pcpus, it will return 111100000..... These don't match, and yet there are no unreachable cpus. So it asks nr_cpus first, then only compares bits 1..[nr_cpus-1].

What about the inverse? 2 PCPUs and 16 VCPUS? Won't that still throw this calculation off?

Quoted 14 lines · George Dunlap

Quoted 4 lines · Ian Jackson

Dario Faggioli writes ("Re: [PATCH v4 03/15] libxl: introduce libxl_get_nr_cpus()"):

Quoted 3 lines · Ian Jackson

Quoted 1 line

This number might be out of date as soon as it is read, won't it ?

Quoted 8 lines

Quoted 4 lines

Quite possible, yes.

So, are you suggesting that we shouldn't even allow the user to read it? Or that I should mention that in the comment? (Or something else?)

Perhaps I didn't explain my concerns clearly enough.

I wonder what is it for ? Isn't it difficult to use correctly ?

Quoted 7 lines

Dario uses it in the new version of libxl_vcpu_set_affinity() to limit what was considered an "unreachable cpu" in its warning. (v5 14/17) That is, if you set the affinity to 1111111111111111111111..., and there are only 4 pcpus, it will return 111100000..... These don't match, and yet there are no unreachable cpus. So it asks nr_cpus first, then only compares bits 1..[nr_cpus-1].

What about the inverse? 2 PCPUs and 16 VCPUS? Won't that still throw this calculation off?

Quoted 11 lines · Ian Jackson

Dario Faggioli writes ("Re: [PATCH v4 03/15] libxl: introduce libxl_get_nr_cpus()"):

Quoted 7 lines · Ian Jackson

Quoted 1 line

This number might be out of date as soon as it is read, won't it ?

Quite possible, yes.

So, are you suggesting that we shouldn't even allow the user to read it? Or that I should mention that in the comment? (Or something else?)

Perhaps I didn't explain my concerns clearly enough.

Quoted 1 line

I wonder what is it for ? Isn't it difficult to use correctly ?

Dario uses it in the new version of libxl_vcpu_set_affinity() to limit what was considered an "unreachable cpu" in its warning. (v5 14/17) That is, if you set the affinity to 1111111111111111111111..., and there are only 4 pcpus, it will return 111100000..... These don't match, and yet there are no unreachable cpus. So it asks nr_cpus first, then only compares bits 1..[nr_cpus-1].

I'm not sure how often that would change, but if there was a race, I think it would just be a spurious error message (or lack thereof).

Dario Faggioli writes ("Re: [PATCH v4 03/15] libxl: introduce libxl_get_nr_cpus()"):

Quoted 3 lines · Ian Jackson

Quoted 1 line

This number might be out of date as soon as it is read, won't it ?

Quoted 4 lines

Quite possible, yes.

So, are you suggesting that we shouldn't even allow the user to read it? Or that I should mention that in the comment? (Or something else?)

Perhaps I didn't explain my concerns clearly enough.

I wonder what is it for ? Isn't it difficult to use correctly ?

On an IRC conversation, IanJ seems to be convinced about this too:

Diziet: Eg if you run it while also adding a cpu ?
dariof: Diziet: it's in the dry_run path
dariof: Diziet: so, when it prints the cpumap that would have been used (if not in dryrun_only) mode, like this: print_bitmap(cpumap.map, nb_cpu, stdout);
dariof: Diziet: it's possible for the output to not be consistent with the new situation
dariof: Diziet: basically, if it asks for the number of onlince cpus, it gets 3, and then the mask is <0111> it will print "all"
Diziet: I think you have convinced me that this is too intractable a problem to solve at this level of the API and that therefore the call you propose to add is fine.
Diziet: Shall I c&p this scrool into an email with my ack ?

I cut-&-paste-d this with Ian's permission, but I'd rather avoid providing a formal "Acked-by: Ian Jackson" line myself! :-P

Quoted 1 line

Quoted 1 line · Dario Faggioli

xen/common/domctl.c +6 -0

+            /*
+             * Report back to the caller what the "effective affinity", that
+             * is the intersection of cpupool's pcpus, the (new?) hard
+             * affinity and the (new?) soft-affinity.
+             */
+            if ( !guest_handle_is_null(op->u.vcpuaffinity.eff_cpumap.bitmap) )
             {

         else
         {

xen/common/domctl.c +17 -0

+            cpumask_var_t affinity;
+
+            /*
+             * If the caller asks for both _HARD and _SOFT, what we return
+             * is the intersection of hard and soft affinity for the vcpu.
+             */
+            if ( !alloc_cpumask_var(&affinity) )
+            {
+                ret = -ENOMEM;
+                break;
+            }
+            cpumask_setall(affinity);
+
+            if ( op->u.vcpuaffinity.flags == 0 )
+            {
+                ret = -EINVAL;
+                free_cpumask_var(affinity);

xen/include/public/domctl.h +0 -2

@@ -300,8 +300,19 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_nodeaffinity_t);
 /* XEN_DOMCTL_setvcpuaffinity */
 /* XEN_DOMCTL_getvcpuaffinity */
 struct xen_domctl_vcpuaffinity {
-    uint32_t  vcpu;              /* IN */
-    struct xenctl_bitmap cpumap; /* IN/OUT */

Quoted 8 lines

Quoted 1 line

Quoted 1 line · Dario Faggioli

xen/common/domctl.c +6 -0

+            /*
+             * Report back to the caller what the "effective affinity", that
+             * is the intersection of cpupool's pcpus, the (new?) hard
+             * affinity and the (new?) soft-affinity.
+             */
+            if ( !guest_handle_is_null(op->u.vcpuaffinity.eff_cpumap.bitmap) )
              {

xen/common/domctl.c long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl) · line 678

Quoted 10 lines

xen/common/domctl.c +8 -2

-                ret = vcpu_set_affinity(v, new_affinity);
-                free_cpumask_var(new_affinity);
+                online = cpupool_online_cpumask(v->domain->cpupool);
+                cpumask_and(new_affinity, online, v->cpu_hard_affinity);
+                if ( op->u.vcpuaffinity.flags & XEN_VCPUAFFINITY_SOFT)
+                    cpumask_and(new_affinity, new_affinity,
+                                v->cpu_soft_affinity);
+
+                ret = cpumask_to_xenctl_bitmap(
+                    &op->u.vcpuaffinity.eff_cpumap, new_affinity);

So with both flags set, how is the caller supposed to know what hard affinity is now in effect? I said on the previous version already that with you _having_ two CPU masks, you should return both.

Quoted 18 lines · Jan Beulich

Quoted 18 lines

Quoted 1 line

Quoted 1 line · Dario Faggioli

xen/common/domctl.c +14 -2

+            /*
+             * Report back to the caller what the "effective affinity", that
+             * is the intersection of cpupool's pcpus, the (new?) hard
+             * affinity and the (new?) soft-affinity.
+             */
+            if ( !guest_handle_is_null(op->u.vcpuaffinity.eff_cpumap.bitmap) )
              {
-                ret = vcpu_set_affinity(v, new_affinity);
-                free_cpumask_var(new_affinity);
+                online = cpupool_online_cpumask(v->domain->cpupool);
+                cpumask_and(new_affinity, online, v->cpu_hard_affinity);
+                if ( op->u.vcpuaffinity.flags & XEN_VCPUAFFINITY_SOFT)
+                    cpumask_and(new_affinity, new_affinity,
+                                v->cpu_soft_affinity);
+
+                ret = cpumask_to_xenctl_bitmap(
+                    &op->u.vcpuaffinity.eff_cpumap, new_affinity);

Quoted 3 lines

So with both flags set, how is the caller supposed to know what hard affinity is now in effect? I said on the previous version already that with you _having_ two CPU masks, you should return both.

If I recall the timing correctly, I think this series was sent out before you guys had come to that conclusion on the other thread.

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 is the first change breaking the libxl ABI, so it bumps the MAJOR.

Signed-off-by Dario Faggioli <[email protected]>

---
Changes from v3:
* only introduce one LIBXL_HAVE_ symbol for soft affinity,

    as requested during review;
  * use LIBXL_API_VERSION instead of having multiple version
    of the same function, as suggested during review;
  * use libxl_get_nr_cpus() rather than libxl_get_cputopology(),
    as suggested during review;
  * use LOGE() instead of LIBXL__LOG_ERRNO(), as requested
    during review;

* kill the flags and use just one _set_vcpuaffinity() function with two cpumaps, allowing either of them to be NULL, as suggested during review;
* avoid overflowing the bitmaps in libxl_bitmap_equal(), as suggested during review.

Changes from v2:
* interface completely redesigned, as discussed during review.

---
  tools/libxl/Makefile        |    2 +
  tools/libxl/libxl.c         |   79 ++++++++++++++++++++++++++++++++++++++-----
  tools/libxl/libxl.h         |   38 ++++++++++++++++++++-
  tools/libxl/libxl_create.c  |    6 +++
  tools/libxl/libxl_dom.c     |    3 +-
  tools/libxl/libxl_types.idl |    4 ++
  tools/libxl/libxl_utils.h   |   25 +++++++++++++-
  tools/libxl/xl_cmdimpl.c    |    6 ++-
  8 files changed, 145 insertions(+), 18 deletions(-)

tools/libxl/Makefile +1 -1

index d8495bb..1410c44 100644
@@ -5,7 +5,7 @@
  XEN_ROOT = $(CURDIR)/../..
  include $(XEN_ROOT)/tools/Rules.mk
 
-MAJOR = 4.3
+MAJOR = 4.4
  MINOR = 0
 
  XLUMAJOR = 4.3

tools/libxl/libxl.c +70 -9

index 961d55e..4b871d7 100644
@@ -4538,6 +4538,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;

@@ -4548,6 +4550,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;

@@ -4559,28 +4567,81 @@ libxl_vcpuinfo *libxl_list_vcpu(libxl_ctx *ctx, uint32_t domid,
  }
 
  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)
  {
-    if (xc_vcpu_setaffinity(ctx->xch, domid, vcpuid, cpumap->map,
-                            XEN_VCPUAFFINITY_HARD, NULL)) {
-        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "setting vcpu affinity");
-        return ERROR_FAIL;
+    GC_INIT(ctx);
+    libxl_bitmap ecpumap;
+    int nr_cpus = 0, rc;
+
+    if (!cpumap_hard && !cpumap_soft)
+        return ERROR_INVAL;
+
+    rc = libxl_cpu_bitmap_alloc(ctx, &ecpumap, 0);
+    if (rc)
+        return rc;
+
+    nr_cpus = libxl_get_nr_cpus(ctx);
+    if (nr_cpus <= 0) {
+        rc = ERROR_FAIL;
+        goto out;
      }
+
+    if (cpumap_hard && xc_vcpu_setaffinity(ctx->xch, domid, vcpuid,
+                                           cpumap_hard->map,
+                                           XEN_VCPUAFFINITY_HARD,
+                                           ecpumap.map)) {
+        LOGE(ERROR, "setting vcpu hard affinity");
+        rc = ERROR_FAIL;
+        goto out;
+    }
+    if (cpumap_hard && !libxl_bitmap_equal(cpumap_hard, &ecpumap, nr_cpus))
+        LOG(DEBUG, "New hard affinity for vcpu %d contains unreachable cpus",
+            vcpuid);
+
+    if (cpumap_soft && xc_vcpu_setaffinity(ctx->xch, domid, vcpuid,
+                                           cpumap_soft->map,
+                                           XEN_VCPUAFFINITY_SOFT,
+                                           ecpumap.map)) {
+        LOGE(ERROR, "setting vcpu soft affinity");
+        rc = ERROR_FAIL;
+        goto out;
+    }
+    if (cpumap_soft && !libxl_bitmap_equal(cpumap_soft, &ecpumap, nr_cpus))
+        LOG(DEBUG, "New soft affinity for vcpu %d contains unreachable cpus",
+            vcpuid);
+
+    /*
+     * When setting hard affinity, it is guaranteed that the result will not
+     * be empty, so we need to check for that only if soft.
+     */
+    if (cpumap_soft && libxl_bitmap_is_empty(&ecpumap))
+        LOG(WARN, "New soft affinity for vcpu %d has only unreachable cpus."
+            " Only hard affinity will be considered for scheduling", vcpuid);
+
+    rc = 0;
+ out:
+    libxl_bitmap_dispose(&ecpumap);
+    GC_FREE;
      return 0;
  }
 
  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)
  {
+    GC_INIT(ctx);
      int i, rc = 0;
 
      for (i = 0; i < max_vcpus; i++) {
-        if (libxl_set_vcpuaffinity(ctx, domid, i, cpumap)) {
-            LIBXL__LOG(ctx, LIBXL__LOG_WARNING,
-                       "failed to set affinity for %d", i);
+        if (libxl_set_vcpuaffinity(ctx, domid, i, cpumap_hard, cpumap_soft)) {
+            LOG(WARN, "failed to set affinity for %d", i);
              rc = ERROR_FAIL;
          }
      }
+
+    GC_FREE;
      return rc;
  }
 

tools/libxl/libxl.h +36 -2

index 2fab5ba..9864927 100644
@@ -82,6 +82,14 @@
  #define LIBXL_HAVE_DOMAIN_NODEAFFINITY 1
 
  /*
+ * LIBXL_HAVE_SOFT_AFFINITY indicates that a 'cpumap_soft'
+ * field (of libxl_bitmap type) is present in both
+ * libxl_domain_build_info and libxl_vcpuinfo, containing
+ * the soft affinity for the vcpu.
+ */
+#define LIBXL_HAVE_SOFT_AFFINITY 1
+
+/*
   * 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

@@ -994,9 +1002,35 @@ 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 < 0x040400
+
+static inline
+int libxl_set_vcpuaffinity_0x040200(libxl_ctx *ctx, uint32_t domid,
+                                    uint32_t vcpuid, libxl_bitmap *cpumap)
+{
+    return libxl_set_vcpuaffinity(ctx, domid, vcpuid, cpumap, NULL);
+}
+
+static inline
+int libxl_set_vcpuaffinity_all_0x040200(libxl_ctx *ctx, uint32_t domid,
+                                        unsigned int max_vcpus,
+                                        libxl_bitmap *cpumap)
+{
+    return libxl_set_vcpuaffinity_all(ctx, domid, max_vcpus, cpumap, NULL);
+}
+
+#define libxl_set_vcpuaffinity libxl_set_vcpuaffinity_0x040200
+#define libxl_set_vcpuaffinity_all libxl_set_vcpuaffinity_all_0x040200
+
+#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,

tools/libxl/libxl_create.c +6 -0

index fe7ba0d..58f80df 100644
@@ -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) {

tools/libxl/libxl_dom.c +2 -1

index 72489f8..4bfed60 100644
@@ -236,7 +236,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);
 
      xc_domain_setmaxmem(ctx->xch, domid, info->target_memkb + LIBXL_MAXMEM_CONSTANT);
      xs_domid = xs_read(ctx->xsh, XBT_NULL, "/tool/xenstored/domid", NULL);

tools/libxl/libxl_types.idl +3 -1

index cba8eff..48699c9 100644
@@ -298,6 +298,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),

@@ -510,7 +511,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", [

tools/libxl/libxl_utils.h +24 -1

index b11cf28..c205dd7 100644
@@ -90,7 +90,7 @@ static inline void libxl_bitmap_set_none(libxl_bitmap *bitmap)
  {
      memset(bitmap->map, 0, bitmap->size);
  }
-static inline int libxl_bitmap_cpu_valid(libxl_bitmap *bitmap, int bit)
+static inline int libxl_bitmap_valid(const libxl_bitmap *bitmap, int bit)
  {
      return bit >= 0 && bit < (bitmap->size * 8);
  }

@@ -98,6 +98,29 @@ 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;
+    for (i = 0; i < nr_bits; i++) {
+        /* If overflowing one of the bitmaps, we call them different */
+        if (!libxl_bitmap_valid(ba, i) || !libxl_bitmap_valid(bb, i))
+            return 0;
+        if (libxl_bitmap_test(ba, i) != libxl_bitmap_test(bb, i))
+            return 0;
+    }
+    return 1;
+}
+
+static inline int libxl_bitmap_cpu_valid(libxl_bitmap *cpumap, int cpu)
+{
+    return libxl_bitmap_valid(cpumap, cpu);
+}
+
  int libxl_cpu_bitmap_alloc(libxl_ctx *ctx,
                             libxl_bitmap *cpumap,
                             int max_cpus);

tools/libxl/xl_cmdimpl.c +3 -3

index 7b4d058..17fffdd 100644
@@ -2222,7 +2222,7 @@ start:
              } else {
                  libxl_bitmap_set_any(&vcpu_cpumap);
              }
-            if (libxl_set_vcpuaffinity(ctx, domid, i, &vcpu_cpumap)) {
+            if (libxl_set_vcpuaffinity(ctx, domid, i, &vcpu_cpumap, NULL)) {
                  fprintf(stderr, "setting affinity failed on vcpu `%d'.\n", i);
                  libxl_bitmap_dispose(&vcpu_cpumap);
                  free(vcpu_to_pcpu);

@@ -4630,7 +4630,7 @@ static int vcpupin(uint32_t domid, const char *vcpu, char *cpu)
      }
 
      if (vcpuid != -1) {
-        if (libxl_set_vcpuaffinity(ctx, domid, vcpuid, &cpumap) == -1) {
+        if (libxl_set_vcpuaffinity(ctx, domid, vcpuid, &cpumap, NULL)) {
              fprintf(stderr, "Could not set affinity for vcpu `%u'.\n", vcpuid);
              goto out;
          }

@@ -4642,7 +4642,7 @@ static int vcpupin(uint32_t domid, const char *vcpu, char *cpu)
          }
          for (i = 0; i < nb_vcpu; i++) {
              if (libxl_set_vcpuaffinity(ctx, domid, vcpuinfo[i].vcpuid,
-                                       &cpumap) == -1) {
+                                       &cpumap, NULL)) {
                  fprintf(stderr, "libxl_set_vcpuaffinity failed"
                                  " on vcpu `%u'.\n", vcpuinfo[i].vcpuid);
              }

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 is the first change breaking the libxl ABI, so it bumps the MAJOR.

Signed-off-by Dario Faggioli <[email protected]>

---
Changes from v3:
* only introduce one LIBXL_HAVE_ symbol for soft affinity, as requested during review;
* use LIBXL_API_VERSION instead of having multiple version of the same function, as suggested during review;
* use libxl_get_nr_cpus() rather than libxl_get_cputopology(), as suggested during review;
* use LOGE() instead of LIBXL__LOG_ERRNO(), as requested during review;
* kill the flags and use just one _set_vcpuaffinity() function with two cpumaps, allowing either of them to be NULL, as suggested during review;
* avoid overflowing the bitmaps in libxl_bitmap_equal(), as suggested during review.

Changes from v2:
* interface completely redesigned, as discussed during review.

---
 tools/libxl/Makefile        |    2 +
 tools/libxl/libxl.c         |   79 ++++++++++++++++++++++++++++++++++++++-----
 tools/libxl/libxl.h         |   38 ++++++++++++++++++++-
 tools/libxl/libxl_create.c  |    6 +++
 tools/libxl/libxl_dom.c     |    3 +-
 tools/libxl/libxl_types.idl |    4 ++
 tools/libxl/libxl_utils.h   |   25 +++++++++++++-
 tools/libxl/xl_cmdimpl.c    |    6 ++-
 8 files changed, 145 insertions(+), 18 deletions(-)

tools/libxl/Makefile +1 -1

index d8495bb..1410c44 100644
@@ -5,7 +5,7 @@
 XEN_ROOT = $(CURDIR)/../..
 include $(XEN_ROOT)/tools/Rules.mk
 
-MAJOR = 4.3
+MAJOR = 4.4
 MINOR = 0
 
 XLUMAJOR = 4.3

tools/libxl/libxl.c +0 -0

index 961d55e..4b871d7 100644

tools/libxl/libxl.c +51 -5

         if (xc_vcpu_getinfo(ctx->xch, domid, *nb_vcpu, &vcpuinfo) == -1) {
             LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting vcpu info");
             return NULL;

@@ -4548,6 +4550,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;

@@ -4559,28 +4567,81 @@ libxl_vcpuinfo *libxl_list_vcpu(libxl_ctx *ctx, uint32_t domid,
 }
 
 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)
 {
-    if (xc_vcpu_setaffinity(ctx->xch, domid, vcpuid, cpumap->map,
-                            XEN_VCPUAFFINITY_HARD, NULL)) {
-        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "setting vcpu affinity");
-        return ERROR_FAIL;
+    GC_INIT(ctx);
+    libxl_bitmap ecpumap;
+    int nr_cpus = 0, rc;
+
+    if (!cpumap_hard && !cpumap_soft)
+        return ERROR_INVAL;
+
+    rc = libxl_cpu_bitmap_alloc(ctx, &ecpumap, 0);
+    if (rc)
+        return rc;
+
+    nr_cpus = libxl_get_nr_cpus(ctx);
+    if (nr_cpus <= 0) {
+        rc = ERROR_FAIL;
+        goto out;
     }
+
+    if (cpumap_hard && xc_vcpu_setaffinity(ctx->xch, domid, vcpuid,
+                                           cpumap_hard->map,
+                                           XEN_VCPUAFFINITY_HARD,
+                                           ecpumap.map)) {
+        LOGE(ERROR, "setting vcpu hard affinity");
+        rc = ERROR_FAIL;
+        goto out;
+    }
+    if (cpumap_hard && !libxl_bitmap_equal(cpumap_hard, &ecpumap, nr_cpus))
+        LOG(DEBUG, "New hard affinity for vcpu %d contains unreachable cpus",
+            vcpuid);
+
+    if (cpumap_soft && xc_vcpu_setaffinity(ctx->xch, domid, vcpuid,
+                                           cpumap_soft->map,
+                                           XEN_VCPUAFFINITY_SOFT,
+                                           ecpumap.map)) {
+        LOGE(ERROR, "setting vcpu soft affinity");
+        rc = ERROR_FAIL;
+        goto out;
+    }
+    if (cpumap_soft && !libxl_bitmap_equal(cpumap_soft, &ecpumap, nr_cpus))
+        LOG(DEBUG, "New soft affinity for vcpu %d contains unreachable cpus",
+            vcpuid);
+
+    /*
+     * When setting hard affinity, it is guaranteed that the result will not
+     * be empty, so we need to check for that only if soft.

tools/libxl/libxl.c +8 -4

 }
 
 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)
 {
+    GC_INIT(ctx);
     int i, rc = 0;
 
     for (i = 0; i < max_vcpus; i++) {
-        if (libxl_set_vcpuaffinity(ctx, domid, i, cpumap)) {
-            LIBXL__LOG(ctx, LIBXL__LOG_WARNING,
-                       "failed to set affinity for %d", i);
+        if (libxl_set_vcpuaffinity(ctx, domid, i, cpumap_hard, cpumap_soft)) {
+            LOG(WARN, "failed to set affinity for %d", i);
             rc = ERROR_FAIL;
         }
     }
+
+    GC_FREE;
     return rc;
 }
 

tools/libxl/libxl.h +0 -0

index 2fab5ba..9864927 100644

tools/libxl/libxl.h +19 -2

+ */
+#define LIBXL_HAVE_SOFT_AFFINITY 1
+
+/*
  * 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

@@ -994,9 +1002,35 @@ 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 < 0x040400
+
+static inline
+int libxl_set_vcpuaffinity_0x040200(libxl_ctx *ctx, uint32_t domid,
+                                    uint32_t vcpuid, libxl_bitmap *cpumap)
+{
+    return libxl_set_vcpuaffinity(ctx, domid, vcpuid, cpumap, NULL);
+}
+

tools/libxl/libxl.h +3 -0

+
+#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,

tools/libxl/libxl_create.c +6 -0

index fe7ba0d..58f80df 100644
@@ -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) {

tools/libxl/libxl_dom.c +2 -1

index 72489f8..4bfed60 100644
@@ -236,7 +236,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);
 
     xc_domain_setmaxmem(ctx->xch, domid, info->target_memkb + LIBXL_MAXMEM_CONSTANT);
     xs_domid = xs_read(ctx->xsh, XBT_NULL, "/tool/xenstored/domid", NULL);

tools/libxl/libxl_types.idl +3 -1

index cba8eff..48699c9 100644
@@ -298,6 +298,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),

@@ -510,7 +511,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", [

tools/libxl/libxl_utils.h +0 -0

index b11cf28..c205dd7 100644

tools/libxl/libxl_utils.h +23 -0

 {
     return bit >= 0 && bit < (bitmap->size * 8);
 }

@@ -98,6 +98,29 @@ 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;
+    for (i = 0; i < nr_bits; i++) {
+        /* If overflowing one of the bitmaps, we call them different */
+        if (!libxl_bitmap_valid(ba, i) || !libxl_bitmap_valid(bb, i))
+            return 0;
+        if (libxl_bitmap_test(ba, i) != libxl_bitmap_test(bb, i))
+            return 0;
+    }
+    return 1;
+}
+
+static inline int libxl_bitmap_cpu_valid(libxl_bitmap *cpumap, int cpu)
+{
+    return libxl_bitmap_valid(cpumap, cpu);
+}
+
 int libxl_cpu_bitmap_alloc(libxl_ctx *ctx,
                            libxl_bitmap *cpumap,
                            int max_cpus);

tools/libxl/xl_cmdimpl.c +3 -3

index 7b4d058..17fffdd 100644
@@ -2222,7 +2222,7 @@ start:
             } else {
                 libxl_bitmap_set_any(&vcpu_cpumap);
             }
-            if (libxl_set_vcpuaffinity(ctx, domid, i, &vcpu_cpumap)) {
+            if (libxl_set_vcpuaffinity(ctx, domid, i, &vcpu_cpumap, NULL)) {
                 fprintf(stderr, "setting affinity failed on vcpu `%d'.\n", i);
                 libxl_bitmap_dispose(&vcpu_cpumap);
                 free(vcpu_to_pcpu);

@@ -4630,7 +4630,7 @@ static int vcpupin(uint32_t domid, const char *vcpu, char *cpu)
     }
 
     if (vcpuid != -1) {
-        if (libxl_set_vcpuaffinity(ctx, domid, vcpuid, &cpumap) == -1) {
+        if (libxl_set_vcpuaffinity(ctx, domid, vcpuid, &cpumap, NULL)) {
             fprintf(stderr, "Could not set affinity for vcpu `%u'.\n", vcpuid);
             goto out;
         }

@@ -4642,7 +4642,7 @@ static int vcpupin(uint32_t domid, const char *vcpu, char *cpu)
         }
         for (i = 0; i < nb_vcpu; i++) {
             if (libxl_set_vcpuaffinity(ctx, domid, vcpuinfo[i].vcpuid,
-                                       &cpumap) == -1) {
+                                       &cpumap, NULL)) {
                 fprintf(stderr, "libxl_set_vcpuaffinity failed"
                                 " on vcpu `%u'.\n", vcpuinfo[i].vcpuid);
             }

tools/libxl/libxl.c libxl_vcpuinfo *libxl_list_vcpu(libxl_ctx *ctx, uint32_t domid, · line 4542

Quoted 10 lines · Dario Faggioli

tools/libxl/libxl.c +2 -0

index 961d55e..4b871d7 100644
@@ -4538,6 +4538,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;

Leaks ptr and ptr->cpumap...

Quoted 35 lines

tools/libxl/libxl.c +25 -2

+ */
+#define LIBXL_HAVE_SOFT_AFFINITY 1
+
+/*
  * 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

@@ -994,9 +1002,35 @@ 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 < 0x040400
+
+static inline
+int libxl_set_vcpuaffinity_0x040200(libxl_ctx *ctx, uint32_t domid,
+                                    uint32_t vcpuid, libxl_bitmap *cpumap)
+{
+    return libxl_set_vcpuaffinity(ctx, domid, vcpuid, cpumap, NULL);
+}
+
+static inline
+int libxl_set_vcpuaffinity_all_0x040200(libxl_ctx *ctx, uint32_t domid,
+                                        unsigned int max_vcpus,
+                                        libxl_bitmap *cpumap)
+{
+    return libxl_set_vcpuaffinity_all(ctx, domid, max_vcpus, cpumap, NULL);

tools/libxl/libxl.c int libxl_userdata_retrieve(libxl_ctx *ctx, uint32_t domid, · line 1030

Quoted 4 lines

tools/libxl/libxl.c +4 -0

+}
+
+#define libxl_set_vcpuaffinity libxl_set_vcpuaffinity_0x040200
+#define libxl_set_vcpuaffinity_all libxl_set_vcpuaffinity_all_0x040200

This looks correct, thanks. It could also have jsut been #defined since the inline is only used in the libxl_domain_create_restore_0x040200 case because there is a local variable.

Quoted 35 lines · Ian Campbell

Quoted 35 lines

+ */

tools/libxl/libxl.h +24 -2

+#define LIBXL_HAVE_SOFT_AFFINITY 1
+
+/*
  * 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

@@ -994,9 +1002,35 @@ 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 < 0x040400
+
+static inline
+int libxl_set_vcpuaffinity_0x040200(libxl_ctx *ctx, uint32_t domid,
+                                    uint32_t vcpuid, libxl_bitmap *cpumap)
+{
+    return libxl_set_vcpuaffinity(ctx, domid, vcpuid, cpumap, NULL);
+}
+
+static inline
+int libxl_set_vcpuaffinity_all_0x040200(libxl_ctx *ctx, uint32_t domid,
+                                        unsigned int max_vcpus,
+                                        libxl_bitmap *cpumap)
+{
+    return libxl_set_vcpuaffinity_all(ctx, domid, max_vcpus, cpumap, NULL);

Quoted 9 lines

tools/libxl/libxl.h int libxl_userdata_retrieve(libxl_ctx *ctx, uint32_t domid, · line 1030

Quoted 4 lines

tools/libxl/libxl.h +4 -0

+}
+
+#define libxl_set_vcpuaffinity libxl_set_vcpuaffinity_0x040200
+#define libxl_set_vcpuaffinity_all libxl_set_vcpuaffinity_all_0x040200

This looks correct, thanks. It could also have jsut been #defined since the inline is only used in the libxl_domain_create_restore_0x040200 case because there is a local variable.