Jeremy Fitzhardinge wrote:
On 10/30/2013 12:04 PM, Eric Blake wrote:
> On 10/30/2013 01:00 PM, Jeremy Fitzhardinge wrote:
>
>> On 10/30/2013 11:37 AM, Eric Blake wrote:
>>
>>> On 10/30/2013 11:38 AM, Jeremy Fitzhardinge wrote:
>>>
>>>> libxl: fix dubious cpumask handling in libxlDomainSetVcpuAffinities
>>>>
>>>> Rather than casting the virBitmap pointer to uint8_t* and then
>>>>
using
>>>> the structure contents as a byte array, use the virBitmap API to
>>>> determine
>>>> the bitmap size and test each bit.
>>>>
>>> Hmm, we already have virBitmapToData for converting from a virBitmap to
>>> a uint8_t; using that would be much faster for populating cpumap than
>>> doing a per-bit iteration over cpumask.
>>>
>> I looked at that, but I couldn't see it being any more efficient given
>> that we're typically talking about a small number of (v)CPUs.
>>
> Efficiency isn't my concern. Maintainability is. It's better to reuse
> existing functions instead of risking open-coding it wrong. We have a
> testsuite over our existing functions, but not over your open-coding.
>
Well, that was also a consideration, but using virBitmapToData would be
more complex. The same loop would exist, but I'd also have to manage
freeing the array. So this seems like the better of the two approaches.
What is the consensus here? It would be nice to get this pushed for
1.1.4 since it fixes a libvirtd segfault. I prefer Jeremy's approach of
less memory management.
Regards,
Jim