Re: [Qemu-devel] [PATCH v4 2/2] query-command-line-options: query all the options in qemu-options.hx

[Amos Kong <akong@redhat.com>]

On Thu, Mar 20, 2014 at 10:12:43PM +0800, Amos Kong wrote:
> On Tue, Mar 11, 2014 at 10:04:56AM +0100, Markus Armbruster wrote:
> > Eric Blake <eblake@redhat.com> writes:
> > 
> > > On 03/07/2014 02:54 AM, Markus Armbruster wrote:
> > >> Eric Blake <eblake@redhat.com> writes:
> > >> 
> > >>> On 03/05/2014 07:36 PM, Amos Kong wrote:
> > >>>> vm_config_groups[] only contains part of the options which have
> > >>>> argument, and all options which have no argument aren't added
> > >>>> to vm_config_groups[]. Current query-command-line-options only
> > >>>> checks options from vm_config_groups[], so some options will
> > >>>> be lost.
> > >>>>
> > >
> > >>    Example: -device takes unspecified parameters.  -cdrom doesn't take
> > >>    parameters, it takes a file name.  Yet, the command reports the same
> > >>    for both: "parameters": [], "argument": true.
> > >> 
> > >>    Looks like we need a tri-state: option takes no argument, QemuOpts
> > >>    argument, or other argument.
> > >
> > > I don't buy that.  '-cdrom filename' could easily be re-written [in a
> > > future qemu version] to use QemuOpts with an implied parameter name
> > > (we've done that elsewhere, such as for '-machine').  In other words, I
> > > think we could make it become shorthand for '-cdrom file=filename', at
> > > which point the QemuOpts spelling is available and would now show up as
> > > "parameters":[{"name":"file"...}].  Thus, in converting -cdrom to
> > > QemuOpts, we can still maintain command-line back-compat, while making
> > > the query-command-line-options output more featureful.  In other words,
> > > _for now_ it takes unspecified parameters, and the fact that it is only
> > > a single parameter in the form 'filename' rather than a more typical
> > > parameter 'file=filename' is not a show-stopper.
> > 
> > Incompatible change for funny filenames: -cdrom you,break=me.
> > 
> > Besides breaking funny filenames, we'd also buy ourselves some stupid
> > -readconfig / -writeconfig trouble.  Let me explain.
> > 
> > -cdrom F is effectively sugar for "-drive media=cdrom,index=2,file=FF",
> > where FF is F with comma doubled.
> > 
> > -writeconfig writes out desugared QemuOpts.  Therefore, "-cdrom r7.iso"
> > gets written as
> > 
> >     [drive]
> >       media = "cdrom"
> >       index = "2"
> >       file = "r7.iso"
> > 
> > which -readconfig can read.
> > 
> > If we convert -cdrom to QemuOpts, it gets written out like this:
> > 
> >     [cdrom]
> >        file = "r7.iso"
> > 
> > If we continue to desugar it, it'll *also* get written out as before.
> > Either we *delete* the sugared QemuOpts to avoid duplication, or we
> > *stop* desugaring.  The latter breaks -readconfig of existing
> > configuration files, and complicates the code reading configuration from
> > QemuOpts.
> > 
> > I don't think any of the old non-QemuOpts options that have become sugar
> > for newer, more flexible QemuOpts options should be converted to
> > QemuOpts.
> > 
> > > So your idea of a tri-state (QemuOpts, no argument, or other argument)
> > > doesn't add anything - any option that takes "other argument" could be
> > > converted to take QemuOpts, and from the command line, we can't tell the
> > > difference from whether something was implemented by QemuOpts, only by
> > > whether we have introspection on what the argument consists of.
> > 
> > I doubt we can convert all existing options to QemuOpts without breaking
> > backward compatibility and complicating the code.
> > 
> > > Meanwhile, it DOES point out that our use of implicit argument in
> > > QemuOpts ought to be exposed to the introspection mechanism, for
> > > introspection to be fully descriptive.  That is, maybe we should modify
> > > our introspection to add a new 'implied-name':
> > >
> > > ##
> > > # @CommandLineParameterInfo:
> > > #
> > > ...
> > > # @implied-name: #optional, if present and true, the parameter can be
> > > #                specified as '-option value' instead of the preferred
> > > #                spelling of '-option name=value' (since 2.0)
> > > # Since 1.5
> > > { 'type': 'CommandLineParameterInfo',
> > >   'data': { 'name': 'str',
> > >             'type': 'CommandLineParameterType',
> > >             '*help': 'str', '*implied-name': 'bool' } }
  
reply-myself

> How can we get this information? it's not good to rely on the help message.
> 
> And the parameters [] only have content when the option have a non-NULL desc
> table, so we always just return a NULL parameters list, the 'implied-name'
> information will be lost.

I'm wrong, 'implied-name' is attribute of parameter, it's stored in
QemuOptsList.implied_opt_name.

I will add this filed in V6.
 
> I thinks Markus's suggestion is fine, we can use tri-state (no-arg,
> unsuecified-para-arg, no-para-arg).

Thanks, Amos
 
> > The only use for implied-name I can think of is interpreting a user's
> > command line.  Is that a real use case?
> 
>  
> > >> 
> > >>    parameters is [] unless it's a QemuOpts argument.  Then it lists the
> > >>    recognized parameters.
> > >
> > > This part is still true.  When parameters[] is non-empty, it is a
> > > QemuOpts and we know all recognized parameters (well, more precisely,
> > > the subset of QemuOpts that were explicitly called out - given your
> > > point 2 about the mess of -drive); when it is empty, then all we know is
> > > whether the argument is a boolean or takes unspecified arguments (where
> > > the conversion of those unknown arguments to QemuOpts will be what
> > > finally lets us introspect the format of those unknown arguments).
> > 
> > QemuOpts argument with only unspecified parameters is not the same as
> > non-QemuOpts argument.  I don't think conflating the two is useful.