* [PATCH v2 0/2] audio: make audiodev introspectable by management apps
@ 2023-01-23 8:39 Thomas Huth
2023-01-23 8:39 ` [PATCH v2 1/2] qapi, audio: add query-audiodev command Thomas Huth
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Thomas Huth @ 2023-01-23 8:39 UTC (permalink / raw)
To: Markus Armbruster, Eric Blake, qemu-devel, Gerd Hoffmann,
Daniel P . Berrangé
Cc: Volker Rümelin, Christian Schoenebeck
Here's a respin from Daniel's audiodev introspection patches from
2021. I've rebased them to the current master branch and addressed
the review comments from v1.
The Audiodev QAPI type is not introspectable via query-qmp-schema as
nothing in QMP uses it. "-audiodev" is not introspectable via
query-command-line-options because it avoided legacy QemuOpts.
To fix it, introduce a tiny "query-audiodev" QMP command that uses
the "Audiodev" QAPI structure, so that it shows up in the schema.
Then mark the various backend types with conditionals so that only
the ones that were available at compile time show up in the schema.
Daniel P. Berrangé (2):
qapi, audio: add query-audiodev command
qapi, audio: Make introspection reflect build configuration more
closely
qapi/audio.json | 57 +++++++++++++++++++++++++++++++++---------
audio/audio_template.h | 20 +++++++++++++++
audio/audio.c | 32 ++++++++++++++++++++++++
audio/audio_legacy.c | 41 +++++++++++++++++++++++++++++-
4 files changed, 137 insertions(+), 13 deletions(-)
--
2.31.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 1/2] qapi, audio: add query-audiodev command
2023-01-23 8:39 [PATCH v2 0/2] audio: make audiodev introspectable by management apps Thomas Huth
@ 2023-01-23 8:39 ` Thomas Huth
2023-01-23 9:20 ` Philippe Mathieu-Daudé
2023-01-23 8:39 ` [PATCH v2 2/2] qapi, audio: Make introspection reflect build configuration more closely Thomas Huth
2023-01-31 10:09 ` [PATCH v2 0/2] audio: make audiodev introspectable by management apps Thomas Huth
2 siblings, 1 reply; 11+ messages in thread
From: Thomas Huth @ 2023-01-23 8:39 UTC (permalink / raw)
To: Markus Armbruster, Eric Blake, qemu-devel, Gerd Hoffmann,
Daniel P . Berrangé
Cc: Volker Rümelin, Christian Schoenebeck
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
+#
+# 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) {
+ QAPI_LIST_PREPEND(ret, QAPI_CLONE(Audiodev, e->dev));
+ }
+ return ret;
+}
--
2.31.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 2/2] qapi, audio: Make introspection reflect build configuration more closely
2023-01-23 8:39 [PATCH v2 0/2] audio: make audiodev introspectable by management apps Thomas Huth
2023-01-23 8:39 ` [PATCH v2 1/2] qapi, audio: add query-audiodev command Thomas Huth
@ 2023-01-23 8:39 ` Thomas Huth
2023-01-31 10:09 ` [PATCH v2 0/2] audio: make audiodev introspectable by management apps Thomas Huth
2 siblings, 0 replies; 11+ messages in thread
From: Thomas Huth @ 2023-01-23 8:39 UTC (permalink / raw)
To: Markus Armbruster, Eric Blake, qemu-devel, Gerd Hoffmann,
Daniel P . Berrangé
Cc: Volker Rümelin, Christian Schoenebeck
From: Daniel P. Berrangé <berrange@redhat.com>
Currently the -audiodev accepts any audiodev type regardless of what is
built in to QEMU. An error only occurs later at runtime when a sound
device tries to use the audio backend.
With this change QEMU will immediately reject -audiodev args that are
not compiled into the binary. The QMP schema will also be introspectable
to identify what is compiled in.
This also helps to avoid compiling code that is not required in the
binary. Note: When building the audiodevs as modules, the patch only
compiles out code for modules that we don't build at all.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
[thuth: Rebase, take sndio and dbus devices into account]
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
qapi/audio.json | 44 ++++++++++++++++++++++++++++++------------
audio/audio_template.h | 20 +++++++++++++++++++
audio/audio.c | 20 +++++++++++++++++++
audio/audio_legacy.c | 41 ++++++++++++++++++++++++++++++++++++++-
4 files changed, 112 insertions(+), 13 deletions(-)
diff --git a/qapi/audio.json b/qapi/audio.json
index c7aafa2763..4e54c00f51 100644
--- a/qapi/audio.json
+++ b/qapi/audio.json
@@ -408,8 +408,18 @@
# Since: 4.0
##
{ 'enum': 'AudiodevDriver',
- 'data': [ 'none', 'alsa', 'coreaudio', 'dbus', 'dsound', 'jack', 'oss', 'pa',
- 'sdl', 'sndio', 'spice', 'wav' ] }
+ 'data': [ 'none',
+ { 'name': 'alsa', 'if': 'CONFIG_AUDIO_ALSA' },
+ { 'name': 'coreaudio', 'if': 'CONFIG_AUDIO_COREAUDIO' },
+ { 'name': 'dbus', 'if': 'CONFIG_DBUS_DISPLAY' },
+ { 'name': 'dsound', 'if': 'CONFIG_AUDIO_DSOUND' },
+ { 'name': 'jack', 'if': 'CONFIG_AUDIO_JACK' },
+ { 'name': 'oss', 'if': 'CONFIG_AUDIO_OSS' },
+ { 'name': 'pa', 'if': 'CONFIG_AUDIO_PA' },
+ { 'name': 'sdl', 'if': 'CONFIG_AUDIO_SDL' },
+ { 'name': 'sndio', 'if': 'CONFIG_AUDIO_SNDIO' },
+ { 'name': 'spice', 'if': 'CONFIG_SPICE' },
+ 'wav' ] }
##
# @Audiodev:
@@ -432,16 +442,26 @@
'discriminator': 'driver',
'data': {
'none': 'AudiodevGenericOptions',
- 'alsa': 'AudiodevAlsaOptions',
- 'coreaudio': 'AudiodevCoreaudioOptions',
- 'dbus': 'AudiodevGenericOptions',
- 'dsound': 'AudiodevDsoundOptions',
- 'jack': 'AudiodevJackOptions',
- 'oss': 'AudiodevOssOptions',
- 'pa': 'AudiodevPaOptions',
- 'sdl': 'AudiodevSdlOptions',
- 'sndio': 'AudiodevSndioOptions',
- 'spice': 'AudiodevGenericOptions',
+ 'alsa': { 'type': 'AudiodevAlsaOptions',
+ 'if': 'CONFIG_AUDIO_ALSA' },
+ 'coreaudio': { 'type': 'AudiodevCoreaudioOptions',
+ 'if': 'CONFIG_AUDIO_COREAUDIO' },
+ 'dbus': { 'type': 'AudiodevGenericOptions',
+ 'if': 'CONFIG_DBUS_DISPLAY' },
+ 'dsound': { 'type': 'AudiodevDsoundOptions',
+ 'if': 'CONFIG_AUDIO_DSOUND' },
+ 'jack': { 'type': 'AudiodevJackOptions',
+ 'if': 'CONFIG_AUDIO_JACK' },
+ 'oss': { 'type': 'AudiodevOssOptions',
+ 'if': 'CONFIG_AUDIO_OSS' },
+ 'pa': { 'type': 'AudiodevPaOptions',
+ 'if': 'CONFIG_AUDIO_PA' },
+ 'sdl': { 'type': 'AudiodevSdlOptions',
+ 'if': 'CONFIG_AUDIO_SDL' },
+ 'sndio': { 'type': 'AudiodevSndioOptions',
+ 'if': 'CONFIG_AUDIO_SNDIO' },
+ 'spice': { 'type': 'AudiodevGenericOptions',
+ 'if': 'CONFIG_SPICE' },
'wav': 'AudiodevWavOptions' } }
##
diff --git a/audio/audio_template.h b/audio/audio_template.h
index 720a32e57e..42b4712acb 100644
--- a/audio/audio_template.h
+++ b/audio/audio_template.h
@@ -326,27 +326,47 @@ AudiodevPerDirectionOptions *glue(audio_get_pdo_, TYPE)(Audiodev *dev)
switch (dev->driver) {
case AUDIODEV_DRIVER_NONE:
return dev->u.none.TYPE;
+#ifdef CONFIG_AUDIO_ALSA
case AUDIODEV_DRIVER_ALSA:
return qapi_AudiodevAlsaPerDirectionOptions_base(dev->u.alsa.TYPE);
+#endif
+#ifdef CONFIG_AUDIO_COREAUDIO
case AUDIODEV_DRIVER_COREAUDIO:
return qapi_AudiodevCoreaudioPerDirectionOptions_base(
dev->u.coreaudio.TYPE);
+#endif
+#ifdef CONFIG_DBUS_DISPLAY
case AUDIODEV_DRIVER_DBUS:
return dev->u.dbus.TYPE;
+#endif
+#ifdef CONFIG_AUDIO_DSOUND
case AUDIODEV_DRIVER_DSOUND:
return dev->u.dsound.TYPE;
+#endif
+#ifdef CONFIG_AUDIO_JACK
case AUDIODEV_DRIVER_JACK:
return qapi_AudiodevJackPerDirectionOptions_base(dev->u.jack.TYPE);
+#endif
+#ifdef CONFIG_AUDIO_OSS
case AUDIODEV_DRIVER_OSS:
return qapi_AudiodevOssPerDirectionOptions_base(dev->u.oss.TYPE);
+#endif
+#ifdef CONFIG_AUDIO_PA
case AUDIODEV_DRIVER_PA:
return qapi_AudiodevPaPerDirectionOptions_base(dev->u.pa.TYPE);
+#endif
+#ifdef CONFIG_AUDIO_SDL
case AUDIODEV_DRIVER_SDL:
return qapi_AudiodevSdlPerDirectionOptions_base(dev->u.sdl.TYPE);
+#endif
+#ifdef CONFIG_AUDIO_SNDIO
case AUDIODEV_DRIVER_SNDIO:
return dev->u.sndio.TYPE;
+#endif
+#ifdef CONFIG_SPICE
case AUDIODEV_DRIVER_SPICE:
return dev->u.spice.TYPE;
+#endif
case AUDIODEV_DRIVER_WAV:
return dev->u.wav.TYPE;
diff --git a/audio/audio.c b/audio/audio.c
index 6f270c07b7..4290309d18 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -2048,16 +2048,36 @@ void audio_create_pdos(Audiodev *dev)
break
CASE(NONE, none, );
+#ifdef CONFIG_AUDIO_ALSA
CASE(ALSA, alsa, Alsa);
+#endif
+#ifdef CONFIG_AUDIO_COREAUDIO
CASE(COREAUDIO, coreaudio, Coreaudio);
+#endif
+#ifdef CONFIG_DBUS_DISPLAY
CASE(DBUS, dbus, );
+#endif
+#ifdef CONFIG_AUDIO_DSOUND
CASE(DSOUND, dsound, );
+#endif
+#ifdef CONFIG_AUDIO_JACK
CASE(JACK, jack, Jack);
+#endif
+#ifdef CONFIG_AUDIO_OSS
CASE(OSS, oss, Oss);
+#endif
+#ifdef CONFIG_AUDIO_PA
CASE(PA, pa, Pa);
+#endif
+#ifdef CONFIG_AUDIO_SDL
CASE(SDL, sdl, Sdl);
+#endif
+#ifdef CONFIG_AUDIO_SNDIO
CASE(SNDIO, sndio, );
+#endif
+#ifdef CONFIG_SPICE
CASE(SPICE, spice, );
+#endif
CASE(WAV, wav, );
case AUDIODEV_DRIVER__MAX:
diff --git a/audio/audio_legacy.c b/audio/audio_legacy.c
index 18a89ffffb..b848001ff7 100644
--- a/audio/audio_legacy.c
+++ b/audio/audio_legacy.c
@@ -90,6 +90,7 @@ static void get_fmt(const char *env, AudioFormat *dst, bool *has_dst)
}
+#if defined(CONFIG_AUDIO_ALSA) || defined(CONFIG_AUDIO_DSOUND)
static void get_millis_to_usecs(const char *env, uint32_t *dst, bool *has_dst)
{
const char *val = getenv(env);
@@ -98,15 +99,20 @@ static void get_millis_to_usecs(const char *env, uint32_t *dst, bool *has_dst)
*has_dst = true;
}
}
+#endif
+#if defined(CONFIG_AUDIO_ALSA) || defined(CONFIG_AUDIO_COREAUDIO) || \
+ defined(CONFIG_AUDIO_PA) || defined(CONFIG_AUDIO_SDL) || \
+ defined(CONFIG_AUDIO_DSOUND) || defined(CONFIG_AUDIO_OSS)
static uint32_t frames_to_usecs(uint32_t frames,
AudiodevPerDirectionOptions *pdo)
{
uint32_t freq = pdo->has_frequency ? pdo->frequency : 44100;
return (frames * 1000000 + freq / 2) / freq;
}
+#endif
-
+#ifdef CONFIG_AUDIO_COREAUDIO
static void get_frames_to_usecs(const char *env, uint32_t *dst, bool *has_dst,
AudiodevPerDirectionOptions *pdo)
{
@@ -116,14 +122,19 @@ static void get_frames_to_usecs(const char *env, uint32_t *dst, bool *has_dst,
*has_dst = true;
}
}
+#endif
+#if defined(CONFIG_AUDIO_PA) || defined(CONFIG_AUDIO_SDL) || \
+ defined(CONFIG_AUDIO_DSOUND) || defined(CONFIG_AUDIO_OSS)
static uint32_t samples_to_usecs(uint32_t samples,
AudiodevPerDirectionOptions *pdo)
{
uint32_t channels = pdo->has_channels ? pdo->channels : 2;
return frames_to_usecs(samples / channels, pdo);
}
+#endif
+#if defined(CONFIG_AUDIO_PA) || defined(CONFIG_AUDIO_SDL)
static void get_samples_to_usecs(const char *env, uint32_t *dst, bool *has_dst,
AudiodevPerDirectionOptions *pdo)
{
@@ -133,7 +144,9 @@ static void get_samples_to_usecs(const char *env, uint32_t *dst, bool *has_dst,
*has_dst = true;
}
}
+#endif
+#if defined(CONFIG_AUDIO_DSOUND) || defined(CONFIG_AUDIO_OSS)
static uint32_t bytes_to_usecs(uint32_t bytes, AudiodevPerDirectionOptions *pdo)
{
AudioFormat fmt = pdo->has_format ? pdo->format : AUDIO_FORMAT_S16;
@@ -150,8 +163,11 @@ static void get_bytes_to_usecs(const char *env, uint32_t *dst, bool *has_dst,
*has_dst = true;
}
}
+#endif
/* backend specific functions */
+
+#ifdef CONFIG_AUDIO_ALSA
/* ALSA */
static void handle_alsa_per_direction(
AudiodevAlsaPerDirectionOptions *apdo, const char *prefix)
@@ -197,7 +213,9 @@ static void handle_alsa(Audiodev *dev)
get_millis_to_usecs("QEMU_ALSA_THRESHOLD",
&aopt->threshold, &aopt->has_threshold);
}
+#endif
+#ifdef CONFIG_AUDIO_COREAUDIO
/* coreaudio */
static void handle_coreaudio(Audiodev *dev)
{
@@ -210,7 +228,9 @@ static void handle_coreaudio(Audiodev *dev)
&dev->u.coreaudio.out->buffer_count,
&dev->u.coreaudio.out->has_buffer_count);
}
+#endif
+#ifdef CONFIG_AUDIO_DSOUND
/* dsound */
static void handle_dsound(Audiodev *dev)
{
@@ -225,7 +245,9 @@ static void handle_dsound(Audiodev *dev)
&dev->u.dsound.in->has_buffer_length,
dev->u.dsound.in);
}
+#endif
+#ifdef CONFIG_AUDIO_OSS
/* OSS */
static void handle_oss_per_direction(
AudiodevOssPerDirectionOptions *opdo, const char *try_poll_env,
@@ -253,7 +275,9 @@ static void handle_oss(Audiodev *dev)
get_bool("QEMU_OSS_EXCLUSIVE", &oopt->exclusive, &oopt->has_exclusive);
get_int("QEMU_OSS_POLICY", &oopt->dsp_policy, &oopt->has_dsp_policy);
}
+#endif
+#ifdef CONFIG_AUDIO_PA
/* pulseaudio */
static void handle_pa_per_direction(
AudiodevPaPerDirectionOptions *ppdo, const char *env)
@@ -277,7 +301,9 @@ static void handle_pa(Audiodev *dev)
get_str("QEMU_PA_SERVER", &dev->u.pa.server);
}
+#endif
+#ifdef CONFIG_AUDIO_SDL
/* SDL */
static void handle_sdl(Audiodev *dev)
{
@@ -286,6 +312,7 @@ static void handle_sdl(Audiodev *dev)
&dev->u.sdl.out->has_buffer_length,
qapi_AudiodevSdlPerDirectionOptions_base(dev->u.sdl.out));
}
+#endif
/* wav */
static void handle_wav(Audiodev *dev)
@@ -345,29 +372,41 @@ static AudiodevListEntry *legacy_opt(const char *drvname)
}
switch (e->dev->driver) {
+#ifdef CONFIG_AUDIO_ALSA
case AUDIODEV_DRIVER_ALSA:
handle_alsa(e->dev);
break;
+#endif
+#ifdef CONFIG_AUDIO_COREAUDIO
case AUDIODEV_DRIVER_COREAUDIO:
handle_coreaudio(e->dev);
break;
+#endif
+#ifdef CONFIG_AUDIO_DSOUND
case AUDIODEV_DRIVER_DSOUND:
handle_dsound(e->dev);
break;
+#endif
+#ifdef CONFIG_AUDIO_OSS
case AUDIODEV_DRIVER_OSS:
handle_oss(e->dev);
break;
+#endif
+#ifdef CONFIG_AUDIO_PA
case AUDIODEV_DRIVER_PA:
handle_pa(e->dev);
break;
+#endif
+#ifdef CONFIG_AUDIO_SDL
case AUDIODEV_DRIVER_SDL:
handle_sdl(e->dev);
break;
+#endif
case AUDIODEV_DRIVER_WAV:
handle_wav(e->dev);
--
2.31.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/2] qapi, audio: add query-audiodev command
2023-01-23 8:39 ` [PATCH v2 1/2] qapi, audio: add query-audiodev command Thomas Huth
@ 2023-01-23 9:20 ` Philippe Mathieu-Daudé
2023-01-23 11:11 ` Daniel P. Berrangé
0 siblings, 1 reply; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-01-23 9:20 UTC (permalink / raw)
To: Thomas Huth, Markus Armbruster, Eric Blake, qemu-devel,
Gerd Hoffmann, Daniel P . Berrangé
Cc: Volker Rümelin, Christian Schoenebeck
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;
> +}
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/2] qapi, audio: add query-audiodev command
2023-01-23 9:20 ` Philippe Mathieu-Daudé
@ 2023-01-23 11:11 ` Daniel P. Berrangé
2023-01-23 12:05 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 11+ messages in thread
From: Daniel P. Berrangé @ 2023-01-23 11:11 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Thomas Huth, Markus Armbruster, Eric Blake, qemu-devel,
Gerd Hoffmann, Volker Rümelin, Christian Schoenebeck
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.
> > +#
> > +# 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?
On the libvirt side we're never going to need to actually call the
query-audiodevs commands. The mere existance of the command, means
that the QMP schema now exposes information about what audio backends
have been compiled into the binary. This is the same trick we've used
for other aspects of QMP. IOW we don't need a separate command just
for the purpose of listing AudiodevDrivers.
The idea of a query-audiodevs command is useful in general. When we
later gain support for hotplug/unplug of audio, the set of audiodevs
will no longer be guaranteed to match the original CLI args.
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 :|
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/2] qapi, audio: add query-audiodev command
2023-01-23 11:11 ` Daniel P. Berrangé
@ 2023-01-23 12:05 ` Philippe Mathieu-Daudé
2023-01-23 12:09 ` Daniel P. Berrangé
0 siblings, 1 reply; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-01-23 12:05 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Thomas Huth, Markus Armbruster, Eric Blake, qemu-devel,
Gerd Hoffmann, Volker Rümelin, Christian Schoenebeck
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.
>
>>> +#
>>> +# 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?
>
> On the libvirt side we're never going to need to actually call the
> query-audiodevs commands. The mere existance of the command, means
> that the QMP schema now exposes information about what audio backends
> have been compiled into the binary. This is the same trick we've used
> for other aspects of QMP. IOW we don't need a separate command just
> for the purpose of listing AudiodevDrivers.
I understand having "what audio backends have been compiled into the
binary" is useful, but I am missing how you get that from &audiodevs.
AFAICT &audiodevs is for the CLI parsed backends, not all the backends
linked within a binary. I probably need sugar / coffee and will revisit
after lunch.
> The idea of a query-audiodevs command is useful in general. When we
> later gain support for hotplug/unplug of audio, the set of audiodevs
> will no longer be guaranteed to match the original CLI args.
>
> With regards,
> Daniel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/2] qapi, audio: add query-audiodev command
2023-01-23 12:05 ` Philippe Mathieu-Daudé
@ 2023-01-23 12:09 ` Daniel P. Berrangé
2023-01-25 11:06 ` Thomas Huth
2023-01-25 12:04 ` Philippe Mathieu-Daudé
0 siblings, 2 replies; 11+ messages in thread
From: Daniel P. Berrangé @ 2023-01-23 12:09 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Thomas Huth, Markus Armbruster, Eric Blake, qemu-devel,
Gerd Hoffmann, Volker Rümelin, Christian Schoenebeck
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.
> >
> > > > +#
> > > > +# 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?
> >
> > On the libvirt side we're never going to need to actually call the
> > query-audiodevs commands. The mere existance of the command, means
> > that the QMP schema now exposes information about what audio backends
> > have been compiled into the binary. This is the same trick we've used
> > for other aspects of QMP. IOW we don't need a separate command just
> > for the purpose of listing AudiodevDrivers.
>
> I understand having "what audio backends have been compiled into the
> binary" is useful, but I am missing how you get that from &audiodevs.
>
> AFAICT &audiodevs is for the CLI parsed backends, not all the backends
> linked within a binary. I probably need sugar / coffee and will revisit
> after lunch.
It ties into the 'query-qmp-schema' command, along with patch #2 in
this series.
The query-audiodevs command will cause the 'AudiodevDriver' enum to
be reported in the 'query-qmp-schema' response. Patch #2, makes all
the AudiodevDriver enum entries conditional on CONFIG_XXXX.
IOW now query-qmp-schema data will tell you what AudiodevDriver
backends are compiled into the binary you're talking to.
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 :|
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/2] qapi, audio: add query-audiodev command
2023-01-23 12:09 ` Daniel P. Berrangé
@ 2023-01-25 11:06 ` Thomas Huth
2023-01-25 12:06 ` Daniel P. Berrangé
2023-01-25 12:04 ` Philippe Mathieu-Daudé
1 sibling, 1 reply; 11+ messages in thread
From: Thomas Huth @ 2023-01-25 11:06 UTC (permalink / raw)
To: Daniel P. Berrangé, Philippe Mathieu-Daudé
Cc: Markus Armbruster, Eric Blake, qemu-devel, Gerd Hoffmann,
Volker Rümelin, Christian Schoenebeck
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?
Thomas
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/2] qapi, audio: add query-audiodev command
2023-01-23 12:09 ` Daniel P. Berrangé
2023-01-25 11:06 ` Thomas Huth
@ 2023-01-25 12:04 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-01-25 12:04 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Thomas Huth, Markus Armbruster, Eric Blake, qemu-devel,
Gerd Hoffmann, Volker Rümelin, Christian Schoenebeck
On 23/1/23 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>
>>>>> +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?
>>>
>>> On the libvirt side we're never going to need to actually call the
>>> query-audiodevs commands. The mere existance of the command, means
>>> that the QMP schema now exposes information about what audio backends
>>> have been compiled into the binary. This is the same trick we've used
>>> for other aspects of QMP. IOW we don't need a separate command just
>>> for the purpose of listing AudiodevDrivers.
>>
>> I understand having "what audio backends have been compiled into the
>> binary" is useful, but I am missing how you get that from &audiodevs.
>>
>> AFAICT &audiodevs is for the CLI parsed backends, not all the backends
>> linked within a binary. I probably need sugar / coffee and will revisit
>> after lunch.
>
> It ties into the 'query-qmp-schema' command, along with patch #2 in
> this series.
>
> The query-audiodevs command will cause the 'AudiodevDriver' enum to
> be reported in the 'query-qmp-schema' response. Patch #2, makes all
> the AudiodevDriver enum entries conditional on CONFIG_XXXX.
>
> IOW now query-qmp-schema data will tell you what AudiodevDriver
> backends are compiled into the binary you're talking to.
Thanks for the explanation Daniel.
Just FTR, while I'm not confident enough to add a R-b tag, I am
not opposed to it.
Regards,
Phil.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/2] qapi, audio: add query-audiodev command
2023-01-25 11:06 ` Thomas Huth
@ 2023-01-25 12:06 ` Daniel P. Berrangé
0 siblings, 0 replies; 11+ messages in thread
From: Daniel P. Berrangé @ 2023-01-25 12:06 UTC (permalink / raw)
To: Thomas Huth
Cc: Philippe Mathieu-Daudé, Markus Armbruster, Eric Blake,
qemu-devel, Gerd Hoffmann, Volker Rümelin,
Christian Schoenebeck
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 :|
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 0/2] audio: make audiodev introspectable by management apps
2023-01-23 8:39 [PATCH v2 0/2] audio: make audiodev introspectable by management apps Thomas Huth
2023-01-23 8:39 ` [PATCH v2 1/2] qapi, audio: add query-audiodev command Thomas Huth
2023-01-23 8:39 ` [PATCH v2 2/2] qapi, audio: Make introspection reflect build configuration more closely Thomas Huth
@ 2023-01-31 10:09 ` Thomas Huth
2 siblings, 0 replies; 11+ messages in thread
From: Thomas Huth @ 2023-01-31 10:09 UTC (permalink / raw)
To: Markus Armbruster, Eric Blake, qemu-devel, Gerd Hoffmann,
Daniel P . Berrangé
Cc: Volker Rümelin, Christian Schoenebeck
On 23/01/2023 09.39, Thomas Huth wrote:
> Here's a respin from Daniel's audiodev introspection patches from
> 2021. I've rebased them to the current master branch and addressed
> the review comments from v1.
>
> The Audiodev QAPI type is not introspectable via query-qmp-schema as
> nothing in QMP uses it. "-audiodev" is not introspectable via
> query-command-line-options because it avoided legacy QemuOpts.
>
> To fix it, introduce a tiny "query-audiodev" QMP command that uses
> the "Audiodev" QAPI structure, so that it shows up in the schema.
> Then mark the various backend types with conditionals so that only
> the ones that were available at compile time show up in the schema.
>
> Daniel P. Berrangé (2):
> qapi, audio: add query-audiodev command
> qapi, audio: Make introspection reflect build configuration more
> closely
Since there were no objections and since Gerd seems to be pretty busy with
other stuff recently, I'll dare to pick these up for my next pull request.
Thomas
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-01-31 10:10 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-23 8:39 [PATCH v2 0/2] audio: make audiodev introspectable by management apps Thomas Huth
2023-01-23 8:39 ` [PATCH v2 1/2] qapi, audio: add query-audiodev command Thomas Huth
2023-01-23 9:20 ` Philippe Mathieu-Daudé
2023-01-23 11:11 ` Daniel P. Berrangé
2023-01-23 12:05 ` Philippe Mathieu-Daudé
2023-01-23 12:09 ` Daniel P. Berrangé
2023-01-25 11:06 ` Thomas Huth
2023-01-25 12:06 ` Daniel P. Berrangé
2023-01-25 12:04 ` Philippe Mathieu-Daudé
2023-01-23 8:39 ` [PATCH v2 2/2] qapi, audio: Make introspection reflect build configuration more closely Thomas Huth
2023-01-31 10:09 ` [PATCH v2 0/2] audio: make audiodev introspectable by management apps Thomas Huth
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).