1. Patchew
  2. QEMU
  3. View series

[PATCH] audio: exit(1) if audio backend failed to be found or initialized

marcandre.lureau@redhat.com posted 1 patch 1 year, 8 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20220822131021.975656-1-marcandre.lureau@redhat.com
Maintainers: Gerd Hoffmann <kraxel@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>
audio/audio.h |  2 +-
audio/audio.c | 14 +++++++++++---
softmmu/vl.c  |  4 +++-
3 files changed, 15 insertions(+), 5 deletions(-)
[PATCH] audio: exit(1) if audio backend failed to be found or initialized
Posted by marcandre.lureau@redhat.com 1 year, 8 months ago 
From: Marc-André Lureau <marcandre.lureau@redhat.com>

If you specify a known backend but it isn't compiled in, or failed to
initialize, you get a simple warning and the "none" backend as a
fallback, and QEMU runs happily:

$ qemu-system-x86_64 -audiodev id=audio,driver=dsound
audio: Unknown audio driver `dsound'
audio: warning: Using timer based audio emulation
...

Instead, QEMU should fail to start:
$ qemu-system-x86_64 -audiodev id=audio,driver=dsound
audio: Unknown audio driver `dsound'
$

Resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=1983493

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 audio/audio.h |  2 +-
 audio/audio.c | 14 +++++++++++---
 softmmu/vl.c  |  4 +++-
 3 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/audio/audio.h b/audio/audio.h
index b5e17cd218..27e67079a0 100644
--- a/audio/audio.h
+++ b/audio/audio.h
@@ -170,7 +170,7 @@ void audio_sample_from_uint64(void *samples, int pos,
 
 void audio_define(Audiodev *audio);
 void audio_parse_option(const char *opt);
-void audio_init_audiodevs(void);
+bool audio_init_audiodevs(void);
 void audio_legacy_help(void);
 
 AudioState *audio_state_by_name(const char *name);
diff --git a/audio/audio.c b/audio/audio.c
index a02f3ce5c6..76b8735b44 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1743,7 +1743,6 @@ static AudioState *audio_init(Audiodev *dev, const char *name)
         atexit(audio_cleanup);
         atexit_registered = true;
     }
-    QTAILQ_INSERT_TAIL(&audio_states, s, list);
 
     s->ts = timer_new_ns(QEMU_CLOCK_VIRTUAL, audio_timer, s);
 
@@ -1769,6 +1768,10 @@ static AudioState *audio_init(Audiodev *dev, const char *name)
         } else {
             dolog ("Unknown audio driver `%s'\n", drvname);
         }
+        if (!done) {
+            free_audio_state(s);
+            return NULL;
+        }
     } else {
         for (i = 0; audio_prio_list[i]; i++) {
             AudiodevListEntry *e = audiodev_find(&head, audio_prio_list[i]);
@@ -1806,6 +1809,7 @@ static AudioState *audio_init(Audiodev *dev, const char *name)
                "(Audio can continue looping even after stopping the VM)\n");
     }
 
+    QTAILQ_INSERT_TAIL(&audio_states, s, list);
     QLIST_INIT (&s->card_head);
     vmstate_register (NULL, 0, &vmstate_audio, s);
     return s;
@@ -2119,13 +2123,17 @@ void audio_define(Audiodev *dev)
     QSIMPLEQ_INSERT_TAIL(&audiodevs, e, next);
 }
 
-void audio_init_audiodevs(void)
+bool audio_init_audiodevs(void)
 {
     AudiodevListEntry *e;
 
     QSIMPLEQ_FOREACH(e, &audiodevs, next) {
-        audio_init(e->dev, NULL);
+        if (!audio_init(e->dev, NULL)) {
+            return false;
+        }
     }
+
+    return true;
 }
 
 audsettings audiodev_to_audsettings(AudiodevPerDirectionOptions *pdo)
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 706bd7cff7..dea4005e47 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -1885,7 +1885,9 @@ static void qemu_create_early_backends(void)
      * setting machine properties, so they can be referred to.
      */
     configure_blockdev(&bdo_queue, machine_class, snapshot);
-    audio_init_audiodevs();
+    if (!audio_init_audiodevs()) {
+        exit(1);
+    }
 }
 
 
