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

[Daniel P . Berrangé]

On Mon, Apr 25, 2022 at 02:34:08PM +0100, Daniel P. Berrangé wrote:
> 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.

Oh, I missed that we had already deprecated the omission of audiodev
with the intent to make it mandatory (having previously tried to
make it mandatory earlier)

commit 4b3b7793e18e1e3edb90bbc21112e875f9ff826d
Author: Kővágó, Zoltán <dirty.ice.hu@gmail.com>
Date:   Mon Aug 26 21:59:02 2019 +0200

    audio: omitting audiodev= parameter is only deprecated

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

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