On Tue, Feb 21, 2012 at 08:57:54AM +0100, Gerd Hoffmann wrote: > Hi, > > > TBH I'm having problems without doing it with my own Allocator. In > > particular calling ppm_save from spice_server thread seems to be a > > problem. > > Grabbed the qemu mutex?
Didn't we remove qemu mutex grabbing from the spice server thread a long long time ago? I've switched to using a bh, it seems to work better but still not perfectly. I've also found my notes on why I tried the allocator approach in the first place (all the following is in QXL_NATIVE mode): Boot with qxl only, no vnc or sdl (so no qxl_render_update users except screendump). Issue screendump. Right now qxl_render_update checks if the displaysurface buffer is not shared, meaning it was allocated by qemu, and in this case it replaces it with the flipped buffer. But right after that surface->data gets reset, by vga_hw_screen_dump: vga_hw_screen_dump/console_select/qemu_resize_displaysurface/ds->allocator->resize_displaysurface/defaultallocator_resize_displaysurface/qemu_alloc_display Hence my line of thought that replacing the allocator with my own would prevent this. Since you have misgivings about using our own allocator that I don't know how to resolve, I'm instead doing a second reallocation in our dpy_resize callback qxl.c:display_resize, in affect it means that we have three allocations and three deallocations for every screendump. Do you still think it's less ugly then an allocator? note that I have sdl and vnc working with spice with my allocator scheme. (just didn't test all three together yet). > > >> BTW: qxl insisting on a shared displaysurface isn't very clean too, it > >> better should be able to fallback to just copying/converting for the > >> non-shared case. > > > > I'll try to get that working, although it seems to require having some > > timer/bh to do the ppm_save. > > bh should do, that is one of the things they are supposed to handle. > > cheers, > Gerd > >