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

[Boris Ostrovsky <boris.ostrovsky@oracle.com>]

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