Re: [PATCH 13/18] audio: Make AUD_register_card fallible and require audiodev=

[Daniel P . Berrangé]

On Mon, Apr 25, 2022 at 10:21:56AM +0200, Martin Kletzander wrote:
> Now that all callers support error reporting with errp and all machine-default
> devices use an explicit audiodev, this can be changed.  To make the detection
> easier make AUD_register_card() return false on error.
> 
> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> ---
>  audio/audio.c        | 7 +++++--
>  audio/audio.h        | 2 +-
>  hw/arm/omap2.c       | 3 ++-
>  hw/audio/ac97.c      | 6 +++++-
>  hw/audio/adlib.c     | 7 +++++--
>  hw/audio/cs4231a.c   | 6 ++++--
>  hw/audio/es1370.c    | 5 ++++-
>  hw/audio/gus.c       | 4 +++-
>  hw/audio/hda-codec.c | 5 ++++-
>  hw/audio/lm4549.c    | 4 +++-
>  hw/audio/pcspk.c     | 4 +++-
>  hw/audio/sb16.c      | 6 ++++--
>  hw/audio/wm8750.c    | 5 ++++-
>  hw/display/xlnx_dp.c | 6 ++++--
>  hw/input/tsc210x.c   | 3 ++-
>  hw/usb/dev-audio.c   | 5 ++++-
>  16 files changed, 57 insertions(+), 21 deletions(-)
> 
> diff --git a/audio/audio.c b/audio/audio.c
> index 671845c65d18..b95aca444382 100644
> --- a/audio/audio.c
> +++ b/audio/audio.c
> @@ -1822,15 +1822,18 @@ void audio_free_audiodev_list(AudiodevListHead *head)
>      }
>  }
>  
> -void AUD_register_card (const char *name, QEMUSoundCard *card)
> +bool AUD_register_card (const char *name, QEMUSoundCard *card, Error **errp)
>  {
>      if (!card->state) {
> -        card->state = audio_init(NULL, name);
> +        error_setg(errp, "No audiodev specified for %s", name);
> +        return false;
>      }

This is a significant change in semantics.

  qemu-system-x86_64 -device ac97

will currently automatically create a default audio backend for the
user, but now it just reports an error. I don't think we want todo
this, as allowing 'audiodev' to be optional was an intentionale
thing to be more user friendly to casual userss. It lets command
line args they use "just work" regardless of which audio subsystem
their host OS happens to be using, which wouldn't be the case if we
force them to use -audiodev every time.

>  
>      card->name = g_strdup (name);
>      memset (&card->entries, 0, sizeof (card->entries));
>      QLIST_INSERT_HEAD(&card->state->card_head, card, entries);
> +
> +    return true;
>  }

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 :|