[2/7] Introduce -display argument

From: Jes Sorensen <Jes.Sorensen@redhat.com>

This patch introduces a -display argument which consolidates the
setting of the display mode. Valid options are:
sdl/curses/default/serial (serial is equivalent to -nographic)

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 qemu-options.hx |   27 +++++++++++++++++++
 vl.c            |   77 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 104 insertions(+), 0 deletions(-)

On 15 March 2011 12:36,  <Jes.Sorensen@redhat.com> wrote:
> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>
> This patch introduces a -display argument which consolidates the
> setting of the display mode. Valid options are:
> sdl/curses/default/serial (serial is equivalent to -nographic)

So I still think that we should not be including any new
-display subargument which mirrors the behaviour of -nographic.

-display represents an opportunity to provide a set of orthogonal
command line options which affect the handling of particular
devices; it ought to mean "what happens to VGA/video output?",
and should not change the behaviour of any other devices.

-nographic is effectively a convenience shortcut which changes
the behaviour of several different devices (display, serial,
parallel, at least). It doesn't belong under '-display' from
an orthogonality argument, and people who want it because it
is a shortcut will be better served by the existing '-nographic'
because it's less typing than '-display serial' anyway.

(Ideally we should document '-nographic' by saying that it
is equivalent to some set of other options including
-display none -serial stdio and whatever else it does.)

-- PMM

On 03/15/2011 07:36 AM, Jes.Sorensen@redhat.com wrote:
> From: Jes Sorensen<Jes.Sorensen@redhat.com>
>
> This patch introduces a -display argument which consolidates the
> setting of the display mode. Valid options are:
> sdl/curses/default/serial (serial is equivalent to -nographic)
>
> Signed-off-by: Jes Sorensen<Jes.Sorensen@redhat.com>
> ---
>   qemu-options.hx |   27 +++++++++++++++++++
>   vl.c            |   77 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 104 insertions(+), 0 deletions(-)
>
> diff --git a/qemu-options.hx b/qemu-options.hx
> index badb730..f08ffb1 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -590,6 +590,33 @@ STEXI
>   @table @option
>   ETEXI
>
> +DEF("display", HAS_ARG, QEMU_OPTION_display,
> +    "-display sdl[,frame=on|off][,alt_grab=on|off][,ctrl_grab=on|off]\n"
> +    "            [,window_close=on|off]|curses|serial\n"
> +    "                select display type\n", QEMU_ARCH_ALL)
> +STEXI
> +@item -display @var{type}
> +@findex -display
> +Select type of display to use. This option is a replacement for the
> +old style -sdl/-curses/... options. Valid values for @var{type} are
> +@table @option
> +@item sdl
> +Pick the SDL display option.
> +@item curses
> +Pick the curses display option. Normally, QEMU uses SDL to display the
> +VGA output.  With this option, QEMU can display the VGA output when in
> +text mode using a curses/ncurses interface.  Nothing is displayed in
> +graphical mode.
> +@item serial
> +Normally, QEMU uses SDL to display the VGA output. With this option,
> +you can totally disable graphical output so that QEMU is a simple
> +command line application. The emulated serial port is redirected on
> +the console. Therefore, you can still use QEMU to debug a Linux kernel
> +with a serial console. This option is equivalent to the old -nographic
> +argument.
> +@end table
> +ETEXI
> +
>   DEF("nographic", 0, QEMU_OPTION_nographic,
>       "-nographic      disable graphical output and redirect serial I/Os to console\n",
>       QEMU_ARCH_ALL)
> diff --git a/vl.c b/vl.c
> index 5e007a7..c88ee58 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1554,6 +1554,80 @@ static void select_vgahw (const char *p)
>       }
>   }
>
> +static DisplayType select_display(const char *p)
> +{
> +    const char *opts;
> +    DisplayType display = DT_DEFAULT;
> +
> +    if (strstart(p, "sdl",&opts)) {
> +#ifdef CONFIG_SDL
> +        display = DT_SDL;
> +        while (*opts) {
> +            const char *nextopt;
> +
> +            if (strstart(opts, ",frame=",&nextopt)) {
> +                opts = nextopt;
> +                if (strstart(opts, "on",&nextopt)) {
> +                    no_frame = 0;
> +                } else if (strstart(opts, "off",&nextopt)) {
> +                    no_frame = 1;
> +                } else {
> +                    goto invalid_display;
> +                }
> +            } else if (strstart(opts, ",alt_grab=",&nextopt)) {
> +                opts = nextopt;
> +                if (strstart(opts, "on",&nextopt)) {
> +                    alt_grab = 1;
> +                } else if (strstart(opts, "off",&nextopt)) {
> +                    alt_grab = 0;
> +                } else {
> +                    goto invalid_display;
> +                }
> +            } else if (strstart(opts, ",ctrl_grab=",&nextopt)) {
> +                opts = nextopt;
> +                if (strstart(opts, "on",&nextopt)) {
> +                    ctrl_grab = 1;
> +                } else if (strstart(opts, "off",&nextopt)) {
> +                    ctrl_grab = 0;
> +                } else {
> +                    goto invalid_display;
> +                }
> +            } else if (strstart(opts, ",window_close=",&nextopt)) {
> +                opts = nextopt;
> +                if (strstart(opts, "on",&nextopt)) {
> +                    no_quit = 0;
> +                } else if (strstart(opts, "off",&nextopt)) {
> +                    no_quit = 1;
> +                } else {
> +                    goto invalid_display;
> +                }
> +            } else {
> +                goto invalid_display;
> +            }
> +            opts = nextopt;
> +        }

So the natural reaction here is going to be, "just use QemuOpts".  But 
this is harder than it seems.  The problem is that the VNC options 
inverse the meaning of booleans making conversion of VNC to use QemuOpts 
much harder than it would appear.

Doing it this way lets us pass the vnc option string directly to 
vnc_display_open().  I don't like it much, but I don't mind it as an 
interim step.

Regards,

Anthony Liguori

> +#else
> +        fprintf(stderr, "SDL support is disabled\n");
> +        exit(1);
> +#endif
> +    } else if (strstart(p, "curses",&opts)) {
> +#ifdef CONFIG_CURSES
> +        display = DT_CURSES;
> +#else
> +        fprintf(stderr, "Curses support is disabled\n");
> +        exit(1);
> +#endif
> +    } else if (strstart(p, "serial",&opts)) {
> +        display = DT_NOGRAPHIC;
> +    } else {
> +    invalid_display:
> +        fprintf(stderr, "Unknown display type: %s\n", p);
> +        exit(1);
> +    }
> +
> +    return display;
> +}
> +
>   static int balloon_parse(const char *arg)
>   {
>       QemuOpts *opts;
> @@ -2152,6 +2226,9 @@ int main(int argc, char **argv, char **envp)
>                   }
>                   numa_add(optarg);
>                   break;
> +            case QEMU_OPTION_display:
> +                display_type = select_display(optarg);
> +                break;
>               case QEMU_OPTION_nographic:
>                   display_type = DT_NOGRAPHIC;
>                   break;