-- 
2.37.1
Re: [PATCH] audio: exit(1) if audio backend failed to be found or initialized
Posted by Volker Rümelin 1 year, 8 months ago 
> From: Marc-André Lureau<marcandre.lureau@redhat.com>
>
> If you specify a known backend but it isn't compiled in, or failed to
> initialize, you get a simple warning and the "none" backend as a
> fallback, and QEMU runs happily:
>
> $ qemu-system-x86_64 -audiodev id=audio,driver=dsound
> audio: Unknown audio driver `dsound'
> audio: warning: Using timer based audio emulation
> ...
>
> Instead, QEMU should fail to start:
> $ qemu-system-x86_64 -audiodev id=audio,driver=dsound
> audio: Unknown audio driver `dsound'
> $
>
> Resolves:
> https://bugzilla.redhat.com/show_bug.cgi?id=1983493
>
> Signed-off-by: Marc-André Lureau<marcandre.lureau@redhat.com>
> ---
>   audio/audio.h |  2 +-
>   audio/audio.c | 14 +++++++++++---
>   softmmu/vl.c  |  4 +++-
>   3 files changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/audio/audio.h b/audio/audio.h
> index b5e17cd218..27e67079a0 100644
> --- a/audio/audio.h
> +++ b/audio/audio.h
> @@ -170,7 +170,7 @@ void audio_sample_from_uint64(void *samples, int pos,
>   
>   void audio_define(Audiodev *audio);
>   void audio_parse_option(const char *opt);
> -void audio_init_audiodevs(void);
> +bool audio_init_audiodevs(void);
>   void audio_legacy_help(void);
>   
>   AudioState *audio_state_by_name(const char *name);
> diff --git a/audio/audio.c b/audio/audio.c
> index a02f3ce5c6..76b8735b44 100644
> --- a/audio/audio.c
> +++ b/audio/audio.c
> @@ -1743,7 +1743,6 @@ static AudioState *audio_init(Audiodev *dev, const char *name)
>           atexit(audio_cleanup);
>           atexit_registered = true;
>       }
> -    QTAILQ_INSERT_TAIL(&audio_states, s, list);
>   
>       s->ts = timer_new_ns(QEMU_CLOCK_VIRTUAL, audio_timer, s);
>   
> @@ -1769,6 +1768,10 @@ static AudioState *audio_init(Audiodev *dev, const char *name)
>           } else {
>               dolog ("Unknown audio driver `%s'\n", drvname);
>           }
> +        if (!done) {
> +            free_audio_state(s);
> +            return NULL;

Hi Marc-André,

I don't understand why you move the QTAILQ_INSERT_TAIL(&audio_states, s, 
list) macro down. Without this you don't need the additional 
free_audio_state() call. audio_cleanup() takes care to free the audio state.

The rest looks good. You can add my

Reviewed-by: Volker Rümelin <vr_qemu@t-online.de>

even if you insist on keeping the patch as it is.

With best regards,
Volker

> +        }
>       } else {
>           for (i = 0; audio_prio_list[i]; i++) {
>               AudiodevListEntry *e = audiodev_find(&head, audio_prio_list[i]);
> @@ -1806,6 +1809,7 @@ static AudioState *audio_init(Audiodev *dev, const char *name)
>                  "(Audio can continue looping even after stopping the VM)\n");
>       }
>   
> +    QTAILQ_INSERT_TAIL(&audio_states, s, list);
>       QLIST_INIT (&s->card_head);
>       vmstate_register (NULL, 0, &vmstate_audio, s);
>       return s;
> @@ -2119,13 +2123,17 @@ void audio_define(Audiodev *dev)
>       QSIMPLEQ_INSERT_TAIL(&audiodevs, e, next);
>   }
>   
> -void audio_init_audiodevs(void)
> +bool audio_init_audiodevs(void)
>   {
>       AudiodevListEntry *e;
>   
>       QSIMPLEQ_FOREACH(e, &audiodevs, next) {
> -        audio_init(e->dev, NULL);
> +        if (!audio_init(e->dev, NULL)) {
> +            return false;
> +        }
>       }
> +
> +    return true;
>   }
>   
>   audsettings audiodev_to_audsettings(AudiodevPerDirectionOptions *pdo)
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index 706bd7cff7..dea4005e47 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -1885,7 +1885,9 @@ static void qemu_create_early_backends(void)
>        * setting machine properties, so they can be referred to.
>        */
>       configure_blockdev(&bdo_queue, machine_class, snapshot);
> -    audio_init_audiodevs();
> +    if (!audio_init_audiodevs()) {
> +        exit(1);
> +    }
>   }
>   
>
Re: [PATCH] audio: exit(1) if audio backend failed to be found or initialized
Posted by Marc-André Lureau 1 year, 8 months ago 
Hi

On Tue, Aug 23, 2022 at 10:53 PM Volker Rümelin <vr_qemu@t-online.de> wrote:

