[Xen-devel] [PATCH for-4.5] libxl: allow copying between bitmaps of different sizes

[Wei Liu <wei.liu2@xxxxxxxxxx>]

When parsing bitmap objects JSON parser will create libxl_bitmap map of the
smallest size needed.

This can cause problems when saved image file specifies CPU affinity.  For
example, if 'vcpu_hard_affinity' in the saved image has only the first CPU
specified, just a single byte will be allocated and libxl_bitmap->size will be
set to 1.

This will result in assertion in libxl_set_vcpuaffinity()->libxl_bitmap_copy()
since the destination bitmap is created for maximum number of CPUs.

We could allocate that bitmap of the same size as the source, however, it is
later passed to xc_vcpu_setaffinity() which expects it to be sized to the max
number of CPUs

To fix this issue, introduce an internal function to allowing copying between
bitmaps of different sizes. Note that this function is only used in
libxl_set_vcpuaffinity at the moment. Though NUMA placement logic invokes
libxl_bitmap_copy as well there's no need to replace those invocations.  NUMA
placement logic comes into effect when no vcpu / node pinning is provided, so
it always operates on bitmap of the same sizes (that is, size of maximum
number of cpus /nodes).

Reported-and-tested-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
Reviewed-by: Dario Faggioli <dario.faggioli@xxxxxxxxxx>
Cc: Ian Campbell <ian.campbell@xxxxxxxxxx>
Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
Cc: Konrad Wilk <konrad.wilk@xxxxxxxxxx>
---
This fixes a regression for 4.5. Can't think of obvious risk.
---
 tools/libxl/libxl.c          |    4 ++--
 tools/libxl/libxl_internal.h |   11 +++++++++++
 tools/libxl/libxl_utils.c    |   15 +++++++++++++++
 3 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index de23fec..1e9da10 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -5320,7 +5320,7 @@ int libxl_set_vcpuaffinity(libxl_ctx *ctx, uint32_t 
domid, uint32_t vcpuid,
         if (rc)
             goto out;
 
-        libxl_bitmap_copy(ctx, &hard, cpumap_hard);
+        libxl__bitmap_copy_best_effort(gc, &hard, cpumap_hard);
         flags = XEN_VCPUAFFINITY_HARD;
     }
     if (cpumap_soft) {
@@ -5328,7 +5328,7 @@ int libxl_set_vcpuaffinity(libxl_ctx *ctx, uint32_t 
domid, uint32_t vcpuid,
         if (rc)
             goto out;
 
-        libxl_bitmap_copy(ctx, &soft, cpumap_soft);
+        libxl__bitmap_copy_best_effort(gc, &soft, cpumap_soft);
         flags |= XEN_VCPUAFFINITY_SOFT;
     }
 
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 4361421..a38f695 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -3617,6 +3617,17 @@ static inline void libxl__update_config_vtpm(libxl__gc 
*gc,
         libxl_device_##type##_copy(CTX, DA_p, (dev));           \
     })
 
+/* This function copies X bytes from source to destination bitmap,
+ * where X is the smaller of the two sizes.
+ *
+ * If destination's size is larger than source, the extra bytes are
+ * untouched.
+ *
+ * XXX This is introduced to fix a regression for 4.5. It shall
+ * be revisited in 4.6 time frame.
+ */
+void libxl__bitmap_copy_best_effort(libxl__gc *gc, libxl_bitmap *dptr,
+                                    const libxl_bitmap *sptr);
 #endif
 
 /*
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 58df4f3..3e1ba17 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -614,6 +614,21 @@ void libxl_bitmap_copy(libxl_ctx *ctx, libxl_bitmap *dptr,
     memcpy(dptr->map, sptr->map, sz * sizeof(*dptr->map));
 }
 
+/* This function copies X bytes from source to destination bitmap,
+ * where X is the smaller of the two sizes.
+ *
+ * If destination's size is larger than source, the extra bytes are
+ * untouched.
+ */
+void libxl__bitmap_copy_best_effort(libxl__gc *gc, libxl_bitmap *dptr,
+                                    const libxl_bitmap *sptr)
+{
+    int sz;
+
+    sz = dptr->size < sptr->size ? dptr->size : sptr->size;
+    memcpy(dptr->map, sptr->map, sz * sizeof(*dptr->map));
+}
+
 void libxl_bitmap_copy_alloc(libxl_ctx *ctx,
                              libxl_bitmap *dptr,
                              const libxl_bitmap *sptr)
-- 
1.7.10.4


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