Re: [Qemu-devel] [PATCH 1 of 2] [UPDATE] DisplayState interface change

[Anthony Liguori <anthony@codemonkey.ws>]

Hi Stefano,

Stefano Stabellini wrote:
> Hi all,
> this patch changes the DisplayState interface adding support for
> multiple frontends at the same time (sdl and vnc) and implements most
> of the benefit of the shared_buf patch without the added complexity.
>
> Currently DisplayState is managed by sdl (or vnc) and sdl (or vnc) is
> also responsible for allocating the data and setting the depth.
> Vga.c (or another backend) will do any necessary conversion.
>
> The idea is to change it so that is vga.c (or another backend) that
> fully manages the DisplayState interface allocating data and setting the
> depth (either 16 or 32 bit, if the guest uses a different resolution or
> is in text mode, vga.c (or another backend) is in charge of
> doing the conversion seamlessly).
>
> The other idea is that DisplayState supports *multiple* frontends
> like sdl and vnc; each of them can register some callbacks to be called
> when a display event occurs.
>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>   

There are really two parts to this patch.  One part is a completely 
mechanical conversion of things like ds->linesize to 
ds->surface->linesize.  The second is the guys of the API change.

If you introduce accessors to access things like linesize from 
DisplayState, so something like:

int ds_get_linesize(DisplayState *ds) {
     return ds->linesize;
}

You can do the mechanical conversion in a single patch.  That patch will 
then be a breeze to review.  Then in your next patch, you can just 
convert these accessors to use the new ds->surface->linesize.

That should make this all considerably easier to thoroughly review.  
It's just too big right now to review properly.

Regards,

Anthony Liguori