Re: [Qemu-devel] [PATCH 14/25] audio: audiodev= parameters no longer optional when -audiodev present

[Marc-André Lureau]

Hi

On Thu, Aug 6, 2015 at 8:28 PM, Kővágó, Zoltán <address@hidden> wrote:
> Signed-off-by: Kővágó, Zoltán <address@hidden>
> ---
>  audio/audio.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/audio/audio.c b/audio/audio.c
> index 10b9871..7468b94 100644
> --- a/audio/audio.c
> +++ b/audio/audio.c
> @@ -69,6 +69,8 @@ const struct mixeng_volume nominal_volume = {
>  #endif
>  };
>
> +static bool legacy_config;
> +
>  #ifdef AUDIO_IS_FLAWLESS_AND_NO_CHECKS_ARE_REQURIED
>  #error No its not
>  #else
> @@ -1331,8 +1333,13 @@ static AudioState *audio_init(Audiodev *dev)
>      if (dev) {
>          drvname = AudiodevDriver_lookup[dev->kind];
>      } else if (!QTAILQ_EMPTY(&audio_states)) {
> +        if (!legacy_config) {
> +            dolog("Must specify audiodev when using -audiodev\n");
> +            exit(1);
> +        }
>          return QTAILQ_FIRST(&audio_states);
>      } else {
> +        legacy_config = true;
>          audio_handle_legacy_opts();
>          list = qemu_find_opts("audiodev");
>          dev = parse_option(QTAILQ_FIRST(&list->head), &error_abort);
> @@ -1463,7 +1470,11 @@ CaptureVoiceOut *AUD_add_capture(
>      CaptureVoiceOut *cap;
>      struct capture_callback *cb;
>
> -    if (!s) { /* todo */
> +    if (!s) {
> +        if (!legacy_config) {
> +            dolog("Must specify audiodev when using -audiodev\n");

This error message is a bit confusing. I think adding a bit of context
could help, you could pass the card name from the AUD_register_card
call, and use a sentence like: "You must specify an audiodev= for the
device %s".

> +            goto err0;
> +        }
>          s = QTAILQ_FIRST(&audio_states);
>      }
>
> --
> 2.4.5
>
>

looks good otherwise,

Reviewed-by: Marc-André Lureau <address@hidden>



-- 
Marc-André Lureau