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

[Amos Kong <akong@redhat.com>]

On Wed, Feb 12, 2014 at 02:00:51PM -0700, Eric Blake wrote:
> On 01/27/2014 08:53 PM, Amos Kong wrote:
> > 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"
> >  },
> > 
> > Signed-off-by: Amos Kong <akong@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
> 
> Missing "#optional" and "(since 2.0)" designations
> 
> >  #
> >  # @parameters: an array of @CommandLineParameterInfo
> 
> Might be worth documenting "#optional since 2.0" (we don't yet have good
> precedent for documenting when a formerly mandatory field became optional).
> 
> Groan.  This is an output struct.  On input structs, changing a
> mandatory field to optional is safe - old callers will always supply the
> field.  But on output structs, changing a mandatory field to optional is
> backwards-incompatible.  Old callers may be blindly expecting the field,
> and crash when it is not present.
> 
> Your approach needs to be modified.
> 
> >  #
> >  # Since 1.5
> >  ##
> >  { 'type': 'CommandLineOptionInfo',
> > -  'data': { 'option': 'str', 'parameters': ['CommandLineParameterInfo'] } }
> > +  'data': { 'option': 'str',
> > +            '*parameters': ['CommandLineParameterInfo'],
> > +            '*helpmsg': 'str' } }
> 
> I suggest:
> 
> 
> # @parameters: array of @CommandLineParameterInfo, possibly empty
> # @argument: @optional present if the @parameters array is empty. If
> #            true, then the option takes unspecified arguments, if
> #            false, then the option is merely a boolean flag (since 2.0)
> 
> { 'type': 'CommandLineOptionInfo',
> 'data': { 'option': 'str', 'parameters': ['CommandLineParameterInfo'],
>           '*argument': 'bool' } }
> 
> used as:
> [
>   { "option":"enable-fips", "parameters":[], "argument":false },
>   { "option":"smbios", "parameters":[], "argument":true },
>   { "option":"iscsi", "paramters":[ ... ] },
>   ...
> ]
 
It's ok to use a split "argument" to indicate if option has
parameters. and the parameters will not be optional, it will
be NULL list in the case of no parameters.

Thanks.

> which adequately differentiates between -iscsi taking arguments (but
> where we haven't yet hooked it in to introspect those arguments) vs.
> -enable-fips being boolean.
> 
> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
> 


-- 
			Amos.