From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wei Liu Subject: Re: [PATCH for-xen-4.5] libxl: Allow copying smaller bitmap into a larger one Date: Fri, 21 Nov 2014 11:12:44 +0000 Message-ID: <20141121111244.GA18698@zion.uk.xensource.com> References: <1416518854-5284-1-git-send-email-boris.ostrovsky@oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <1416518854-5284-1-git-send-email-boris.ostrovsky@oracle.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Boris Ostrovsky Cc: wei.liu2@citrix.com, ian.campbell@citrix.com, stefano.stabellini@eu.citrix.com, ian.jackson@eu.citrix.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org 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) In any case I think this is a regression and should be fixed for 4.5. Wei. > Signed-off-by: Boris Ostrovsky > --- > 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