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

[Stefano Stabellini]

On Thu, 23 Feb 2012, Anthony Liguori wrote:
> > 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;
> >   }

You should be wary of all the callers of this function because they
probably assume that there is just one DisplayState and may not cope
well with a multiple DisplayState scenario.
It might be better to rename this function "get_active_displaystate" or
"get_current_displaystate". Even better would be to replace it entirely
with a for_each_display_state type of iterator, see for example
pci_for_each_device or irq_domain_for_each_irq in Linux.


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

Like Anthony wrote, you probably need a more substantial refactoring to
make this work, but the basic idea that you can call
graphic_console_init twice to instantiate two DisplayState instances is
correct.
Then you should be able to call sdl_display_init on all of them independently.
If sdl_display_init (or any other display_init function) modifies a
single global state, that needs to be fixed.