[libvirt] [Qemu-devel] [PATCH 5/5] query-command-line-options: return help message for legacy options

[Amos Kong]

On Tue, Feb 11, 2014 at 01:19:16PM +0100, Markus Armbruster wrote:
> [Note cc: Eric]
 
Hi Markus,

> Amos Kong <akong at redhat.com> writes:
> 
> > Some legacy options that have arguments weren't added to
> > vm_config_groups[], so query-command-line-options returns a
> > NULL parameters infolist. This patch try to return help message
> > for this kind of legacy options.
> >
> > Example:
> >  {
> >      "helpmsg": "\"-vnc display    start a VNC server on display\\n\"",
> >      "parameters": [
> >      ],
> >      "option": "vnc"
> >  },
> 
> Do we have prospective users for this feature?

I had posted a RFC mail to discuss about "fix/redo query-command-line-options",
this patch is a solution for legacy options that have not arguments.

Current QEMU returns NULL list for this kind of options, this patch
tries to return option name and help message to the libvirt. (it's
better)

You can find some examples in the end.
 
> > Signed-off-by: Amos Kong <akong at redhat.com>
> > ---
> >  qapi-schema.json   | 5 ++++-
> >  util/qemu-config.c | 3 +++
> >  2 files changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/qapi-schema.json b/qapi-schema.json
> > index 05ced9d..b3e6f46 100644
> > --- a/qapi-schema.json
> > +++ b/qapi-schema.json
> > @@ -3943,13 +3943,16 @@
> >  # Details about a command line option, including its list of parameter details
> >  #
> >  # @option: option name
> > +# @helpmsg: help message for legacy options
> >  #
> >  # @parameters: an array of @CommandLineParameterInfo
> >  #
> >  # Since 1.5
> >  ##
> >  { 'type': 'CommandLineOptionInfo',
> > -  'data': { 'option': 'str', 'parameters': ['CommandLineParameterInfo'] } }
> > +  'data': { 'option': 'str',
> > +            '*parameters': ['CommandLineParameterInfo'],
> > +            '*helpmsg': 'str' } }
> >  
> >  ##
> >  # @query-command-line-options:
> 
> Aha, here's the schema change missing in PATCH 3/5.

Yeah

> The schema needs to cover these kinds of options:
> 
> 1. No argument
> 
>    { 'option': OPT-NAME }
> 
> 2. QemuOpts argument
> 
> 2a. with known parameters (QemuOptsList member desc[] not empty)
> 
>    { 'option': OPT-NAME,
>      'parameters': [ { 'name': PAR-NAME, 'type': PAR-TYPE }, ... ] }
> 
> 2b. with unknown parameters (desc[] empty)
> 
>    { 'option': OPT-NAME, parameters: [] }
> 
> 3. Other argument
> 
>    { 'option': OPT-NAME, ? }
> 
> This patch adds 3.  We need to decide what we want there.
> 
> Any particular reason why we have to invent something new, and can't
> simply fold it into 2b?

I thinks it's ok, it's just the output format, then the 'parameters'
isn't optional.
 
> Note that I completely left out help strings, both on the option and on
> the parameter level.  Both for brevity of presentation, and because I'd
> like to see a use before we add them.

Something was _wrong_ in this patch, I want to additionally output
helpmsg only for this case:
  If option has parameter, and we didn't convert the option to use
  QemuOpts (with good desc table), then help message is helpful.
  This only effects some legacy options.

|-set group.id.arg=value
|                set <arg> parameter for item <id> of type <group>
|                i.e. -set drive.$id.file=/path/to/image
|-qtest-log /dev/null
|-qtest 
|-writeconfig <file>
|              read/write config file
|....
|....

 
> > diff --git a/util/qemu-config.c b/util/qemu-config.c
> > index de233d8..a2def03 100644
> > --- a/util/qemu-config.c
> > +++ b/util/qemu-config.c
> > @@ -176,6 +176,9 @@ CommandLineOptionInfoList *qmp_query_command_line_options(bool has_option,
> >              } else if (idx >= 0) {
> >                  info->parameters =
> >                      get_param_infolist(vm_config_groups[idx]->desc);
> > +            } else if (info->has_parameters) {
> > +                info->has_helpmsg = true;
> > +                info->helpmsg = g_strdup(option_helpmsgs[i]);
> >              }
> >  
> >              entry = g_malloc0(sizeof(*entry));

-- 
			Amos.