> > From: Marc-André Lureau<marcandre.lureau@redhat.com>
> >
> > If you specify a known backend but it isn't compiled in, or failed to
> > initialize, you get a simple warning and the "none" backend as a
> > fallback, and QEMU runs happily:
> >
> > $ qemu-system-x86_64 -audiodev id=audio,driver=dsound
> > audio: Unknown audio driver `dsound'
> > audio: warning: Using timer based audio emulation
> > ...
> >
> > Instead, QEMU should fail to start:
> > $ qemu-system-x86_64 -audiodev id=audio,driver=dsound
> > audio: Unknown audio driver `dsound'
> > $
> >
> > Resolves:
> > https://bugzilla.redhat.com/show_bug.cgi?id=1983493
> >
> > Signed-off-by: Marc-André Lureau<marcandre.lureau@redhat.com>
> > ---
> >   audio/audio.h |  2 +-
> >   audio/audio.c | 14 +++++++++++---
> >   softmmu/vl.c  |  4 +++-
> >   3 files changed, 15 insertions(+), 5 deletions(-)
> >
> > diff --git a/audio/audio.h b/audio/audio.h
> > index b5e17cd218..27e67079a0 100644
> > --- a/audio/audio.h
> > +++ b/audio/audio.h
> > @@ -170,7 +170,7 @@ void audio_sample_from_uint64(void *samples, int pos,
> >
> >   void audio_define(Audiodev *audio);
> >   void audio_parse_option(const char *opt);
> > -void audio_init_audiodevs(void);
> > +bool audio_init_audiodevs(void);
> >   void audio_legacy_help(void);
> >
> >   AudioState *audio_state_by_name(const char *name);
> > diff --git a/audio/audio.c b/audio/audio.c
> > index a02f3ce5c6..76b8735b44 100644
> > --- a/audio/audio.c
> > +++ b/audio/audio.c
> > @@ -1743,7 +1743,6 @@ static AudioState *audio_init(Audiodev *dev, const
> char *name)
> >           atexit(audio_cleanup);
> >           atexit_registered = true;
> >       }
> > -    QTAILQ_INSERT_TAIL(&audio_states, s, list);
> >
> >       s->ts = timer_new_ns(QEMU_CLOCK_VIRTUAL, audio_timer, s);
> >
> > @@ -1769,6 +1768,10 @@ static AudioState *audio_init(Audiodev *dev,
> const char *name)
> >           } else {
> >               dolog ("Unknown audio driver `%s'\n", drvname);
> >           }
> > +        if (!done) {
> > +            free_audio_state(s);
> > +            return NULL;
>
> Hi Marc-André,
>
> I don't understand why you move the QTAILQ_INSERT_TAIL(&audio_states, s,
> list) macro down. Without this you don't need the additional
> free_audio_state() call. audio_cleanup() takes care to free the audio
> state.


Ah I see what you mean, atexit will be called. But since exit() is called
outside the function, it's not a good idea to rely on the caller imho.
Better not add it at all imho, and free state on failure.



> The rest looks good. You can add my
>
> Reviewed-by: Volker Rümelin <vr_qemu@t-online.de>


> even if you insist on keeping the patch as it is.
>

thanks


>
> With best regards,
> Volker
>
> > +        }
> >       } else {
> >           for (i = 0; audio_prio_list[i]; i++) {
> >               AudiodevListEntry *e = audiodev_find(&head,
> audio_prio_list[i]);
> > @@ -1806,6 +1809,7 @@ static AudioState *audio_init(Audiodev *dev, const
> char *name)
> >                  "(Audio can continue looping even after stopping the
> VM)\n");
> >       }
> >
> > +    QTAILQ_INSERT_TAIL(&audio_states, s, list);
> >       QLIST_INIT (&s->card_head);
> >       vmstate_register (NULL, 0, &vmstate_audio, s);
> >       return s;
> > @@ -2119,13 +2123,17 @@ void audio_define(Audiodev *dev)
> >       QSIMPLEQ_INSERT_TAIL(&audiodevs, e, next);
> >   }
> >
> > -void audio_init_audiodevs(void)
> > +bool audio_init_audiodevs(void)
> >   {
> >       AudiodevListEntry *e;
> >
> >       QSIMPLEQ_FOREACH(e, &audiodevs, next) {
> > -        audio_init(e->dev, NULL);
> > +        if (!audio_init(e->dev, NULL)) {
> > +            return false;
> > +        }
> >       }
> > +
> > +    return true;
> >   }
> >
> >   audsettings audiodev_to_audsettings(AudiodevPerDirectionOptions *pdo)
> > diff --git a/softmmu/vl.c b/softmmu/vl.c
> > index 706bd7cff7..dea4005e47 100644
> > --- a/softmmu/vl.c
> > +++ b/softmmu/vl.c
> > @@ -1885,7 +1885,9 @@ static void qemu_create_early_backends(void)
> >        * setting machine properties, so they can be referred to.
> >        */
> >       configure_blockdev(&bdo_queue, machine_class, snapshot);
> > -    audio_init_audiodevs();
> > +    if (!audio_init_audiodevs()) {
> > +        exit(1);
> > +    }
> >   }
> >
> >
>
>
>

-- 
Marc-André Lureau

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.