Amos Kong <ak...@redhat.com> writes:
> On Fri, Mar 07, 2014 at 10:54:09AM +0100, Markus Armbruster wrote:
>> Eric Blake <ebl...@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.
>> >>
>> >> We have macro in qemu-options.hx to generate a table that
>> >> contains all the options. This patch tries to query options
>> >> from the table.
>> >>
>> >> Then we won't lose the legacy options that weren't added to
>> >> vm_config_groups[] (eg: -vnc, -smbios). The options that have
>> >> no argument will also be returned (eg: -enable-fips)
>> >>
>> >> Some options that have argument have a NULL desc list, some
>> >> options don't have argument, and "parameters" is mandatory
>> >> in the past. So we add a new field "argument" to present
>> >> if the option takes unspecified arguments.
>> >
>> > I like Markus' suggestion of naming the new field
>> > 'unspecified-parameters' rather than 'argument'.
>
> Hi Markus,
>
>> Looking again, there are more problems.
>>
>> 1. Non-parameter argument vs. parameter argument taking unspecified
>> parameters
>>
>> 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.
>
> '-cdrom' is the 'other argument' == 'Non-parameter argument'?
Let me clarify my terminology:
* A "parameter argument" is an option argument of the form KEY=VALUE,...
Since parameter arguments should always be parsed with QemuOpts[*], I
use the term "QemuOpts argument" interchangeably.
* A "non-parameter argument" or "other argument" is an option argument
that doesn't use this form.
Does that answer your question?
> We can use a enum state.
I'm not sure I got that.
> | { 'enum': 'ArgumentStateType',
> | 'data': ['no-argument', 'unspecified-parameters-argument',
> | 'non-parameter-argument']
> | }
> |
> | { 'type': 'CommandLineOptionInfo',
> | 'data': { 'option': 'str', 'parameters': ['CommandLineParameterInfo'],
> | '*argument-state': 'ArgumentStateType' } }
>
>> parameters is [] unless it's a QemuOpts argument. Then it lists the
>> recognized parameters.
>
> How about balloon? we should treat as 'taking unspecified parameters'?
>
> "-balloon none disable balloon device\n"
> "-balloon virtio[,addr=str]\n"
>
> I think we can only check this from help message in qemu-options.hx,
> is it a stable/acceptable method?
-balloon is yet another sugar option:
* "-balloon none" does nothing. It could suppress a default balloon
device, if such a thing existed.
* "-balloon virtio,KEY=VALUE..." is sugar for "-device
virtio-balloon,KEY=VALUE...". Keys other than "addr" are
undocumented.
The actual option argument parsing is ad hoc, not QemuOpts.
I sure hope something like this would not pass review today.
My advice to tools introspecting the command line is to avoid sugared
options, unless the desugaring encapsulates something they don't want to
know.
> We introduce query-command-line-options command to avoid libvirt to
> check qemu commandline information by scanning qemu's help message,
> it's not strict & stable.
>
>> 2. Our dear friend -drive is more complicated than you might think
>>
>> We special-case it to report the union of drive_config_groups[],
>> which contains qemu_legacy_drive_opts, qemu_common_drive_opts and
>> qemu_drive_opts. The latter accepts unspecified parameters.
>
> I'm confused here. But there is only one option (-drive), we should
> return merged desc table (legacy + common).
>
> Desc table of qemu_drive_opts is NULL, then -drive can also take
> unspecified parameters?
Yes: driver-specific parameters.
-drive takes currently takes unspecified parameters (the driver-specific
parameters) in addition to a number of specified parameters (the common
and legacy parameters).
>> I believe qemu_drive_opts is actually not used by the (complex!) code
>> parsing the argument of -drive.
>>
>> Nevertheless, said code accepts more than just qemu_legacy_drive_opts
>> and qemu_common_drive_opts, namely driver-specific parameters.
>>
>> Until we define those properly in a schema, I guess the best we can
>> do is add one more case: option takes QemuOpts argument, but
>> parameters is not exhaustive.
>
>
> Thanks, Amos
[*] Leftovers still parsed by other means, if any, should be converted
to QemuOpts.
