Re: [Qemu-devel] [PATCH 0 of 7] [UPDATE] DisplayState interface change

[Paul Brook]

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

I'm still not convinced you've got the abstraction right here.

By my reading your shared buffer code is fairly fragile. If the users switches 
VCs (which will cause qemu to reallocate the surface independently of the 
device) then qemu will reallocate the buffer. Having both qemu and individual 
device code allocating surfaces seems wrong. It should be one or the other.

It also feels messy the way that allocating the surface and notifying the 
DisplayState of the change are two separate operations.
IMHO DisplayState should be an opaque structure as far as devices are 
concerned. They should never have to access its members (or deal with 
DisplaySurface) directly.

> - do not use is_active_console, use is_graphic_console instead.

This is wrong. There may be multiple graphic consoles.

> This patch changes the graphical_console_init function to return an
> allocated DisplayState instead of a QEMUConsole.

You should also remove QEMUConsole.

> +    s->ds = graphic_console_init(s->update, s->invalidate,
> +                                 s->screen_dump, s->text_update, s);
> +    register_displaystate(s->ds);

Why have you got separate allocate and register functions?

Paul