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.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
audio/audio.c | 16 +++++++++++++++
audio/audio_legacy.c | 41 ++++++++++++++++++++++++++++++++++++++-
audio/audio_template.h | 16 +++++++++++++++
qapi/audio.json | 44 ++++++++++++++++++++++++++++++++----------
4 files changed, 106 insertions(+), 11 deletions(-)
diff --git a/audio/audio.c b/audio/audio.c
index 40a4bbd7ce..53434fc674 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1989,14 +1989,30 @@ 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_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_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 0fe827b057..bb2268f2b2 100644
--- a/audio/audio_legacy.c
+++ b/audio/audio_legacy.c
@@ -93,6 +93,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);
@@ -101,15 +102,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)
{
@@ -119,14 +125,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)
{
@@ -136,7 +147,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;
@@ -153,8 +166,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)
@@ -200,7 +216,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)
{
@@ -213,7 +231,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)
{
@@ -228,7 +248,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,
@@ -256,7 +278,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)
@@ -280,7 +304,9 @@ static void handle_pa(Audiodev *dev)
get_str("QEMU_PA_SERVER", &dev->u.pa.server, &dev->u.pa.has_server);
}
+#endif
+#ifdef CONFIG_AUDIO_SDL
/* SDL */
static void handle_sdl(Audiodev *dev)
{
@@ -289,6 +315,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)
@@ -348,29 +375,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);
diff --git a/audio/audio_template.h b/audio/audio_template.h
index c6714946aa..0847b643be 100644
--- a/audio/audio_template.h
+++ b/audio/audio_template.h
@@ -322,23 +322,39 @@ 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_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_SPICE
case AUDIODEV_DRIVER_SPICE:
return dev->u.spice.TYPE;
+#endif
case AUDIODEV_DRIVER_WAV:
return dev->u.wav.TYPE;
diff --git a/qapi/audio.json b/qapi/audio.json
index d7b91230d7..9af1b8140c 100644
--- a/qapi/audio.json
+++ b/qapi/audio.json
@@ -386,8 +386,24 @@
# Since: 4.0
##
{ 'enum': 'AudiodevDriver',
- 'data': [ 'none', 'alsa', 'coreaudio', 'dsound', 'jack', 'oss', 'pa',
- 'sdl', 'spice', 'wav' ] }
+ 'data': [ 'none',
+ { 'name': 'alsa',
+ 'if': 'defined(CONFIG_AUDIO_ALSA)' },
+ { 'name': 'coreaudio',
+ 'if': 'defined(CONFIG_AUDIO_COREAUDIO)' },
+ { 'name': 'dsound',
+ 'if': 'defined(CONFIG_AUDIO_DSOUND)' },
+ { 'name': 'jack',
+ 'if': 'defined(CONFIG_AUDIO_JACK)' },
+ { 'name': 'oss',
+ 'if': 'defined(CONFIG_AUDIO_OSS)' },
+ { 'name': 'pa',
+ 'if': 'defined(CONFIG_AUDIO_PA)' },
+ { 'name': 'sdl',
+ 'if': 'defined(CONFIG_AUDIO_SDL)' },
+ { 'name': 'spice',
+ 'if': 'defined(CONFIG_SPICE)' },
+ 'wav' ] }
##
# @Audiodev:
@@ -410,14 +426,22 @@
'discriminator': 'driver',
'data': {
'none': 'AudiodevGenericOptions',
- 'alsa': 'AudiodevAlsaOptions',
- 'coreaudio': 'AudiodevCoreaudioOptions',
- 'dsound': 'AudiodevDsoundOptions',
- 'jack': 'AudiodevJackOptions',
- 'oss': 'AudiodevOssOptions',
- 'pa': 'AudiodevPaOptions',
- 'sdl': 'AudiodevSdlOptions',
- 'spice': 'AudiodevGenericOptions',
+ 'alsa': { 'type': 'AudiodevAlsaOptions',
+ 'if': 'defined(CONFIG_AUDIO_ALSA)' },
+ 'coreaudio': { 'type': 'AudiodevCoreaudioOptions',
+ 'if': 'defined(CONFIG_AUDIO_COREAUDIO)' },
+ 'dsound': { 'type': 'AudiodevDsoundOptions',
+ 'if': 'defined(CONFIG_AUDIO_DSOUND)' },
+ 'jack': { 'type': 'AudiodevJackOptions',
+ 'if': 'defined(CONFIG_AUDIO_JACK)' },
+ 'oss': { 'type': 'AudiodevOssOptions',
+ 'if': 'defined(CONFIG_AUDIO_OSS)' },
+ 'pa': { 'type': 'AudiodevPaOptions',
+ 'if': 'defined(CONFIG_AUDIO_PA)' },
+ 'sdl': { 'type': 'AudiodevSdlOptions',
+ 'if': 'defined(CONFIG_AUDIO_SDL)' },
+ 'spice': { 'type': 'AudiodevGenericOptions',
+ 'if': 'defined(CONFIG_SPICE)' },
'wav': 'AudiodevWavOptions' } }
##
--
2.29.2
On 02/03/2021 18.55, Daniel P. Berrangé wrote:
> 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.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> audio/audio.c | 16 +++++++++++++++
> audio/audio_legacy.c | 41 ++++++++++++++++++++++++++++++++++++++-
> audio/audio_template.h | 16 +++++++++++++++
> qapi/audio.json | 44 ++++++++++++++++++++++++++++++++----------
> 4 files changed, 106 insertions(+), 11 deletions(-)
Hi Daniel!
Would you have time to respin this patch for QEMU 8.0 ?
Thomas
> diff --git a/audio/audio.c b/audio/audio.c
> index 40a4bbd7ce..53434fc674 100644
> --- a/audio/audio.c
> +++ b/audio/audio.c
> @@ -1989,14 +1989,30 @@ 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_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_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 0fe827b057..bb2268f2b2 100644
> --- a/audio/audio_legacy.c
> +++ b/audio/audio_legacy.c
> @@ -93,6 +93,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);
> @@ -101,15 +102,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)
> {
> @@ -119,14 +125,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)
> {
> @@ -136,7 +147,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;
> @@ -153,8 +166,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)
> @@ -200,7 +216,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)
> {
> @@ -213,7 +231,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)
> {
> @@ -228,7 +248,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,
> @@ -256,7 +278,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)
> @@ -280,7 +304,9 @@ static void handle_pa(Audiodev *dev)
>
> get_str("QEMU_PA_SERVER", &dev->u.pa.server, &dev->u.pa.has_server);
> }
> +#endif
>
> +#ifdef CONFIG_AUDIO_SDL
> /* SDL */
> static void handle_sdl(Audiodev *dev)
> {
> @@ -289,6 +315,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)
> @@ -348,29 +375,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);
> diff --git a/audio/audio_template.h b/audio/audio_template.h
> index c6714946aa..0847b643be 100644
> --- a/audio/audio_template.h
> +++ b/audio/audio_template.h
> @@ -322,23 +322,39 @@ 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_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_SPICE
> case AUDIODEV_DRIVER_SPICE:
> return dev->u.spice.TYPE;
> +#endif
> case AUDIODEV_DRIVER_WAV:
> return dev->u.wav.TYPE;
>
> diff --git a/qapi/audio.json b/qapi/audio.json
> index d7b91230d7..9af1b8140c 100644
> --- a/qapi/audio.json
> +++ b/qapi/audio.json
> @@ -386,8 +386,24 @@
> # Since: 4.0
> ##
> { 'enum': 'AudiodevDriver',
> - 'data': [ 'none', 'alsa', 'coreaudio', 'dsound', 'jack', 'oss', 'pa',
> - 'sdl', 'spice', 'wav' ] }
> + 'data': [ 'none',
> + { 'name': 'alsa',
> + 'if': 'defined(CONFIG_AUDIO_ALSA)' },
> + { 'name': 'coreaudio',
> + 'if': 'defined(CONFIG_AUDIO_COREAUDIO)' },
> + { 'name': 'dsound',
> + 'if': 'defined(CONFIG_AUDIO_DSOUND)' },
> + { 'name': 'jack',
> + 'if': 'defined(CONFIG_AUDIO_JACK)' },
> + { 'name': 'oss',
> + 'if': 'defined(CONFIG_AUDIO_OSS)' },
> + { 'name': 'pa',
> + 'if': 'defined(CONFIG_AUDIO_PA)' },
> + { 'name': 'sdl',
> + 'if': 'defined(CONFIG_AUDIO_SDL)' },
> + { 'name': 'spice',
> + 'if': 'defined(CONFIG_SPICE)' },
> + 'wav' ] }
>
> ##
> # @Audiodev:
> @@ -410,14 +426,22 @@
> 'discriminator': 'driver',
> 'data': {
> 'none': 'AudiodevGenericOptions',
> - 'alsa': 'AudiodevAlsaOptions',
> - 'coreaudio': 'AudiodevCoreaudioOptions',
> - 'dsound': 'AudiodevDsoundOptions',
> - 'jack': 'AudiodevJackOptions',
> - 'oss': 'AudiodevOssOptions',
> - 'pa': 'AudiodevPaOptions',
> - 'sdl': 'AudiodevSdlOptions',
> - 'spice': 'AudiodevGenericOptions',
> + 'alsa': { 'type': 'AudiodevAlsaOptions',
> + 'if': 'defined(CONFIG_AUDIO_ALSA)' },
> + 'coreaudio': { 'type': 'AudiodevCoreaudioOptions',
> + 'if': 'defined(CONFIG_AUDIO_COREAUDIO)' },
> + 'dsound': { 'type': 'AudiodevDsoundOptions',
> + 'if': 'defined(CONFIG_AUDIO_DSOUND)' },
> + 'jack': { 'type': 'AudiodevJackOptions',
> + 'if': 'defined(CONFIG_AUDIO_JACK)' },
> + 'oss': { 'type': 'AudiodevOssOptions',
> + 'if': 'defined(CONFIG_AUDIO_OSS)' },
> + 'pa': { 'type': 'AudiodevPaOptions',
> + 'if': 'defined(CONFIG_AUDIO_PA)' },
> + 'sdl': { 'type': 'AudiodevSdlOptions',
> + 'if': 'defined(CONFIG_AUDIO_SDL)' },
> + 'spice': { 'type': 'AudiodevGenericOptions',
> + 'if': 'defined(CONFIG_SPICE)' },
> 'wav': 'AudiodevWavOptions' } }
>
> ##
On Mon, Dec 12, 2022 at 05:53:21PM +0100, Thomas Huth wrote: > On 02/03/2021 18.55, Daniel P. Berrangé wrote: > > 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. > > > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > > --- > > audio/audio.c | 16 +++++++++++++++ > > audio/audio_legacy.c | 41 ++++++++++++++++++++++++++++++++++++++- > > audio/audio_template.h | 16 +++++++++++++++ > > qapi/audio.json | 44 ++++++++++++++++++++++++++++++++---------- > > 4 files changed, 106 insertions(+), 11 deletions(-) > > Hi Daniel! > > Would you have time to respin this patch for QEMU 8.0 ? Realistically I'm not going to get it soon. If you want to take over feel free. 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 :|
On 3/2/21 11:55 AM, Daniel P. Berrangé wrote:
> 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.
Nice!
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> audio/audio.c | 16 +++++++++++++++
> audio/audio_legacy.c | 41 ++++++++++++++++++++++++++++++++++++++-
> audio/audio_template.h | 16 +++++++++++++++
> qapi/audio.json | 44 ++++++++++++++++++++++++++++++++----------
> 4 files changed, 106 insertions(+), 11 deletions(-)
>
> +++ b/qapi/audio.json
> @@ -386,8 +386,24 @@
> # Since: 4.0
> ##
> { 'enum': 'AudiodevDriver',
> - 'data': [ 'none', 'alsa', 'coreaudio', 'dsound', 'jack', 'oss', 'pa',
> - 'sdl', 'spice', 'wav' ] }
> + 'data': [ 'none',
> + { 'name': 'alsa',
> + 'if': 'defined(CONFIG_AUDIO_ALSA)' },
> + { 'name': 'coreaudio',
> + 'if': 'defined(CONFIG_AUDIO_COREAUDIO)' },
> + { 'name': 'dsound',
> + 'if': 'defined(CONFIG_AUDIO_DSOUND)' },
> + { 'name': 'jack',
> + 'if': 'defined(CONFIG_AUDIO_JACK)' },
> + { 'name': 'oss',
> + 'if': 'defined(CONFIG_AUDIO_OSS)' },
> + { 'name': 'pa',
> + 'if': 'defined(CONFIG_AUDIO_PA)' },
> + { 'name': 'sdl',
> + 'if': 'defined(CONFIG_AUDIO_SDL)' },
> + { 'name': 'spice',
> + 'if': 'defined(CONFIG_SPICE)' },
> + 'wav' ] }
I'll trust that you compiled multiple times to test the various
interplays between options.
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
On Tue, Mar 02, 2021 at 01:05:45PM -0600, Eric Blake wrote:
> On 3/2/21 11:55 AM, Daniel P. Berrangé wrote:
> > 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.
>
> Nice!
>
> >
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> > audio/audio.c | 16 +++++++++++++++
> > audio/audio_legacy.c | 41 ++++++++++++++++++++++++++++++++++++++-
> > audio/audio_template.h | 16 +++++++++++++++
> > qapi/audio.json | 44 ++++++++++++++++++++++++++++++++----------
> > 4 files changed, 106 insertions(+), 11 deletions(-)
> >
>
> > +++ b/qapi/audio.json
> > @@ -386,8 +386,24 @@
> > # Since: 4.0
> > ##
> > { 'enum': 'AudiodevDriver',
> > - 'data': [ 'none', 'alsa', 'coreaudio', 'dsound', 'jack', 'oss', 'pa',
> > - 'sdl', 'spice', 'wav' ] }
> > + 'data': [ 'none',
> > + { 'name': 'alsa',
> > + 'if': 'defined(CONFIG_AUDIO_ALSA)' },
> > + { 'name': 'coreaudio',
> > + 'if': 'defined(CONFIG_AUDIO_COREAUDIO)' },
> > + { 'name': 'dsound',
> > + 'if': 'defined(CONFIG_AUDIO_DSOUND)' },
> > + { 'name': 'jack',
> > + 'if': 'defined(CONFIG_AUDIO_JACK)' },
> > + { 'name': 'oss',
> > + 'if': 'defined(CONFIG_AUDIO_OSS)' },
> > + { 'name': 'pa',
> > + 'if': 'defined(CONFIG_AUDIO_PA)' },
> > + { 'name': 'sdl',
> > + 'if': 'defined(CONFIG_AUDIO_SDL)' },
> > + { 'name': 'spice',
> > + 'if': 'defined(CONFIG_SPICE)' },
> > + 'wav' ] }
>
> I'll trust that you compiled multiple times to test the various
> interplays between options.
No, just sent it through gitlab CI which I assumed would cover it
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
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 :|
On Tue, Mar 02, 2021 at 05:55:23PM +0000, Daniel P. Berrangé wrote: > 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. Note that audio backends are modularized, so "compiled with CONFIG_AUDIO_ALSA" doesn't imply "alsa support is available". take care, Gerd
On Wed, Mar 03, 2021 at 08:00:59AM +0100, Gerd Hoffmann wrote: > On Tue, Mar 02, 2021 at 05:55:23PM +0000, Daniel P. Berrangé wrote: > > 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. > > Note that audio backends are modularized, so "compiled with > CONFIG_AUDIO_ALSA" doesn't imply "alsa support is available". AFAIK, there's no way to handle this with QAPI schema reporting. We can only conditionalize based on what's available at compile time, not what's installed at runtime. To get runtime info, we would have to introduce an explicit "query-audiodev-types" command where just report the backends that have been installed. 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 :|
Daniel P. Berrangé <berrange@redhat.com> writes: > On Wed, Mar 03, 2021 at 08:00:59AM +0100, Gerd Hoffmann wrote: >> On Tue, Mar 02, 2021 at 05:55:23PM +0000, Daniel P. Berrangé wrote: >> > 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. >> >> Note that audio backends are modularized, so "compiled with >> CONFIG_AUDIO_ALSA" doesn't imply "alsa support is available". > > AFAIK, there's no way to handle this with QAPI schema reporting. We > can only conditionalize based on what's available at compile time, > not what's installed at runtime. Correct. The schema is fixed at compile-time. query-qmp-schema reflects what we compiled into the binary or modules we built along with the binary. It cannot tell which of the modules the binary can load. I'd like the commit message to discuss how the patch interacts with modules, because my own memory of the details is rather uncertain :) I guess we can configure which audio backends to build, and whether to build them as modules. When not building them as modules, the patch compiles out some useless code. Correct? When building them as modules, the patch compiles out code for modules we don't build. Correct? Such code is useless, unless you somehow manage to supply the resulting binary with working modules from another build. Which is probably a bad idea. Compiling out the code stops this (questionable) usage cold. No objection, but the commit message should at least hint at it. > To get runtime info, we would have to introduce an explicit > "query-audiodev-types" command where just report the backends > that have been installed. TTOCTOU. Harmless one. Still, the robust check for "can module M be loaded?" is to try loading it.
Daniel P. Berrangé <berrange@redhat.com> writes:
> 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.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Subject "qapi, audio: respect build time conditions in audio schema"
feels too narrow. The patch goes beyond the schema, because it has to:
guarding QAPI schema parts with 'if' requires guarding use of C code
generated for it with #if.
An easy way out is perhaps stating just the aim:
audio: Make introspection reflect build configuration
This assumes the patch does a complete job. If there's more audio build
configuration to reflect, add a suitable qualifier like "more closely".
Fun: before the patch, the CONFIG_AUDIO_ conditionals are effectively
applied just to output of -help.
[...]
> diff --git a/qapi/audio.json b/qapi/audio.json
> index d7b91230d7..9af1b8140c 100644
> --- a/qapi/audio.json
> +++ b/qapi/audio.json
> @@ -386,8 +386,24 @@
> # Since: 4.0
> ##
> { 'enum': 'AudiodevDriver',
> - 'data': [ 'none', 'alsa', 'coreaudio', 'dsound', 'jack', 'oss', 'pa',
> - 'sdl', 'spice', 'wav' ] }
> + 'data': [ 'none',
> + { 'name': 'alsa',
> + 'if': 'defined(CONFIG_AUDIO_ALSA)' },
> + { 'name': 'coreaudio',
> + 'if': 'defined(CONFIG_AUDIO_COREAUDIO)' },
> + { 'name': 'dsound',
> + 'if': 'defined(CONFIG_AUDIO_DSOUND)' },
> + { 'name': 'jack',
> + 'if': 'defined(CONFIG_AUDIO_JACK)' },
> + { 'name': 'oss',
> + 'if': 'defined(CONFIG_AUDIO_OSS)' },
> + { 'name': 'pa',
> + 'if': 'defined(CONFIG_AUDIO_PA)' },
> + { 'name': 'sdl',
> + 'if': 'defined(CONFIG_AUDIO_SDL)' },
> + { 'name': 'spice',
> + 'if': 'defined(CONFIG_SPICE)' },
> + 'wav' ] }
>
> ##
> # @Audiodev:
> @@ -410,14 +426,22 @@
> 'discriminator': 'driver',
> 'data': {
> 'none': 'AudiodevGenericOptions',
> - 'alsa': 'AudiodevAlsaOptions',
> - 'coreaudio': 'AudiodevCoreaudioOptions',
> - 'dsound': 'AudiodevDsoundOptions',
> - 'jack': 'AudiodevJackOptions',
> - 'oss': 'AudiodevOssOptions',
> - 'pa': 'AudiodevPaOptions',
> - 'sdl': 'AudiodevSdlOptions',
> - 'spice': 'AudiodevGenericOptions',
> + 'alsa': { 'type': 'AudiodevAlsaOptions',
> + 'if': 'defined(CONFIG_AUDIO_ALSA)' },
> + 'coreaudio': { 'type': 'AudiodevCoreaudioOptions',
> + 'if': 'defined(CONFIG_AUDIO_COREAUDIO)' },
> + 'dsound': { 'type': 'AudiodevDsoundOptions',
> + 'if': 'defined(CONFIG_AUDIO_DSOUND)' },
> + 'jack': { 'type': 'AudiodevJackOptions',
> + 'if': 'defined(CONFIG_AUDIO_JACK)' },
> + 'oss': { 'type': 'AudiodevOssOptions',
> + 'if': 'defined(CONFIG_AUDIO_OSS)' },
> + 'pa': { 'type': 'AudiodevPaOptions',
> + 'if': 'defined(CONFIG_AUDIO_PA)' },
> + 'sdl': { 'type': 'AudiodevSdlOptions',
> + 'if': 'defined(CONFIG_AUDIO_SDL)' },
> + 'spice': { 'type': 'AudiodevGenericOptions',
> + 'if': 'defined(CONFIG_SPICE)' },
> 'wav': 'AudiodevWavOptions' } }
>
> ##
For the QAPI schema part:
Acked-by: Markus Armbruster <armbru@redhat.com>
© 2016 - 2026 Red Hat, Inc.