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

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

[Boris Ostrovsky]

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)

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@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

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

[Wei Liu]

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 <boris.ostrovsky@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

Re: [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.ostrovsky@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

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

[Dario Faggioli]

[-- Attachment #1.1: Type: text/plain, Size: 1426 bytes --]

On Thu, 2014-11-20 at 16:27 -0500, Boris Ostrovsky wrote:

> 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));
> +}
> +
Looking at other callers of libxl_bitmap_copy(), I think something like
this makes sense for pretty much all of them.

And even abstracting from them, and thinking to how a function like
'libxl_bitmap_copy()' this should behave, copying only the "common part"
makes sense to me.

So, should we make libxl_bitmap_copy() behave like implemented above,
rather than introducing a new function. I know this is public stable
API, but I think this is a fine behavioral change, isn't it?

Regards,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

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

[Boris Ostrovsky]

On 11/21/2014 06:26 AM, Dario Faggioli wrote:
> On Thu, 2014-11-20 at 16:27 -0500, Boris Ostrovsky wrote:
>
>> 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));
>> +}
>> +
> Looking at other callers of libxl_bitmap_copy(), I think something like
> this makes sense for pretty much all of them.
>
> And even abstracting from them, and thinking to how a function like
> 'libxl_bitmap_copy()' this should behave, copying only the "common part"
> makes sense to me.
>
> So, should we make libxl_bitmap_copy() behave like implemented above,
> rather than introducing a new function. I know this is public stable
> API, but I think this is a fine behavioral change, isn't it?

I did consider this but wasn't sure about it from the point of view of 
API behavior. If people feel it's OK to slightly change behavior here 
I'd rather go this route.


-boris

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

[Wei Liu]

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)
> 

Here is some more thoughts on this issue:

This API is used by VCPU placement and NUMA placement logic in libxl. To
fix the breakage, we don't necessary need to expose new API or change
API behaviour, we only need to have a internal function to do it.

The reversed case (large bitmap to small one) is not valid in libxl, as
in the pinning will fail. But bitmap copy happens before that, we would
still need to deal with that. Dario, can you provide some input on
the expected behaviour?

The partial copy function should explicitly zero-out all remaining bits.

Wei.

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

[Wei Liu]

CC'ing Dario...

On Mon, Nov 24, 2014 at 10:41:27AM +0000, 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)
> > 
> 
> Here is some more thoughts on this issue:
> 
> This API is used by VCPU placement and NUMA placement logic in libxl. To
> fix the breakage, we don't necessary need to expose new API or change
> API behaviour, we only need to have a internal function to do it.
> 
> The reversed case (large bitmap to small one) is not valid in libxl, as
> in the pinning will fail. But bitmap copy happens before that, we would
> still need to deal with that. Dario, can you provide some input on
> the expected behaviour?
> 
> The partial copy function should explicitly zero-out all remaining bits.
> 
> Wei.

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

[Boris Ostrovsky]

On 11/24/2014 05:47 AM, Wei Liu wrote:
> CC'ing Dario...
>
> On Mon, Nov 24, 2014 at 10:41:27AM +0000, 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)
>>>
>> Here is some more thoughts on this issue:
>>
>> This API is used by VCPU placement and NUMA placement logic in libxl. To
>> fix the breakage, we don't necessary need to expose new API or change
>> API behaviour, we only need to have a internal function to do it.
>>
>> The reversed case (large bitmap to small one) is not valid in libxl, as
>> in the pinning will fail. But bitmap copy happens before that, we would
>> still need to deal with that. Dario, can you provide some input on
>> the expected behaviour?

I understand that hypervisor will ignore bits that are beyond what it 
knows about --- see xenctl_bitmap_to_bitmap(). And libxl will will issue 
a warning. But only if byte-sized bitmaps are the same. If they are not 
will will pop the assertion (in debug builds).

>>
>> The partial copy function should explicitly zero-out all remaining bits.

I actually thought that partial copy function should do just that --- 
copy bits that it has and leave others unchanged. The caller, if desires 
so, should have cleared the mask prior to the call. (This is for the 
case when destination is larger than source, of course).


-boris

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

[Dario Faggioli]

