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

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.