Re: [Xen-devel] [PATCH for-xen-4.5] libxl: Allow copying smaller bitmap into a larger one

[Wei Liu <wei.liu2@xxxxxxxxxx>]

And here it is.

Boris, can you give it a shot?

---8<---
From 77531e31d239887b9f36c03e434300bc30683092 Mon Sep 17 00:00:00 2001
From: Wei Liu <wei.liu2@xxxxxxxxxx>
Date: Tue, 25 Nov 2014 10:59:47 +0000
Subject: [PATCH] libxl: allow copying between bitmaps of different sizes

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 invoke
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-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
Cc: Ian Campbell <ian.campbell@xxxxxxxxxx>
Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
Cc: Dario Faggioli <dario.faggioli@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