Re: [Qemu-devel] [libvirt] [PULL 04/14] audio: -audiodev command line option basic implementation

[Markus Armbruster]

Markus Armbruster <address@hidden> writes:

> Pavel Hrdina <address@hidden> writes:
>
>> On Tue, Mar 12, 2019 at 08:12:40AM +0100, Gerd Hoffmann wrote:
>>> From: Kővágó, Zoltán <address@hidden>
>>> 
>>> Audio drivers now get an Audiodev * as config paramters, instead of the
>>> global audio_option structs.  There is some code in audio/audio_legacy.c
>>> that converts the old environment variables to audiodev options (this
>>> way backends do not have to worry about legacy options).  It also
>>> contains a replacement of -audio-help, which prints out the equivalent
>>> -audiodev based config of the currently specified environment variables.
>>> 
>>> Note that backends are not updated and still rely on environment
>>> variables.
>>> 
>>> Also note that (due to moving try-poll from global to backend specific
>>> option) currently ALSA and OSS will always try poll mode, regardless of
>>> environment variables or -audiodev options.
>>
>> Hi,
>>
>> I'm glad that this is merged now and I wanted to start working on
>> libvirt patches, but there is one big issue with this command,
>> it's not introspectable by query-command-line-options.
>>
>> My guess based on the QEMU code is that it uses the new function that
>> allows you to use JSON on qemu command line.
>>
>> Without the introspection libvirt cannot implement the new option in
>> sensible way (without version check).
>>
>> Adding Markus to CC so we can figure out how to wire up the
>> introspection for such command line options.
>
> query-command-line-options has always been woefully incomplete.  Sadly,
> my replacement is still not ready.
>
> A reliable "witness" could serve a stop gap.  Unfortunately,
> query-qmp-schema doesn't provide one: the series does not change
> generated qapi-introspect.c.
>
> Need to think some more.

There is no witness in query-qmp-schema.

We don't want to parse output of -help.

query-command-line-options sucks.  To recap, here are its known defects:

0. It doesn't actually return information on command line options, it
returns information on QemuOpts.  All the other defects follow from this
one.

1. It fails to report most command line options.  Example: -audio.

2. It sometimes screws up the option name.  Example: -m is reported as
   "option": "memory".

2. It sometimes reports no parameters when there are in fact parameters.
   Example: -netdev.

3. It sometimes misses parameters.  Example: -drive misses driver-
   specific parameters.

Can we make q-c-l-o suck less?

Command line options are actually defined in qemu-options.hx, which gets
massaged into qemu_options[].  For each option, qemu_options[] gives us
the option name (without leading '-'), and whether the option takes an
argument.

This is less information per option than q-c-l-o provides now.  Can we
add it to its output anyway without confusing existing users?

Trouble is the QAPI schema is rather inflexible.  It returns a array of
objects with two members "option" and "parameters".  Strictly speaking,
the following are all ABI:

    "option" is the name of the option (modulo the defect mentioned
    above).

    The option takes an argument.

    The argument uses QemuOpts syntax.

    At least the parameters in "parameters" are recognized.

Therefore,

    we can't add options that take no argument, and

    we can't add options with different argument syntax

without breaking ABI.

Alternatives:

1. We add only options that take an argument, like this:

        {"option": "NAME", "parameters": []}

   The meaning of "parameters": [] changes.  In addition to "argument
   uses QemuOpts syntax, parameters unknown", it can now also mean
   "argument uses some other syntax".

2. We add all options that way.

   The meaning of "parameters": [] changes more drastically.  In
   addition to "argument uses QemuOpts syntax, parameters unknown", it
   can now also mean "argument uses some other syntax" and "no
   argument".

3. We add options that take an argument like

        {"option": "NAME", "parameters": null}

   and options that don't take one like

        {"option": "NAME"}

   This turns the *semantic* ABI breaks into *syntactic* ones.

4. We make the syntactic ABI breaks opt-in, i.e. make q-c-l-o take a
flag.

5. Screw it, create a new query command to return just the information
   from qemu_options[].

Alternatives 1. to 3. break ABI in different ways.  Finding out which of
the ABI breaks upset libvirt would be easy enough.  But even if one of
them is fine with libvirt, ABI breaks are best avoided.  Risking one in
4.0 is out of the question, we're far too late in the cycle.

Alternative 5. feels simpler than 4., and is similarly ugly.  This might
still be acceptable (barely) for 4.0.

Opinions?