Re: [PATCH 1/3] qapi, audio: add query-audiodev command

[Markus Armbruster <armbru@redhat.com>]

Daniel P. Berrangé <berrange@redhat.com> writes:

> Way back in QEMU 4.0, the -audiodev command line option was introduced
> for configuring audio backends. This CLI option does not use QemuOpts
> so it is not visible for introspection in 'query-command-line-options',
> instead using the QAPI Audiodev type.  Unfortunately there is also no
> QMP command that uses the Audiodev type, so it is not introspectable
> with 'query-qmp-schema' either.

This is a gap that will only widen.

By design, query-qmp-schema covers just QMP.  It doesn't cover the
QAPIfied parts of the CLI.  They have been growing slowly, and this
trend will continue.  We need schema introspection to cover the CLI,
too.  Observation, not demand.

Work-arounds:

1. When a QMP command equivalent to a QAPIfied CLI option exists,
introspect that.  Involves hardcoding the connection between the two.
Example: -blockdev and blockdev-add.

2. When a QMP query exists that returns the CLI option argument,
introspect that.  Involves hardcoding the connection between the two.
Example: -display and query-display-options.

3. When neither exists, create a suitable query just to enable
introspection.  Example: query-display-options.  Commit e1ca8f7e19
"qapi: add query-display-options command" explains this clearly:

    Add query-display-options command, which allows querying the qemu
    display configuration.  This isn't particularly useful, except it
    exposes QAPI type DisplayOptions in query-qmp-schema, so that libvirt
    can discover recently added -display parameter rendernode (commit
    d4dc4ab133b).  Works around lack of sufficiently powerful command line
    introspection.

> This introduces a 'query-audiodev' command that simply reflects back
> the list of configured -audiodev command line options. This in turn
> makes Audiodev introspectable via 'query-qmp-schema'.

Is the query just for enabling introspection, or does it have other
uses?

If the former, please steal from Gerd's explanation for your commit
message.

>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  audio/audio.c   | 19 +++++++++++++++++++
>  qapi/audio.json | 13 +++++++++++++
>  2 files changed, 32 insertions(+)
>
> diff --git a/audio/audio.c b/audio/audio.c
> index 6734c8af70..40a4bbd7ce 100644
> --- a/audio/audio.c
> +++ b/audio/audio.c
> @@ -28,8 +28,10 @@
>  #include "monitor/monitor.h"
>  #include "qemu/timer.h"
>  #include "qapi/error.h"
> +#include "qapi/clone-visitor.h"
>  #include "qapi/qobject-input-visitor.h"
>  #include "qapi/qapi-visit-audio.h"
> +#include "qapi/qapi-commands-audio.h"
>  #include "qemu/cutils.h"
>  #include "qemu/module.h"
>  #include "sysemu/replay.h"
> @@ -2201,3 +2203,20 @@ size_t audio_rate_get_bytes(struct audio_pcm_info *info, RateCtl *rate,
>      rate->bytes_sent += ret;
>      return ret;
>  }
> +
> +AudiodevList *qmp_query_audiodevs(Error **errp)
> +{
> +    AudiodevList *ret = NULL, *prev = NULL, *curr;
> +    AudiodevListEntry *e;
> +    QSIMPLEQ_FOREACH(e, &audiodevs, next) {
> +        curr = g_new0(AudiodevList, 1);
> +        curr->value = QAPI_CLONE(Audiodev, e->dev);
> +        if (prev) {
> +            prev->next = curr;
> +            prev = curr;
> +        } else {
> +            ret = prev = curr;
> +        }

Please use QAPI_LIST_APPEND().

> +    }
> +    return ret;
> +}
> diff --git a/qapi/audio.json b/qapi/audio.json
> index 9cba0df8a4..d7b91230d7 100644
> --- a/qapi/audio.json
> +++ b/qapi/audio.json
> @@ -419,3 +419,16 @@
>      'sdl':       'AudiodevSdlOptions',
>      'spice':     'AudiodevGenericOptions',
>      'wav':       'AudiodevWavOptions' } }
> +
> +##
> +# @query-audiodevs:
> +#
> +# Returns information about audiodev configuration
> +#
> +# Returns: array of @Audiodev
> +#
> +# Since: 6.0
> +#
> +##
> +{ 'command': 'query-audiodevs',
> +  'returns': ['Audiodev'] }