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. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list