[Qemu-devel] [PATCH 0/2] Reveal 'to' parameter of 'vnc' option to user

[Qemu-devel] [PATCH 0/2] Reveal 'to' parameter of 'vnc' option to user

[Robert Ho]

(If you received this patch set several times, I appologize, I met some
problem in the smtp setting.)
I find that '-vnc' option actually has a parameter 'to', implicitly;
while actually is there and can be used but not be public.
Don't know why but this may probably confuse user, especially when used in
some default situation implicitly. 

So shall I?
1.	Add its description in QEMU manual info
2.	Expilicitly print out when it's used with non-default value

Robert Ho (2):
  Reveal 'to' param of '-vnc' option in qemu man page
  Explicitly print out default vnc option in use

 qemu-options.hx | 7 +++++++
 vl.c            | 5 ++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH 1/2] Reveal 'to' param of '-vnc' option in qemu man page

[Robert Ho]

Add the descrption of the 'to' param of 'vnc' option in QEMU man page.

Signed-off-by: Robert Ho <robert.hu@intel.com>
---
 qemu-options.hx | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/qemu-options.hx b/qemu-options.hx
index 6106520..68b4cf2 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1241,6 +1241,13 @@ syntax for the @var{display} is
 
 @table @option
 
+@item to=@var{L}
+
+With this option, QEMU will try next available VNC @var{display}s, until the
+number @var{L}, if the origianlly defined "-vnc @var{display}" is not
+available, e.g. port 5900+@var{display} is already used by another
+application. By default, to=0.
+
 @item @var{host}:@var{d}
 
 TCP connections will only be allowed from @var{host} on display @var{d}.
-- 
1.8.3.1

[Qemu-devel] [PATCH 2/2] Explicitly print out default vnc option in use

[Robert Ho]

If no display option defined in QEMU command line, and SDL is not
available, then it by default uses '-vnc localhost:0,to=99,id=default'.
This patch simply print out the default option parameters out, so that
user is aware of that.

Signed-off-by: Robert Ho <robert.hu@intel.com>
---
 vl.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/vl.c b/vl.c
index 18d1423..8617a68 100644
--- a/vl.c
+++ b/vl.c
@@ -4213,7 +4213,10 @@ int main(int argc, char **argv, char **envp)
 #elif defined(CONFIG_COCOA)
         display_type = DT_COCOA;
 #elif defined(CONFIG_VNC)
-        vnc_parse("localhost:0,to=99,id=default", &error_abort);
+		#define DEFAULT_VNC_DISPLAY_OPTION	"localhost:0,to=99,id=default"
+        vnc_parse(DEFAULT_VNC_DISPLAY_OPTION, &error_abort);
+		printf("No display option defined, using '-vnc %s' by default	\
+\n", DEFAULT_VNC_DISPLAY_OPTION);
         show_vnc_port = 1;
 #else
         display_type = DT_NONE;
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 0/2] Reveal 'to' parameter of 'vnc' option to user

[Paolo Bonzini]

On 31/05/2016 09:03, Robert Ho wrote:
> I find that '-vnc' option actually has a parameter 'to', implicitly;
> while actually is there and can be used but not be public.
> Don't know why but this may probably confuse user, especially when used in
> some default situation implicitly. 
> 
> So shall I?
> 1.	Add its description in QEMU manual info
> 2.	Expilicitly print out when it's used with non-default value

I think patch 1 is okay.

For patch 2, I am not sure I see the point of printing the VNC options;
the VNC port is printed immediately after.

However, I think it is useful to print the VNC port whenever the user
specifies "-vnc to=XX".  Perhaps you could move show_vnc_port (the
variable, the assignments and the "if (show_vnc_port)") from vl.c to
vnc_display_open?

Thanks,

Paolo

Re: [Qemu-devel] [PATCH 0/2] Reveal 'to' parameter of 'vnc' option to user

[Robert Hu]

On Tue, 2016-05-31 at 14:59 +0200, Paolo Bonzini wrote:
> 
> On 31/05/2016 09:03, Robert Ho wrote:
> > I find that '-vnc' option actually has a parameter 'to', implicitly;
> > while actually is there and can be used but not be public.
> > Don't know why but this may probably confuse user, especially when used in
> > some default situation implicitly. 
> > 
> > So shall I?
> > 1.	Add its description in QEMU manual info
> > 2.	Expilicitly print out when it's used with non-default value
> 
> I think patch 1 is okay.

