Re: [Qemu-devel] [RFC 7/7] qxl: add allocator

[Alon Levy <alevy@redhat.com>]

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
> 
>