[-- Attachment #1.1: Type: text/plain, Size: 1237 bytes --]

On Mon, 2014-11-24 at 10:48 -0500, Boris Ostrovsky wrote:
> On 11/24/2014 05:47 AM, Wei Liu wrote:

> >>
> >> The partial copy function should explicitly zero-out all remaining bits.
> 
> I actually thought that partial copy function should do just that --- 
> copy bits that it has and leave others unchanged. The caller, if desires 
> so, should have cleared the mask prior to the call. (This is for the 
> case when destination is larger than source, of course).
> 
Agreed.

Anyway, AFAIU Wei's proposal, he's saying that this new _copy_partial()
function can be an internal one, i.e., not part of the public  API, or
am I wrong, Wei?

If yes, I agree with that.

This leaves the question of whether we should change the behavior of the
publicly exposed libxl_bitmap_copy(), which I'm still not sure about,
but I guess, if we keep this internal, we can defer thinking to that to
some other period not in between RCs?

Thanks and Regards,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

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

[Wei Liu]

On Tue, Nov 25, 2014 at 10:42:58AM +0100, Dario Faggioli wrote:
> On Mon, 2014-11-24 at 10:48 -0500, Boris Ostrovsky wrote:
> > On 11/24/2014 05:47 AM, Wei Liu wrote:
> 
> > >>
> > >> The partial copy function should explicitly zero-out all remaining bits.
> > 
> > I actually thought that partial copy function should do just that --- 
> > copy bits that it has and leave others unchanged. The caller, if desires 
> > so, should have cleared the mask prior to the call. (This is for the 
> > case when destination is larger than source, of course).
> > 
> Agreed.
> 
> Anyway, AFAIU Wei's proposal, he's saying that this new _copy_partial()
> function can be an internal one, i.e., not part of the public  API, or
> am I wrong, Wei?
> 
> If yes, I agree with that.
> 

You're right. I want to make it internal so that we don't need to worry
about the public API for now.

> This leaves the question of whether we should change the behavior of the
> publicly exposed libxl_bitmap_copy(), which I'm still not sure about,
> but I guess, if we keep this internal, we can defer thinking to that to
> some other period not in between RCs?
> 

Yes.

Wei.

> Thanks and Regards,
> Dario
> 
> -- 
> <<This happens because I choose it to happen!>> (Raistlin Majere)
> -----------------------------------------------------------------
> Dario Faggioli, Ph.D, http://about.me/dario.faggioli
> Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
> 

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

[Wei Liu]

On Mon, Nov 24, 2014 at 10:48:19AM -0500, Boris Ostrovsky wrote:
> On 11/24/2014 05:47 AM, Wei Liu wrote:
> >CC'ing Dario...
> >
> >On Mon, Nov 24, 2014 at 10:41:27AM +0000, 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)
> >>>
> >>Here is some more thoughts on this issue:
> >>
> >>This API is used by VCPU placement and NUMA placement logic in libxl. To
> >>fix the breakage, we don't necessary need to expose new API or change
> >>API behaviour, we only need to have a internal function to do it.
> >>
> >>The reversed case (large bitmap to small one) is not valid in libxl, as
> >>in the pinning will fail. But bitmap copy happens before that, we would
> >>still need to deal with that. Dario, can you provide some input on
> >>the expected behaviour?
> 
> I understand that hypervisor will ignore bits that are beyond what it knows
> about --- see xenctl_bitmap_to_bitmap(). And libxl will will issue a
> warning. But only if byte-sized bitmaps are the same. If they are not will
> will pop the assertion (in debug builds).
> 

After a discussion on IRC with Dario, migrating from large host to small
one is legit use case. That means we need to take care about the
reversed case as well.

BTW, are you on xen-devel@freenode? It would be much faster if we can
discuss on IRC.

> >>
> >>The partial copy function should explicitly zero-out all remaining bits.
> 
> I actually thought that partial copy function should do just that --- copy
> bits that it has and leave others unchanged. The caller, if desires so,
> should have cleared the mask prior to the call. (This is for the case when
> destination is larger than source, of course).
> 

This is OK as long as this behaviour is documented.

I will prepare a patch shortly for you to test.

Wei.

> 
> -boris

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

[Wei Liu]

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@citrix.com>
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@oracle.com>
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Dario Faggioli <dario.faggioli@citrix.com>
---
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

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

[Dario Faggioli]

