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

[Boris Ostrovsky]

On 11/21/2014 06:12 AM, Wei Liu wrote:

On Thu, Nov 20, 2014 at 04:27:34PM -0500, Boris Ostrovsky wrote:

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

Instead, we should allow copying the (smaller) bitmap read by the parser
and keep the rest of bytes in the destination map unmodified (zero in
this case)

What about copying large bitmap to a smaller one? Say, you migrate to
a host whose number of cpus is smaller than the size of the source
bitmap. Is this a valid use case?

Should we have a "best effort" copy function? That is,

   size = min(source_size, destination_size)
   copy(source, destination, size)

Right, I haven't thought about it for the reversed case.

-boris

In any case I think this is a regression and should be fixed for 4.5.

Wei.

Signed-off-by: Boris Ostrovsky <boris.ostrov...@oracle.com>
---
  tools/libxl/libxl.c       |    4 ++--
  tools/libxl/libxl_utils.c |    7 +++++++
  tools/libxl/libxl_utils.h |    2 ++
  3 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index f7961f6..84fd2ca 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -5319,7 +5319,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_partial(ctx, &hard, cpumap_hard);
          flags = XEN_VCPUAFFINITY_HARD;
      }
      if (cpumap_soft) {
@@ -5327,7 +5327,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_partial(ctx, &soft, cpumap_soft);
          flags |= XEN_VCPUAFFINITY_SOFT;
      }

diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c

index 58df4f3..2a08bef 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -614,6 +614,13 @@ void libxl_bitmap_copy(libxl_ctx *ctx, libxl_bitmap *dptr,
      memcpy(dptr->map, sptr->map, sz * sizeof(*dptr->map));
  }

+void libxl_bitmap_copy_partial(libxl_ctx *ctx, libxl_bitmap *dptr,

+                               const libxl_bitmap *sptr)
+{
+    assert(dptr->size >= sptr->size);
+    memcpy(dptr->map, sptr->map, sptr->size * sizeof(*dptr->map));
+}
+
  void libxl_bitmap_copy_alloc(libxl_ctx *ctx,
                               libxl_bitmap *dptr,
                               const libxl_bitmap *sptr)
diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
index 117b229..d4d0a51 100644
--- a/tools/libxl/libxl_utils.h
+++ b/tools/libxl/libxl_utils.h
@@ -80,6 +80,8 @@ void libxl_bitmap_copy_alloc(libxl_ctx *ctx, libxl_bitmap 
*dptr,
                               const libxl_bitmap *sptr);
  void libxl_bitmap_copy(libxl_ctx *ctx, libxl_bitmap *dptr,
                         const libxl_bitmap *sptr);
+void libxl_bitmap_copy_partial(libxl_ctx *ctx, libxl_bitmap *dptr,
+                               const libxl_bitmap *sptr);
  int libxl_bitmap_is_full(const libxl_bitmap *bitmap);
  int libxl_bitmap_is_empty(const libxl_bitmap *bitmap);
  int libxl_bitmap_test(const libxl_bitmap *bitmap, int bit);
--
1.7.1

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