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

[Daniel P . Berrangé]

On Wed, Jan 25, 2023 at 12:06:40PM +0100, Thomas Huth wrote:
> On 23/01/2023 13.09, Daniel P. Berrangé wrote:
> > On Mon, Jan 23, 2023 at 01:05:45PM +0100, Philippe Mathieu-Daudé wrote:
> > > On 23/1/23 12:11, Daniel P. Berrangé wrote:
> > > > On Mon, Jan 23, 2023 at 10:20:29AM +0100, Philippe Mathieu-Daudé wrote:
> > > > > 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'.
> > > > 
> > > > The command line parameter is called '-audiodev' and this
> > > > query-audiodevs command reports the same data, so that
> > > > looks easy enough to understand IMHO.
> 
> Should we maybe use a "x-" prefix here if we feel not certain enough about
> this command yet? ... that would still enable the Audiodev part in the qapi
> schema, but give us the flexibility to rename or drop it later in case there
> is some better way to enable the Audiodevs in the schema?

IMHO there's no reason to add an 'x-' prefix. Even if we found a better
way to detect existance of Audiodev backend types, I think query-audiodev
is still conceptually a useful command for interogating QEMU's config.
To get to our goal of a 100% QMP based replacement for the CLI, we want
to have QMP commands for adding, removing and querying every backend
config (audiodev, device, object, chardev, netdev, etc). This command
addresses one of those gaps for audiodevs

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|