| Message ID | 20160816171352.17021-1-lma@suse.com |
|---|---|
| State | New |
| Headers | show |
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
>>> 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
"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.
>>> 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
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
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
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.
Signed-off-by: Lin Ma <lma@suse.com> --- qemu-char.c | 21 ++++++++++++++++----- qemu-options.hx | 3 +++ 2 files changed, 19 insertions(+), 5 deletions(-)