On 03/15/2011 09:07 AM, Peter Maydell wrote:
> On 15 March 2011 12:36,<Jes.Sorensen@redhat.com>  wrote:
>> From: Jes Sorensen<Jes.Sorensen@redhat.com>
>>
>> This patch introduces a -display argument which consolidates the
>> setting of the display mode. Valid options are:
>> sdl/curses/default/serial (serial is equivalent to -nographic)
> So I still think that we should not be including any new
> -display subargument which mirrors the behaviour of -nographic.
>
> -display represents an opportunity to provide a set of orthogonal
> command line options which affect the handling of particular
> devices; it ought to mean "what happens to VGA/video output?",
> and should not change the behaviour of any other devices.
>
> -nographic is effectively a convenience shortcut which changes
> the behaviour of several different devices (display, serial,
> parallel, at least). It doesn't belong under '-display' from
> an orthogonality argument, and people who want it because it
> is a shortcut will be better served by the existing '-nographic'
> because it's less typing than '-display serial' anyway.

Yeah, I think I agree.  We shouldn't have a -display nographic.

Regards,

Anthony Liguori

> (Ideally we should document '-nographic' by saying that it
> is equivalent to some set of other options including
> -display none -serial stdio and whatever else it does.)
>
> -- PMM

On 03/15/11 15:51, Anthony Liguori wrote:
>> +            } else if (strstart(opts, ",window_close=",&nextopt)) {
>> +                opts = nextopt;
>> +                if (strstart(opts, "on",&nextopt)) {
>> +                    no_quit = 0;
>> +                } else if (strstart(opts, "off",&nextopt)) {
>> +                    no_quit = 1;
>> +                } else {
>> +                    goto invalid_display;
>> +                }
>> +            } else {
>> +                goto invalid_display;
>> +            }
>> +            opts = nextopt;
>> +        }
> 
> So the natural reaction here is going to be, "just use QemuOpts".  But
> this is harder than it seems.  The problem is that the VNC options
> inverse the meaning of booleans making conversion of VNC to use QemuOpts
> much harder than it would appear.
> 
> Doing it this way lets us pass the vnc option string directly to
> vnc_display_open().  I don't like it much, but I don't mind it as an
> interim step.

Yeah, it isn't particularly pretty, but it really requires we rewamp the
arguments completely which I think is out of scope for the initial
change here.

Cheers,
Jes

On 03/15/11 15:53, Anthony Liguori wrote:
>> -nographic is effectively a convenience shortcut which changes
>> the behaviour of several different devices (display, serial,
>> parallel, at least). It doesn't belong under '-display' from
>> an orthogonality argument, and people who want it because it
>> is a shortcut will be better served by the existing '-nographic'
>> because it's less typing than '-display serial' anyway.
> 
> Yeah, I think I agree.  We shouldn't have a -display nographic.

Ok, consider it gone. I've never used it, so a I am more than happy to
axe it :)

Cheers,
Jes

On 15.03.2011, at 15:07, Peter Maydell wrote:

> On 15 March 2011 12:36,  <Jes.Sorensen@redhat.com> wrote:
>> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>> 
>> This patch introduces a -display argument which consolidates the
>> setting of the display mode. Valid options are:
>> sdl/curses/default/serial (serial is equivalent to -nographic)
> 
> So I still think that we should not be including any new
> -display subargument which mirrors the behaviour of -nographic.
> 
> -display represents an opportunity to provide a set of orthogonal
> command line options which affect the handling of particular
> devices; it ought to mean "what happens to VGA/video output?",
> and should not change the behaviour of any other devices.
> 
> -nographic is effectively a convenience shortcut which changes
> the behaviour of several different devices (display, serial,
> parallel, at least). It doesn't belong under '-display' from
> an orthogonality argument, and people who want it because it
> is a shortcut will be better served by the existing '-nographic'
> because it's less typing than '-display serial' anyway.
> 
> (Ideally we should document '-nographic' by saying that it
> is equivalent to some set of other options including
> -display none -serial stdio and whatever else it does.)

Ideally it should also be implemented as such :)


Alex