[-- Attachment #1.1: Type: text/plain, Size: 2199 bytes --]

On Tue, 2014-11-25 at 11:15 +0000, Wei Liu wrote:
> 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@citrix.com>
> 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@oracle.com>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Dario Faggioli <dario.faggioli@citrix.com>
>
If this end up being the approach, it can have the following:

Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

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

[Boris Ostrovsky]

On 11/25/2014 06:41 AM, Dario Faggioli wrote:
> On Tue, 2014-11-25 at 11:15 +0000, Wei Liu wrote:
>> 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@citrix.com>
>> 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@oracle.com>
>> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
>> Cc: Ian Campbell <ian.campbell@citrix.com>
>> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
>> Cc: Dario Faggioli <dario.faggioli@citrix.com>
>>
> If this end up being the approach, it can have the following:
>
> Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>

Tested-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>

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

[Konrad Rzeszutek Wilk]

On Tue, Nov 25, 2014 at 09:57:54AM -0500, Boris Ostrovsky wrote:
> On 11/25/2014 06:41 AM, Dario Faggioli wrote:
> >On Tue, 2014-11-25 at 11:15 +0000, Wei Liu wrote:
> >>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@citrix.com>
> >>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@oracle.com>
> >>Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> >>Cc: Ian Campbell <ian.campbell@citrix.com>
> >>Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> >>Cc: Dario Faggioli <dario.faggioli@citrix.com>
> >>
> >If this end up being the approach, it can have the following:
> >
> >Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>
> 
> Tested-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>

Release-Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

Thank you!
> 
> 

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

[Ian Campbell]

On Tue, 2014-11-25 at 10:54 -0500, Konrad Rzeszutek Wilk wrote:
> > >>Reported-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> > >>Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > >>Cc: Ian Campbell <ian.campbell@citrix.com>
> > >>Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > >>Cc: Dario Faggioli <dario.faggioli@citrix.com>
> > >>
> > >If this end up being the approach, it can have the following:
> > >
> > >Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>
> > 
> > Tested-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> 
> Release-Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

Acked +Applied.

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

[Ian Campbell]

On Tue, 2014-11-25 at 11:15 +0000, Wei Liu wrote:
> This will result in assertion in libxl_set_vcpuaffinity()->libxl_bitmap_copy()
> since the destination bitmap is created for maximum number of CPUs.

FYI I'm also seeing this with libvirt (on ARM, but I don't think that
matters) when the guest XML uses:
        <vcpu placement='static' cpuset='0-7'>1</vcpu>
Results in:
        libvirtd: libxl_utils.c:612: libxl_bitmap_copy: Assertion `dptr->size == sptr->size' failed.
        
        Program received signal SIGABRT, Aborted.
        [Switching to Thread 0xb67f9420 (LWP 3881)]
        0xb6ab7f96 in ?? () from /lib/arm-linux-gnueabihf/libc.so.6
        (gdb) bt
        #0  0xb6ab7f96 in ?? () from /lib/arm-linux-gnueabihf/libc.so.6
        #1  0xb6ac5f8a in raise () from /lib/arm-linux-gnueabihf/libc.so.6
        #2  0xb6ac8428 in abort () from /lib/arm-linux-gnueabihf/libc.so.6
        #3  0xb6ac101e in __assert_fail () from /lib/arm-linux-gnueabihf/libc.so.6
        #4  0xb16caeb4 in libxl_bitmap_copy (ctx=<optimized out>, dptr=<optimized out>, sptr=<optimized out>) at libxl_utils.c:612
        #5  0xb16af15c in libxl_set_vcpuaffinity (ctx=0x2, domid=3065494508, vcpuid=0, cpumap_hard=0xb67f8668, cpumap_soft=0x0) at libxl.c:5323
        #6  0xb17195ae in libxlDomainSetVcpuAffinities () from /opt/libvirt/lib/libvirt/connection-driver/libvirt_driver_libxl.so
        #7  0xb1719cf2 in libxlDomainStart () from /opt/libvirt/lib/libvirt/connection-driver/libvirt_driver_libxl.so
        #8  0xb171b7c2 in libxlDomainCreateXML () from /opt/libvirt/lib/libvirt/connection-driver/libvirt_driver_libxl.so
        #9  0xb6d28630 in virDomainCreateXML () from /opt/libvirt/lib/libvirt.so.0
        [...snip...]
        
libxlDomainSetVcpuAffinities does:
    libxl_bitmap map;
    [...]

        map.size = cpumaplen;
        map.map = cpumap;

        if (libxl_set_vcpuaffinity(priv->ctx, def->id, vcpu, &map) != 0) {

http://libvirt.org/git/?p=libvirt.git;a=blob;f=src/libxl/libxl_domain.c;h=9c622910547ada174a3d787c76c8bb076c9a75c3;hb=HEAD#l1059

Applying this libxl patch fixes things.

I don't think we've explicitly outlawed looking "inside" a libxl_bitmap
in this way anywhere, so I think libvirtd is within its rights to do so,
but it does highlight that we need to be mindful within libxl that
people may not be using libxl_cpu_bitmap_alloc.

> 
> 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@oracle.com>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Dario Faggioli <dario.faggioli@citrix.com>
> ---
> 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)