[Qemu-devel] [PATCH] chardev: Add 'help' option to print all available chardev backend types

[Qemu-devel] [PATCH] chardev: Add 'help' option to print all available chardev backend types

[Lin Ma]

Signed-off-by: Lin Ma <lma@suse.com>
---
 qemu-char.c     | 21 ++++++++++++++++-----
 qemu-options.hx |  3 +++
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 8a0ab05..8a7aef3 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -39,6 +39,7 @@
 #include "io/channel-file.h"
 #include "io/channel-tls.h"
 #include "sysemu/replay.h"
+#include "qemu/help_option.h"
 
 #include <zlib.h>
 
@@ -3877,16 +3878,26 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
     const char *id = qemu_opts_id(opts);
     char *bid = NULL;
 
-    if (id == NULL) {
-        error_setg(errp, "chardev: no id specified");
-        goto err;
-    }
-
     if (qemu_opt_get(opts, "backend") == NULL) {
         error_setg(errp, "chardev: \"%s\" missing backend",
                    qemu_opts_id(opts));
         goto err;
     }
+
+    if (is_help_option(qemu_opt_get(opts, "backend"))) {
+        fprintf(stderr, "Available chardev backend types:\n");
+        for (i = backends; i; i = i->next) {
+            cd = i->data;
+            fprintf(stderr, "%s\n", cd->name);
+        }
+        exit(!is_help_option(qemu_opt_get(opts, "backend")));
+    }
+
+    if (id == NULL) {
+        error_setg(errp, "chardev: no id specified");
+        goto err;
+    }
+
     for (i = backends; i; i = i->next) {
         cd = i->data;
 
diff --git a/qemu-options.hx b/qemu-options.hx
index a71aaf8..379f7a5 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2148,6 +2148,7 @@ The general form of a character device option is:
 ETEXI
 
 DEF("chardev", HAS_ARG, QEMU_OPTION_chardev,
+    "-chardev help\n"
     "-chardev null,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
     "-chardev socket,id=id[,host=host],port=port[,to=to][,ipv4][,ipv6][,nodelay][,reconnect=seconds]\n"
     "         [,server][,nowait][,telnet][,reconnect=seconds][,mux=on|off]\n"
@@ -2213,6 +2214,8 @@ Backend is one of:
 @option{spiceport}.
 The specific backend will determine the applicable options.
 
+Use "-chardev help" to print all available chardev backend types.
+
 All devices must have an id, which can be any string up to 127 characters long.
 It is used to uniquely identify this device in other command line directives.
 
-- 
2.9.2

Re: [Qemu-devel] [PATCH] chardev: Add 'help' option to print all available chardev backend types

[Marc-André Lureau]

Hi

On Tue, Aug 16, 2016 at 9:18 PM Lin Ma <lma@suse.com> wrote:

> Signed-off-by: Lin Ma <lma@suse.com>
> ---
>  qemu-char.c     | 21 ++++++++++++++++-----
>  qemu-options.hx |  3 +++
>  2 files changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/qemu-char.c b/qemu-char.c
> index 8a0ab05..8a7aef3 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -39,6 +39,7 @@
>  #include "io/channel-file.h"
>  #include "io/channel-tls.h"
>  #include "sysemu/replay.h"
> +#include "qemu/help_option.h"
>
>  #include <zlib.h>
>
> @@ -3877,16 +3878,26 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts
> *opts,
>      const char *id = qemu_opts_id(opts);
>      char *bid = NULL;
>
> -    if (id == NULL) {
> -        error_setg(errp, "chardev: no id specified");
> -        goto err;
> -    }
> -
>      if (qemu_opt_get(opts, "backend") == NULL) {
>          error_setg(errp, "chardev: \"%s\" missing backend",
>                     qemu_opts_id(opts));
>          goto err;
>      }
> +
> +    if (is_help_option(qemu_opt_get(opts, "backend"))) {
> +        fprintf(stderr, "Available chardev backend types:\n");
> +        for (i = backends; i; i = i->next) {
> +            cd = i->data;
> +            fprintf(stderr, "%s\n", cd->name);
> +        }
> +        exit(!is_help_option(qemu_opt_get(opts, "backend")));
> +    }
> +
> +    if (id == NULL) {
> +        error_setg(errp, "chardev: no id specified");
> +        goto err;
> +    }
> +
>      for (i = backends; i; i = i->next) {
>          cd = i->data;
>
> diff --git a/qemu-options.hx b/qemu-options.hx
> index a71aaf8..379f7a5 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -2148,6 +2148,7 @@ The general form of a character device option is:
>  ETEXI
>
>  DEF("chardev", HAS_ARG, QEMU_OPTION_chardev,
> +    "-chardev help\n"
>      "-chardev null,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
>      "-chardev
> socket,id=id[,host=host],port=port[,to=to][,ipv4][,ipv6][,nodelay][,reconnect=seconds]\n"
>      "
>  [,server][,nowait][,telnet][,reconnect=seconds][,mux=on|off]\n"
> @@ -2213,6 +2214,8 @@ Backend is one of:
>  @option{spiceport}.
>  The specific backend will determine the applicable options.
>
> +Use "-chardev help" to print all available chardev backend types.
> +
>

How different is it from the list in qemu -help ? Why duplicate that list
with less informations (arguments and details) ? Do you expect it to be
easily machine readable?


>  All devices must have an id, which can be any string up to 127 characters
> long.
>  It is used to uniquely identify this device in other command line
> directives.
>
> --
> 2.9.2
>
>
> --
Marc-André Lureau

[Qemu-devel] 答复： Re: [PATCH] chardev: Add 'help' option to print all available chardev backend types

[Lin Ma]

>>> Marc-André Lureau <marcandre.lureau@gmail.com> 8/17/2016 1:25 上午 >>>
>Hi
>
>On Tue, Aug 16, 2016 at 9:18 PM Lin Ma <lma@suse.com> wrote:
>
>> Signed-off-by: Lin Ma <lma@suse.com>
>> ---
>>  qemu-char.c     | 21 ++++++++++++++++-----
>>  qemu-options.hx |  3 +++
>>  2 files changed, 19 insertions(+), 5 deletions(-)
>>
>> diff --git a/qemu-char.c b/qemu-char.c
>> index 8a0ab05..8a7aef3 100644
>> --- a/qemu-char.c
>> +++ b/qemu-char.c
>> @@ -39,6 +39,7 @@
>>  #include "io/channel-file.h"
>>  #include "io/channel-tls.h"
>>  #include "sysemu/replay.h"
>> +#include "qemu/help_option.h"
>>
>>  #include <zlib.h>
>>
>> @@ -3877,16 +3878,26 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts
>> *opts,
>>	  const char *id = qemu_opts_id(opts);
>>	  char *bid = NULL;
>>
>> -    if (id == NULL) {
>> -	    error_setg(errp, "chardev: no id specified");
>> -	    goto err;
>> -    }
>> -
>>	  if (qemu_opt_get(opts, "backend") == NULL) {
>>		  error_setg(errp, "chardev: \"%s\" missing backend",
>>					 qemu_opts_id(opts));
>>		  goto err;
>>	  }
>> +
>> +    if (is_help_option(qemu_opt_get(opts, "backend"))) {
>> +	    fprintf(stderr, "Available chardev backend types:\n");
>> +	    for (i = backends; i; i = i->next) {
>> +		    cd = i->data;
>> +		    fprintf(stderr, "%s\n", cd->name);
>> +	    }
>> +	    exit(!is_help_option(qemu_opt_get(opts, "backend")));
>> +    }
>> +
>> +    if (id == NULL) {
>> +	    error_setg(errp, "chardev: no id specified");
>> +	    goto err;
>> +    }
>> +
>>	  for (i = backends; i; i = i->next) {
>>		  cd = i->data;
>>
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index a71aaf8..379f7a5 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -2148,6 +2148,7 @@ The general form of a character device option is:
>>  ETEXI
>>
>>  DEF("chardev", HAS_ARG, QEMU_OPTION_chardev,
>> +    "-chardev help\n"
>>	  "-chardev null,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
>>	  "-chardev
>> socket,id=id[,host=host],port=port[,to=to][,ipv4][,ipv6][,nodelay][,reconnect=seconds]\n"
>>	  "
>>  [,server][,nowait][,telnet][,reconnect=seconds][,mux=on|off]\n"
>> @@ -2213,6 +2214,8 @@ Backend is one of:
>>  @option{spiceport}.
>>  The specific backend will determine the applicable options.
>>
>> +Use "-chardev help" to print all available chardev backend types.
>> +
>>
>
>How different is it from the list in qemu -help ? Why duplicate that list
>with less informations (arguments and details) ? Do you expect it to be
>easily machine readable?
>
No special reason, Just add 'help' option to make it more friendly, like the
output of '-watchdog help' or '-tpmdev help', Sometimes I forget the chardev
backend names which I want to try, I'd like to have a very easy&fast way to
get the backend list :-).
 
Various backends have various arguments, Printing all of them causes the
complex output, that was my thought before I send the patch.
Now...yes, it makes sense that including the arguments in output, I can add
them if the patch's idea is acceptable.
 
 
Lin

Re: [Qemu-devel] 答复： Re: [PATCH] chardev: Add 'help' option to print all available chardev backend types

[Markus Armbruster]

"Lin Ma" <lma@suse.com> writes:

>>>> Marc-André Lureau <marcandre.lureau@gmail.com> 8/17/2016 1:25 上午 >>>
[...]
>>How different is it from the list in qemu -help ? Why duplicate that list
>>with less informations (arguments and details) ? Do you expect it to be
>>easily machine readable?
>>
> No special reason, Just add 'help' option to make it more friendly, like the
> output of '-watchdog help' or '-tpmdev help', Sometimes I forget the chardev
> backend names which I want to try, I'd like to have a very easy&fast way to
> get the backend list :-).
>  
> Various backends have various arguments, Printing all of them causes the
> complex output, that was my thought before I send the patch.
> Now...yes, it makes sense that including the arguments in output, I can add
> them if the patch's idea is acceptable.

You could do it like -device / device_add: argument help lists types,
and argument T,help shows additional help for type T.

Re: [Qemu-devel] 答复： Re: [PATCH] chardev: Add 'help' option to print all available chardev backend types

[Lin Ma]

>>> Markus Armbruster <armbru@redhat.com> 2016/8/17 星期三 下午 2:52 >>>
>"Lin Ma" <lma@suse.com> writes:
>
>>>>> Marc-André Lureau <marcandre.lureau@gmail.com> 8/17/2016 1:25 上午 >>>
>[...]
>>>How different is it from the list in qemu -help ? Why duplicate that list
>>>with less informations (arguments and details) ? Do you expect it to be
>>>easily machine readable?
>>>
>> No special reason, Just add 'help' option to make it more friendly, like the
>> output of '-watchdog help' or '-tpmdev help', Sometimes I forget the chardev
>> backend names which I want to try, I'd like to have a very easy&fast way to
>> get the backend list :-).
>>  
>> Various backends have various arguments, Printing all of them causes the
>> complex output, that was my thought before I send the patch.
>> Now...yes, it makes sense that including the arguments in output, I can add
>> them if the patch's idea is acceptable.
>
>You could do it like -device / device_add: argument help lists types,
>and argument T,help shows additional help for type T.
Because chardev doesn't have qdev properties, most of chardev options are defined
in qemu_chardev_opts in qemu-char.c, I'd like to hard code the argument output
like these format:
......
vc,id=id[[,width=width][,height=height]][[,cols=cols][,rows=rows]]
socket,id=id[,host=host],port=host[,to=to][,ipv4][,ipv6][,nodelay]
stdio,id=id[,mux=on|off][,signal=on|off]
......
 
May I have your thought?
 
Thanks,
Lin

Re: [Qemu-devel] 答复： Re: [PATCH] chardev: Add 'help' option to print all available chardev backend types

[Paolo Bonzini]

On 25/08/2016 08:08, Lin Ma wrote:
> 
> 
>>>> Markus Armbruster <armbru@redhat.com> 2016/8/17 星期三 下午 2:52 >>>
>>"Lin Ma" <lma@suse.com> writes:
>>
>>>>>> Marc-André Lureau <marcandre.lureau@gmail.com> 8/17/2016 1:25 上午 >>>
>>[...]
>>>>How different is it from the list in qemu -help ? Why duplicate that list
>>>>with less informations (arguments and details) ? Do you expect it to be
>>>>easily machine readable?
>>>>
>>> No special reason, Just add 'help' option to make it more friendly,
> like the
>>> output of '-watchdog help' or '-tpmdev help', Sometimes I forget the
> chardev
>>> backend names which I want to try, I'd like to have a very easy&fast
> way to
>>> get the backend list :-).
>>> 
>>> Various backends have various arguments, Printing all of them causes the
>>> complex output, that was my thought before I send the patch.
>>> Now...yes, it makes sense that including the arguments in output, I
> can add
>>> them if the patch's idea is acceptable.
>>
>>You could do it like -device / device_add: argument help lists types,
>>and argument T,help shows additional help for type T.
> Because chardev doesn't have qdev properties, most of chardev options
> are defined
> in qemu_chardev_opts in qemu-char.c, I'd like to hard code the argument
> output
> like these format:
> ......
> vc,id=id[[,width=width][,height=height]][[,cols=cols][,rows=rows]]
> socket,id=id[,host=host],port=host[,to=to][,ipv4][,ipv6][,nodelay]
> stdio,id=id[,mux=on|off][,signal=on|off]

I don't think this is a good idea if you want the output to be
machine-readable.  I think a simple "-chardev help" is good because it's
a simple addition and something obvious for users to try.

Paolo

Re: [Qemu-devel] [PATCH] chardev: Add 'help' option to print all available chardev backend types

[Paolo Bonzini]

On 16/08/2016 19:13, Lin Ma wrote:
> Signed-off-by: Lin Ma <lma@suse.com>
> ---
>  qemu-char.c     | 21 ++++++++++++++++-----
>  qemu-options.hx |  3 +++
>  2 files changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/qemu-char.c b/qemu-char.c
> index 8a0ab05..8a7aef3 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -39,6 +39,7 @@
>  #include "io/channel-file.h"
>  #include "io/channel-tls.h"
>  #include "sysemu/replay.h"
> +#include "qemu/help_option.h"
>  
>  #include <zlib.h>
>  
> @@ -3877,16 +3878,26 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
>      const char *id = qemu_opts_id(opts);
>      char *bid = NULL;
>  
> -    if (id == NULL) {
> -        error_setg(errp, "chardev: no id specified");
> -        goto err;
> -    }
> -
>      if (qemu_opt_get(opts, "backend") == NULL) {
>          error_setg(errp, "chardev: \"%s\" missing backend",
>                     qemu_opts_id(opts));
>          goto err;
>      }
> +
> +    if (is_help_option(qemu_opt_get(opts, "backend"))) {
> +        fprintf(stderr, "Available chardev backend types:\n");
> +        for (i = backends; i; i = i->next) {
> +            cd = i->data;
> +            fprintf(stderr, "%s\n", cd->name);
> +        }
> +        exit(!is_help_option(qemu_opt_get(opts, "backend")));
> +    }
> +
> +    if (id == NULL) {
> +        error_setg(errp, "chardev: no id specified");
> +        goto err;
> +    }
> +
>      for (i = backends; i; i = i->next) {
>          cd = i->data;
>  
> diff --git a/qemu-options.hx b/qemu-options.hx
> index a71aaf8..379f7a5 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -2148,6 +2148,7 @@ The general form of a character device option is:
>  ETEXI
>  
>  DEF("chardev", HAS_ARG, QEMU_OPTION_chardev,
> +    "-chardev help\n"
>      "-chardev null,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
>      "-chardev socket,id=id[,host=host],port=port[,to=to][,ipv4][,ipv6][,nodelay][,reconnect=seconds]\n"
>      "         [,server][,nowait][,telnet][,reconnect=seconds][,mux=on|off]\n"
> @@ -2213,6 +2214,8 @@ Backend is one of:
>  @option{spiceport}.
>  The specific backend will determine the applicable options.
>  
> +Use "-chardev help" to print all available chardev backend types.
> +
>  All devices must have an id, which can be any string up to 127 characters long.
>  It is used to uniquely identify this device in other command line directives.
>  
> 

Queued for 2.8, thanks.

Paolo