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

[Alon Levy]

On Tue, Feb 21, 2012 at 10:20:49AM +0100, Gerd Hoffmann wrote:
>   Hi,
> 
> >  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.
> 
> I think we should first reqire spice-server 0.8.latest, so
> update_area_complete is available unconditionally.  Then do any
> displaysurface updates in that callback (or a bh kicked by that
> callback).  Handle both shared & non-shared cases.  I think we also can
> get rid of the flip buffer then and just use a non-shared displaysurface
> in that case (and flip the upside-down qxl surface while copying to the
> qemu displaysurface).

ok, I'll try that...

> 
> >  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).
> 
> IMHO that calls for a patch like this to get rid of the pointless
> console_select() call:
> 
> --- a/console.c
> +++ b/console.c
> @@ -181,12 +181,14 @@ void vga_hw_screen_dump(const char *filename)
> 
>      /* There is currently no way of specifying which screen we want to
> dump,
>         so always dump the first one.  */
> -    console_select(0);
> +    if (previous_active_console && previous_active_console->index != 0) {
> +        console_select(0);
> +    }
>      if (consoles[0] && consoles[0]->hw_screen_dump) {
>          consoles[0]->hw_screen_dump(consoles[0]->hw, filename);
>      }
> 
> -    if (previous_active_console) {
> +    if (previous_active_console && previous_active_console->index != 0) {
>          console_select(previous_active_console->index);
>      }
>  }

...and that. Thanks.

> 
> 
> cheers,
>   Gerd
>