Re: [Qemu-devel] [PATCH 1/2][RESEND] vl: initialize all displaystates

[Dmitry Eremin-Solenikov]

Hello,


On 3/10/11, andrzej zaborowski <address@hidden> wrote:
> On 4 March 2011 01:48, Dmitry Eremin-Solenikov <address@hidden> wrote:
>> Init not only first displaystate, but all. Otherwise machines with
>> multiple display devices (e.g. tosa, as it exists now) will just
>> segfault on ds switch.
>>
>> Signed-off-by: Dmitry Eremin-Solenikov <address@hidden>
>> ---
>>  vl.c |  104
>> +++++++++++++++++++++++++++++++++---------------------------------
>>  1 files changed, 52 insertions(+), 52 deletions(-)
>>
>>  Basically this patch is equal to:
>>  @@ -3009,9 +3009,7 @@ int main(int argc, char **argv, char **envp)
>>
>>      net_check_clients();
>>
>>  -    /* just use the first displaystate for the moment */
>>  -    ds = get_displaystate();
>>  -
>>  +    for (ds = get_displaystate(); ds; ds = ds->next) {
>>      if (using_spice)
>>          display_remote++;
>>      if (display_type == DT_DEFAULT && !display_remote) {
>>  @@ -3077,7 +3075,9 @@ int main(int argc, char **argv, char **envp)
>>          nographic_timer = qemu_new_timer(rt_clock, nographic_update,
>> NULL);
>>          qemu_mod_timer(nographic_timer, qemu_get_clock(rt_clock));
>>      }
>>  -    text_consoles_set_display(ds);
>>  +    }
>>  +
>>  +    text_consoles_set_display(get_displaystate());
>>
>>      if (gdbstub_dev && gdbserver_start(gdbstub_dev) < 0) {
>>          fprintf(stderr, "qemu: could not open gdbserver on device
>> '%s'\n",
>>
>> diff --git a/vl.c b/vl.c
>> index 14255c4..b8cd455 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -3009,75 +3009,75 @@ int main(int argc, char **argv, char **envp)
>>
>>     net_check_clients();
>>
>> -    /* just use the first displaystate for the moment */
>> -    ds = get_displaystate();
>> -
>> -    if (using_spice)
>> -        display_remote++;
>> -    if (display_type == DT_DEFAULT && !display_remote) {
>> +    for (ds = get_displaystate(); ds; ds = ds->next) {
>> +        if (using_spice)
>> +            display_remote++;
>> +        if (display_type == DT_DEFAULT && !display_remote) {
>>  #if defined(CONFIG_SDL) || defined(CONFIG_COCOA)
>> -        display_type = DT_SDL;
>> +            display_type = DT_SDL;
>>  #else
>> -        vnc_display = "localhost:0,to=99";
>> -        show_vnc_port = 1;
>> +            vnc_display = "localhost:0,to=99";
>> +            show_vnc_port = 1;
>>  #endif
>> -    }
>> -
>> +        }
>> +
>>
>> -    /* init local displays */
>> -    switch (display_type) {
>> -    case DT_NOGRAPHIC:
>> -        break;
>> +        /* init local displays */
>> +        switch (display_type) {
>> +        case DT_NOGRAPHIC:
>> +            break;
>>  #if defined(CONFIG_CURSES)
>> -    case DT_CURSES:
>> -        curses_display_init(ds, full_screen);
>> -        break;
>> +        case DT_CURSES:
>> +            curses_display_init(ds, full_screen);
>> +            break;
>>  #endif
>>  #if defined(CONFIG_SDL)
>> -    case DT_SDL:
>> -        sdl_display_init(ds, full_screen, no_frame);
>> -        break;
>> +        case DT_SDL:
>> +            sdl_display_init(ds, full_screen, no_frame);
>> +            break;
>>  #elif defined(CONFIG_COCOA)
>> -    case DT_SDL:
>> -        cocoa_display_init(ds, full_screen);
>> -        break;
>> +        case DT_SDL:
>> +            cocoa_display_init(ds, full_screen);
>> +            break;
>
> I'm not sure this will work as intended, I think we shouldn't call
> curses/sdl/cocoa_display_init() for every display state, we should
> just call register_displaychangelistener() etc. for each display
> state.  My assumption is that we want each ds to appear as a graphical
> console in the same window, not open N SDL windows / VNC servers (for
> curses that would break completely I think).

I've not tested VNC/curses (will do this later), but for SDL this
works as expected:
on tosa, where qemu registers both PXA and TC6393xb displays, I've
exactly one window.
Moreover w/o this patch qemu crashes if I try to switch to secondary
graphic console (tc6393).

-- 
With best wishes
Dmitry