Re: [Qemu-devel] [RFC] sdl: initialize all graphic consoles

[Anthony Liguori]

On 02/23/2012 03:39 PM, Hervé Poussineau wrote:
> MIPS Jazz emulation registers two graphical consoles, but second one stays 
> black.
> This patch repairs it.
>
> Other display methods (cocoa, vnc...) also probably require the same kind of 
> fix.

I don't think this is really the right way to solve this problem.

A bunch of assumptions are being made here and since this has been "broken" for 
some years now, I'm not sure that I really view this as a bug.

> ---
>   console.c |    3 +++
>   vl.c      |   30 +++++++++++++++++++++---------
>   2 files changed, 24 insertions(+), 9 deletions(-)
>
> diff --git a/console.c b/console.c
> index 135394f..2c413a7 100644
> --- a/console.c
> +++ b/console.c
> @@ -1376,6 +1376,9 @@ DisplayState *get_displaystate(void)
>       if (!display_state) {
>           dumb_display_init ();
>       }
> +    if (active_console&&  active_console->ds) {
> +        return active_console->ds;
> +    }
>       return display_state;
>   }
>
> diff --git a/vl.c b/vl.c
> index 7a8cc08..98e0091 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3451,8 +3451,14 @@ int main(int argc, char **argv, char **envp)
>   #endif
>   #if defined(CONFIG_SDL)
>       case DT_SDL:
> -        sdl_display_init(ds, full_screen, no_frame);
> +    {
> +        DisplayState *ds2 = ds;
> +        while (ds2) {
> +            sdl_display_init(ds2, full_screen, no_frame);
> +            ds2 = ds2->next;

The fact that this works at all really surprises me.  You're registering double 
input event handlers and doing a number of things that have a global state.

sdl_display_init() isn't meant to be called twice.  I really think more 
substantial refactoring is needed such that we're not maintaining the UI state 
as globals and can independently instantiate a backend for a given DisplayState.

I had some patches that I posted a bit ago that started down this direction.

Regards,

Anthony Liguori

> +        }
>           break;
> +    }
>   #elif defined(CONFIG_COCOA)
>       case DT_SDL:
>           cocoa_display_init(ds, full_screen);
> @@ -3484,15 +3490,21 @@ int main(int argc, char **argv, char **envp)
>   #endif
>
>       /* display setup */
> -    dpy_resize(ds);
> -    dcl = ds->listeners;
> -    while (dcl != NULL) {
> -        if (dcl->dpy_refresh != NULL) {
> -            ds->gui_timer = qemu_new_timer_ms(rt_clock, gui_update, ds);
> -            qemu_mod_timer(ds->gui_timer, qemu_get_clock_ms(rt_clock));
> -            break;
> +    {
> +        DisplayState *ds2 = ds;
> +        while (ds2 != NULL) {
> +            dpy_resize(ds2);
> +            dcl = ds2->listeners;
> +            while (dcl != NULL) {
> +                if (dcl->dpy_refresh != NULL) {
> +                    ds2->gui_timer = qemu_new_timer_ms(rt_clock, gui_update, 
> ds2);
> +                    qemu_mod_timer(ds2->gui_timer, 
> qemu_get_clock_ms(rt_clock));
> +                    break;
> +                }
> +                dcl = dcl->next;
> +            }
> +            ds2 = ds2->next;
>           }
> -        dcl = dcl->next;
>       }
>       text_consoles_set_display(ds);