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

On Fri, Mar 07, 2014 at 10:54:09AM +0100, Markus Armbruster wrote: > Eric Blake <eblake(a)redhat.com&gt; 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'?

We can use a enum state.

| { '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?

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?

> 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