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

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 23/1/23 09:39, Thomas Huth wrote:
> From: Daniel P. Berrangé <berrange@redhat.com>
> 
> 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 introduces a 'query-audiodev' command that simply reflects back
> the list of configured -audiodev command line options. This alone is
> maybe not very useful by itself, but it makes Audiodev introspectable
> via 'query-qmp-schema', so that libvirt (and other upper layer tools)
> can discover the available audiodevs.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> [thuth: Update for upcoming QEMU v8.0, and use QAPI_LIST_PREPEND]
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   qapi/audio.json | 13 +++++++++++++
>   audio/audio.c   | 12 ++++++++++++
>   2 files changed, 25 insertions(+)
> 
> diff --git a/qapi/audio.json b/qapi/audio.json
> index 1e0a24bdfc..c7aafa2763 100644
> --- a/qapi/audio.json
> +++ b/qapi/audio.json
> @@ -443,3 +443,16 @@
>       'sndio':     'AudiodevSndioOptions',
>       'spice':     'AudiodevGenericOptions',
>       'wav':       'AudiodevWavOptions' } }
> +
> +##
> +# @query-audiodevs:
> +#
> +# Returns information about audiodev configuration

Maybe clearer as 'audio backends'?

So similarly, wouldn't be clearer to name this command
'query-audio-backends'? Otherwise we need to go read QEMU
source to understand what is 'audiodevs'.

> +#
> +# Returns: array of @Audiodev
> +#
> +# Since: 8.0
> +#
> +##
> +{ 'command': 'query-audiodevs',
> +  'returns': ['Audiodev'] }
> diff --git a/audio/audio.c b/audio/audio.c
> index d849a94a81..6f270c07b7 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 "qemu/help_option.h"
> @@ -2311,3 +2313,13 @@ size_t audio_rate_get_bytes(RateCtl *rate, struct audio_pcm_info *info,
>   
>       return bytes;
>   }
> +
> +AudiodevList *qmp_query_audiodevs(Error **errp)
> +{
> +    AudiodevList *ret = NULL;
> +    AudiodevListEntry *e;
> +    QSIMPLEQ_FOREACH(e, &audiodevs, next) {

I am a bit confused here, isn't &audiodevs containing what the user 
provided from CLI? How is that useful to libvirt? Maybe the corner case
of a user hand-modifying the QEMU launch arguments from a XML config?

Wouldn't a list of linked in AudiodevDriver be more useful to libvirt
so it could pick the best available backend to start a VM?

> +        QAPI_LIST_PREPEND(ret, QAPI_CLONE(Audiodev, e->dev));
> +    }
> +    return ret;
> +}