Thanks Paolo. Sorry for late reply, since I just part-time work on this.

> 
> For patch 2, I am not sure I see the point of printing the VNC options;
> the VNC port is printed immediately after.

I agree it is not significantly meaningful to print out the options. I
just disliked the implicitly be-on-behalf-chose a value without
notification; especially we don't know there is a 'to' option. So when I
did the density test of huge quantity of guests, always failed on 99th,
made me quizzical. :(
In most ordinary cases, the final 'VNC server running on ...' printf is
sufficient.
Now my that emotion gradually die out, I agree abandon patch 2. :)

> 
> However, I think it is useful to print the VNC port whenever the user
> specifies "-vnc to=XX".  Perhaps you could move show_vnc_port (the
> variable, the assignments and the "if (show_vnc_port)") from vl.c to
> vnc_display_open?

And 2nd reason I abandon patch 2: I grepped in vnc.c, no bare 'printf'.
Any nice wrapper for such info purpose? I don't want to be first 'bad
guy' to break the elegant code. ;)

So do I need to send out v2 patch? which simply contains single patch 1.

> 
> Thanks,
> 
> Paolo

[Qemu-devel] [PATCH 0/2] Reveal 'to' parameter of 'vnc' option to user

[Robert Ho]

I find that '-vnc' option actually has a parameter 'to', implicitly;
while actually is there and can be used but not be public.
Don't know why but this may probably confuse user, especially when used in
some default situation implicitly. 

So shall I?
1.	Add its description in QEMU manual info
2.	Expilicitly print out when it's used with non-default value

Robert Ho (2):
  Reveal 'to' param of '-vnc' option in qemu man page
  Explicitly print out default vnc option in use

 qemu-options.hx | 7 +++++++
 vl.c            | 5 ++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH 2/2] Explicitly print out default vnc option in use

[Robert Ho]

If no display option defined in QEMU command line, and SDL is not
available, then it by default uses '-vnc localhost:0,to=99,id=default'.
This patch simply print out the default option parameters out, so that
user is aware of that.

Signed-off-by: Robert Ho <robert.hu@intel.com>
---
 vl.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/vl.c b/vl.c
index 18d1423..8617a68 100644
--- a/vl.c
+++ b/vl.c
@@ -4213,7 +4213,10 @@ int main(int argc, char **argv, char **envp)
 #elif defined(CONFIG_COCOA)
         display_type = DT_COCOA;
 #elif defined(CONFIG_VNC)
-        vnc_parse("localhost:0,to=99,id=default", &error_abort);
+		#define DEFAULT_VNC_DISPLAY_OPTION	"localhost:0,to=99,id=default"
+        vnc_parse(DEFAULT_VNC_DISPLAY_OPTION, &error_abort);
+		printf("No display option defined, using '-vnc %s' by default	\
+\n", DEFAULT_VNC_DISPLAY_OPTION);
         show_vnc_port = 1;
 #else
         display_type = DT_NONE;
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 2/2] Explicitly print out default vnc option in use

[Markus Armbruster]

Robert Ho <robert.hu@intel.com> writes:

> If no display option defined in QEMU command line, and SDL is not
> available, then it by default uses '-vnc localhost:0,to=99,id=default'.
> This patch simply print out the default option parameters out, so that
> user is aware of that.
>
> Signed-off-by: Robert Ho <robert.hu@intel.com>
> ---
>  vl.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/vl.c b/vl.c
> index 18d1423..8617a68 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4213,7 +4213,10 @@ int main(int argc, char **argv, char **envp)
>  #elif defined(CONFIG_COCOA)
>          display_type = DT_COCOA;
>  #elif defined(CONFIG_VNC)
> -        vnc_parse("localhost:0,to=99,id=default", &error_abort);
> +		#define DEFAULT_VNC_DISPLAY_OPTION	"localhost:0,to=99,id=default"

Preprocessor directives shouldn't be indented.

Also tab damage.  Please use scripts/checkpatch.pl to check your patches.

