[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/2] Explicitly print out default vnc option in
From: |
Robert Hu |
Subject: |
Re: [Qemu-devel] [PATCH 2/2] Explicitly print out default vnc option in use |
Date: |
Tue, 07 Jun 2016 08:28:34 +0800 |
On Mon, 2016-06-06 at 09:28 +0200, Markus Armbruster wrote:
> Robert Hu <address@hidden> writes:
>
> > On Tue, 2016-05-31 at 13:17 +0200, Markus Armbruster wrote:
> >> Robert Hu <address@hidden> writes:
> >>
> >> > On Tue, 2016-05-31 at 09:51 +0200, Markus Armbruster wrote:
[trim...]
> > I don't see a './configure' option related to this '-vnc to' param. Is
> > there any?
> > '--help', you mean 'qemu-system_x86-64 --help'? or './configure --help'?
[seems repeated contents, trim...]
> > I don't see a './configure' option related to this '-vnc to' param. Is
> > there any?
> > '--help', you mean 'qemu-system_x86-64 --help'? or './configure --help'?
>
> The former.
>
> The modern way to select a display is -display. The older -nographic,
> -curses, -sdl are retained for backward compatibility.
>
> Relevant parts of -help:
>
> Display options:
> -display sdl[,frame=on|off][,alt_grab=on|off][,ctrl_grab=on|off]
> [,window_close=on|off]|curses|none|
> gtk[,grab_on_hover=on|off]|
> vnc=<display>[,<optargs>]
> select display type
> -nographic disable graphical output and redirect serial I/Os to
> console
> -curses use a curses/ncurses interface instead of SDL
> [...]
> -sdl enable SDL
> [...]
> -vnc display start a VNC server on display
>
> Issues:
>
> * Help for -display is broken: the mutually exclusive option arguments
> are concatenated. -display curses and -display none are undocumented.
> It should look more like this:
>
> -display sdl[,frame=on|off][,alt_grab=on|off][,ctrl_grab=on|off]
> [,window_close=on|off]|curses|none|
> -display gtk[,grab_on_hover=on|off]|
> -display vnc=<display>[,<optargs>]
> -display curses
> -display none
> select display type
>
> * -display sdl,gl=on|off and -display gtk,gl=on|off are undocumented
> (missed in commit 0b71a5d5c and 97edf3b).
>
> * There is no help on the <display> in -display vnc=<display>.
>
> * There is no help on the default. main() picks the default depending
> on configure options:
>
> #if defined(CONFIG_GTK)
> display_type = DT_GTK;
> #elif defined(CONFIG_SDL)
> display_type = DT_SDL;
> #elif defined(CONFIG_COCOA)
> display_type = DT_COCOA;
> #elif defined(CONFIG_VNC)
> vnc_parse("localhost:0,to=99,id=default", &error_abort);
> show_vnc_port = 1;
> #else
> display_type = DT_NONE;
> #endif
>
> Help should show the default this binary will pick. This is what I
> meant by "Ideally, --help output would show the defaults for this
> build's configuration."
>
> * Help should explain syntacic sugar:
> -curses is sugar for -display curses
> -sdl is sugar for -display sdl
> -vnc display is sugar for -display vnc=display
>
> -nographic is also sugar, but too complicated to explain; I'd leave it
> as is.
>
> Non-issue
>
> * Help shows options even when they're not compiled in. That's okay,
> because trying to use them fails with an "FOO support is disabled"
> error message.
>
> >> If we decide users need more information than the current "VNC server
> >> running on" line, perhaps it should be included right in that line.
>
> This would complement, but not replace better -help ouput.
>
> If you would like to work on these issues, let us know.
OK, if not in a hurry and assuming this is not a huge amount of work.
I also need to look into the build arch so that completely understand
your 'the default this binary will pick', till now I don't.
Another concern is that I'm not a native English speaker, so those
description words may not be that apt and concise.
Meanwhile, this is another work extended from the original patch. How
about accept the patch 1 first? as you and Paolo both think it is OK.
Ought I rework a version 2 of single patch 1? or not necessary?