Re: [libvirt] RHBZ 1013045: Crash on xen domain startup

On Thu, Oct 31, 2013 at 06:50:57PM -0600, Jim Fehlig wrote: > 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. > Absolutely want this in 1.1.4. Personally I think Jeremy's patch is fine.