> +        vnc_parse(DEFAULT_VNC_DISPLAY_OPTION, &error_abort);
> +		printf("No display option defined, using '-vnc %s' by default	\
> +\n", DEFAULT_VNC_DISPLAY_OPTION);
>          show_vnc_port = 1;
>  #else
>          display_type = DT_NONE;

I don't like this.  Programs should be quiet unless they got something
important to say.  Can't see why this particular default is more
important than all the other defaults we don't print.

The default could be documented in output of --help.

Re: [Qemu-devel] [PATCH 2/2] Explicitly print out default vnc option in use

[Robert Hu]

On Tue, 2016-05-31 at 09:51 +0200, Markus Armbruster wrote:
> Robert Ho <robert.hu@intel.com> writes:
> 
> > If no display option defined in QEMU command line, and SDL is not
> > available, then it by default uses '-vnc localhost:0,to=99,id=default'.
> > This patch simply print out the default option parameters out, so that
> > user is aware of that.
> >
> > Signed-off-by: Robert Ho <robert.hu@intel.com>
> > ---
> >  vl.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/vl.c b/vl.c
> > index 18d1423..8617a68 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -4213,7 +4213,10 @@ int main(int argc, char **argv, char **envp)
> >  #elif defined(CONFIG_COCOA)
> >          display_type = DT_COCOA;
> >  #elif defined(CONFIG_VNC)
> > -        vnc_parse("localhost:0,to=99,id=default", &error_abort);
> > +		#define DEFAULT_VNC_DISPLAY_OPTION	"localhost:0,to=99,id=default"
> 
> Preprocessor directives shouldn't be indented.
> 
> Also tab damage.  Please use scripts/checkpatch.pl to check your patches.

Thanks Markus for your review.
Firstly apologize if you received multiple copies of this patch. I'm
still struggling with my egress SMTP setting. I've no idea how many
copies you received:( but glad now see your reply.

Yes, sorry about haven't checked the patch with the auxiliary scripts. I
didn't know that. Thanks for pointing out.
I'm new here, will learn these upstream convention ASAP.
> 
> > +        vnc_parse(DEFAULT_VNC_DISPLAY_OPTION, &error_abort);
> > +		printf("No display option defined, using '-vnc %s' by default	\
> > +\n", DEFAULT_VNC_DISPLAY_OPTION);
> >          show_vnc_port = 1;
> >  #else
> >          display_type = DT_NONE;
> 
> I don't like this.  Programs should be quiet unless they got something
> important to say.  Can't see why this particular default is more
> important than all the other defaults we don't print.
> 
> The default could be documented in output of --help.

Actually my thought was this is not using the default value and
implicitly. The default of 'to' is 0, while in this case (when no
display option defined and SDL not configured in), it implicitly uses
non-default value '99'. Therefore I thought it shall be explicitly print
out so that user would be aware of what was chosen on behalf of him;
like the final print of 'VNC server running on '::1;5900''. 

Re: [Qemu-devel] [PATCH 2/2] Explicitly print out default vnc option in use

[Markus Armbruster]

Robert Hu <robert.hu@vmm.sh.intel.com> writes:

> On Tue, 2016-05-31 at 09:51 +0200, Markus Armbruster wrote:
>> Robert Ho <robert.hu@intel.com> writes:
>> 
>> > If no display option defined in QEMU command line, and SDL is not
>> > available, then it by default uses '-vnc localhost:0,to=99,id=default'.
>> > This patch simply print out the default option parameters out, so that
>> > user is aware of that.
>> >
>> > Signed-off-by: Robert Ho <robert.hu@intel.com>
>> > ---
>> >  vl.c | 5 ++++-
>> >  1 file changed, 4 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/vl.c b/vl.c
>> > index 18d1423..8617a68 100644
>> > --- a/vl.c
>> > +++ b/vl.c
>> > @@ -4213,7 +4213,10 @@ int main(int argc, char **argv, char **envp)
>> >  #elif defined(CONFIG_COCOA)
>> >          display_type = DT_COCOA;
>> >  #elif defined(CONFIG_VNC)
>> > -        vnc_parse("localhost:0,to=99,id=default", &error_abort);
>> > +		#define DEFAULT_VNC_DISPLAY_OPTION	"localhost:0,to=99,id=default"
>> 
>> Preprocessor directives shouldn't be indented.
>> 
>> Also tab damage.  Please use scripts/checkpatch.pl to check your patches.
>
> Thanks Markus for your review.
> Firstly apologize if you received multiple copies of this patch. I'm
> still struggling with my egress SMTP setting. I've no idea how many
> copies you received:( but glad now see your reply.
>
> Yes, sorry about haven't checked the patch with the auxiliary scripts. I
> didn't know that. Thanks for pointing out.
> I'm new here, will learn these upstream convention ASAP.

No problem.  All we expect from new contributors is making an effort to
get their patches right.  Actually getting them 100% right from the
start isn't really in the cards :)

>> > +        vnc_parse(DEFAULT_VNC_DISPLAY_OPTION, &error_abort);
>> > +		printf("No display option defined, using '-vnc %s' by default	\
>> > +\n", DEFAULT_VNC_DISPLAY_OPTION);
>> >          show_vnc_port = 1;
>> >  #else
>> >          display_type = DT_NONE;
>> 
>> I don't like this.  Programs should be quiet unless they got something
>> important to say.  Can't see why this particular default is more
>> important than all the other defaults we don't print.
>> 
>> The default could be documented in output of --help.
>
> Actually my thought was this is not using the default value and
> implicitly. The default of 'to' is 0, while in this case (when no
> display option defined and SDL not configured in), it implicitly uses
> non-default value '99'. Therefore I thought it shall be explicitly print
> out so that user would be aware of what was chosen on behalf of him;
> like the final print of 'VNC server running on '::1;5900''. 

The default depends on configuration options.  Ideally, --help output
would show the defaults for this build's configuration.

If we decide users need more information than the current "VNC server
running on" line, perhaps it should be included right in that line.

Re: [Qemu-devel] [PATCH 2/2] Explicitly print out default vnc option in use

[Robert Hu]

On Tue, 2016-05-31 at 13:17 +0200, Markus Armbruster wrote:
> Robert Hu <robert.hu@vmm.sh.intel.com> writes:
> 
> > On Tue, 2016-05-31 at 09:51 +0200, Markus Armbruster wrote:
> >> Robert Ho <robert.hu@intel.com> writes:
> >> 
> >> > If no display option defined in QEMU command line, and SDL is not
> >> > available, then it by default uses '-vnc localhost:0,to=99,id=default'.
> >> > This patch simply print out the default option parameters out, so that
> >> > user is aware of that.
> >> >
> >> > Signed-off-by: Robert Ho <robert.hu@intel.com>
> >> > ---
> >> >  vl.c | 5 ++++-
> >> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/vl.c b/vl.c
> >> > index 18d1423..8617a68 100644
> >> > --- a/vl.c
> >> > +++ b/vl.c
> >> > @@ -4213,7 +4213,10 @@ int main(int argc, char **argv, char **envp)
> >> >  #elif defined(CONFIG_COCOA)
> >> >          display_type = DT_COCOA;
> >> >  #elif defined(CONFIG_VNC)
> >> > -        vnc_parse("localhost:0,to=99,id=default", &error_abort);
> >> > +		#define DEFAULT_VNC_DISPLAY_OPTION	"localhost:0,to=99,id=default"
> >> 
> >> Preprocessor directives shouldn't be indented.
> >> 
> >> Also tab damage.  Please use scripts/checkpatch.pl to check your patches.
> >
> > Thanks Markus for your review.
> > Firstly apologize if you received multiple copies of this patch. I'm
> > still struggling with my egress SMTP setting. I've no idea how many
> > copies you received:( but glad now see your reply.
> >
> > Yes, sorry about haven't checked the patch with the auxiliary scripts. I
> > didn't know that. Thanks for pointing out.
> > I'm new here, will learn these upstream convention ASAP.
> 
> No problem.  All we expect from new contributors is making an effort to
> get their patches right.  Actually getting them 100% right from the
> start isn't really in the cards :)

Thank You!
Sorry for late following up; for I just part-time do this. I'm too busy
on work these days.

> 
> >> > +        vnc_parse(DEFAULT_VNC_DISPLAY_OPTION, &error_abort);
> >> > +		printf("No display option defined, using '-vnc %s' by default	\
> >> > +\n", DEFAULT_VNC_DISPLAY_OPTION);
> >> >          show_vnc_port = 1;
> >> >  #else
> >> >          display_type = DT_NONE;
> >> 
> >> I don't like this.  Programs should be quiet unless they got something
> >> important to say.  Can't see why this particular default is more
> >> important than all the other defaults we don't print.
> >> 
> >> The default could be documented in output of --help.
> >
> > Actually my thought was this is not using the default value and
> > implicitly. The default of 'to' is 0, while in this case (when no
> > display option defined and SDL not configured in), it implicitly uses
> > non-default value '99'. Therefore I thought it shall be explicitly print
> > out so that user would be aware of what was chosen on behalf of him;
> > like the final print of 'VNC server running on '::1;5900''. 
> 
> The default depends on configuration options.  Ideally, --help output
> would show the defaults for this build's configuration.

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'?
> 
> If we decide users need more information than the current "VNC server
> running on" line, perhaps it should be included right in that line.

Re: [Qemu-devel] [PATCH 2/2] Explicitly print out default vnc option in use

[Markus Armbruster]

Robert Hu <robert.hu@vmm.sh.intel.com> writes:

> On Tue, 2016-05-31 at 13:17 +0200, Markus Armbruster wrote:
>> Robert Hu <robert.hu@vmm.sh.intel.com> writes:
>> 
>> > On Tue, 2016-05-31 at 09:51 +0200, Markus Armbruster wrote:
>> >> Robert Ho <robert.hu@intel.com> writes:
>> >> 
>> >> > If no display option defined in QEMU command line, and SDL is not
>> >> > available, then it by default uses '-vnc localhost:0,to=99,id=default'.
>> >> > This patch simply print out the default option parameters out, so that
>> >> > user is aware of that.
>> >> >
>> >> > Signed-off-by: Robert Ho <robert.hu@intel.com>
>> >> > ---
>> >> >  vl.c | 5 ++++-
>> >> >  1 file changed, 4 insertions(+), 1 deletion(-)
>> >> >
>> >> > diff --git a/vl.c b/vl.c
>> >> > index 18d1423..8617a68 100644
>> >> > --- a/vl.c
>> >> > +++ b/vl.c
>> >> > @@ -4213,7 +4213,10 @@ int main(int argc, char **argv, char **envp)
>> >> >  #elif defined(CONFIG_COCOA)
>> >> >          display_type = DT_COCOA;
>> >> >  #elif defined(CONFIG_VNC)
>> >> > -        vnc_parse("localhost:0,to=99,id=default", &error_abort);
>> >> > +		#define DEFAULT_VNC_DISPLAY_OPTION	"localhost:0,to=99,id=default"
>> >> 
>> >> Preprocessor directives shouldn't be indented.
>> >> 
>> >> Also tab damage.  Please use scripts/checkpatch.pl to check your patches.
>> >
>> > Thanks Markus for your review.
>> > Firstly apologize if you received multiple copies of this patch. I'm
>> > still struggling with my egress SMTP setting. I've no idea how many
>> > copies you received:( but glad now see your reply.
>> >
>> > Yes, sorry about haven't checked the patch with the auxiliary scripts. I
>> > didn't know that. Thanks for pointing out.
>> > I'm new here, will learn these upstream convention ASAP.
>> 
>> No problem.  All we expect from new contributors is making an effort to
>> get their patches right.  Actually getting them 100% right from the
>> start isn't really in the cards :)
>
> Thank You!
> Sorry for late following up; for I just part-time do this. I'm too busy
> on work these days.
>
>> 
>> >> > +        vnc_parse(DEFAULT_VNC_DISPLAY_OPTION, &error_abort);
>> >> > +		printf("No display option defined, using '-vnc %s' by default	\
>> >> > +\n", DEFAULT_VNC_DISPLAY_OPTION);
>> >> >          show_vnc_port = 1;
>> >> >  #else
>> >> >          display_type = DT_NONE;
>> >> 
>> >> I don't like this.  Programs should be quiet unless they got something
>> >> important to say.  Can't see why this particular default is more
>> >> important than all the other defaults we don't print.
>> >> 
>> >> The default could be documented in output of --help.
>> >
>> > Actually my thought was this is not using the default value and
>> > implicitly. The default of 'to' is 0, while in this case (when no
>> > display option defined and SDL not configured in), it implicitly uses
>> > non-default value '99'. Therefore I thought it shall be explicitly print
>> > out so that user would be aware of what was chosen on behalf of him;
>> > like the final print of 'VNC server running on '::1;5900''. 
>> 
>> The default depends on configuration options.  Ideally, --help output
>> would show the defaults for this build's configuration.
>
> 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

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

Re: [Qemu-devel] [PATCH 2/2] Explicitly print out default vnc option in use

[Robert Hu]

On Mon, 2016-06-06 at 09:28 +0200, Markus Armbruster wrote:
> Robert Hu <robert.hu@vmm.sh.intel.com> writes:
> 
> > On Tue, 2016-05-31 at 13:17 +0200, Markus Armbruster wrote:
> >> Robert Hu <robert.hu@vmm.sh.intel.com> 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?

Re: [Qemu-devel] [PATCH 2/2] Explicitly print out default vnc option in use

[Robert Hu]

On Tue, 2016-06-07 at 08:28 +0800, Robert Hu wrote:
> On Mon, 2016-06-06 at 09:28 +0200, Markus Armbruster wrote:
> > Robert Hu <robert.hu@vmm.sh.intel.com> writes:
> > 
> > > On Tue, 2016-05-31 at 13:17 +0200, Markus Armbruster wrote:
> > >> Robert Hu <robert.hu@vmm.sh.intel.com> 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?
Oh, I see Paolo already get it in. Thanks!
> 
> 

Re: [Qemu-devel] [PATCH 2/2] Explicitly print out default vnc option in use

[Markus Armbruster]

Robert Hu <robert.hu@vmm.sh.intel.com> writes:

> On Mon, 2016-06-06 at 09:28 +0200, Markus Armbruster wrote:
>> Robert Hu <robert.hu@vmm.sh.intel.com> writes:
>> 
>> > On Tue, 2016-05-31 at 13:17 +0200, Markus Armbruster wrote:
>> >> Robert Hu <robert.hu@vmm.sh.intel.com> 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.

Imperfect English can be addressed in review.  Can be inefficient when
most of the work is English rather than code.  But if you want to try
anyway, go right ahead regardless.

[...]

Re: [Qemu-devel] [PATCH 2/2] Explicitly print out default vnc option in use

[Robert Hu]

On Wed, 2016-06-08 at 16:22 +0200, Markus Armbruster wrote:
> Robert Hu <robert.hu@vmm.sh.intel.com> writes:
> 
> > On Mon, 2016-06-06 at 09:28 +0200, Markus Armbruster wrote:
> >> Robert Hu <robert.hu@vmm.sh.intel.com> writes:
> >> 
> >> > On Tue, 2016-05-31 at 13:17 +0200, Markus Armbruster wrote:
> >> >> Robert Hu <robert.hu@vmm.sh.intel.com> 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.
> 
> Imperfect English can be addressed in review.  Can be inefficient when
> most of the work is English rather than code.  But if you want to try
> anyway, go right ahead regardless.

Hi Markus,

After did some part-time investigation, I'd like to confirm with you:
this change you required seems also just qemu-options.hx involved, am I
right?
If so, I'd like to do that.

> 
> [...]

Re: [Qemu-devel] [PATCH 2/2] Explicitly print out default vnc option in use

[Markus Armbruster]

Robert Hu <robert.hu@vmm.sh.intel.com> writes:

> On Wed, 2016-06-08 at 16:22 +0200, Markus Armbruster wrote:
>> Robert Hu <robert.hu@vmm.sh.intel.com> writes:
>> 
>> > On Mon, 2016-06-06 at 09:28 +0200, Markus Armbruster wrote:
>> >> Robert Hu <robert.hu@vmm.sh.intel.com> writes:
>> >> 
>> >> > On Tue, 2016-05-31 at 13:17 +0200, Markus Armbruster wrote:
>> >> >> Robert Hu <robert.hu@vmm.sh.intel.com> 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.
>> 
>> Imperfect English can be addressed in review.  Can be inefficient when
>> most of the work is English rather than code.  But if you want to try
>> anyway, go right ahead regardless.
>
> Hi Markus,
>
> After did some part-time investigation, I'd like to confirm with you:
> this change you required seems also just qemu-options.hx involved, am I
> right?
> If so, I'd like to do that.

Yes, because commmand line option help is defined there.