[PATCH 0/9] audio: remove deprecated QEMU_AUDIO env support

[PATCH 0/9] audio: remove deprecated QEMU_AUDIO env support

[Daniel P. Berrangé]

This removes much of the deprecated audio code, most
notably the QEMU_AUDIO env variables. The VNC server
will also stop accepting client requests for audio
streaming unless an audiodev is set.

I tried to make the use of 'audiodev' mandatory for
devices but that turned out to not be practical. Our
test suite assumes it can create any device type
without setting any properties. This is not possible
if 'audiodev' setting is enforced at realize() time.
Also there are several machihe boards with embedded
soundcards that provide no way to set audiodev AFAICT

Daniel P. Berrangé (9):
  audio: don't check qemu_add_vm_change_state_handler failure
  audio: remove special audio_calloc function
  audio: remove unused 'name' in QEMUSoundCard struct
  audio: remove QEMUSoundCard linked list
  audio: remove empty AUD_remove_card method
  docs: split the deprecation warning for soundcards vs VNC
  ui/vnc: don't accept VNC_ENCODING_AUDIO without audiodev
  audio: audio state is now mandatory for capture
  audio: remove support for QEMU_AUDIO_ env variables

 audio/alsaaudio.c               |   6 +-
 audio/audio.c                   |  71 +---
 audio/audio.h                   |   4 -
 audio/audio_int.h               |   2 -
 audio/audio_legacy.c            | 552 --------------------------------
 audio/audio_template.h          |  28 +-
 audio/meson.build               |   1 -
 audio/mixeng.c                  |   7 +-
 docs/about/deprecated.rst       |  16 +-
 docs/about/removed-features.rst |  14 +
 hw/audio/ac97.c                 |   1 -
 hw/audio/adlib.c                |   1 -
 hw/audio/es1370.c               |   1 -
 hw/audio/gus.c                  |   1 -
 hw/audio/hda-codec.c            |   1 -
 hw/audio/wm8750.c               |   1 -
 hw/usb/dev-audio.c              |   1 -
 softmmu/vl.c                    |   4 -
 tests/qtest/fuzz-sb16-test.c    |   6 +-
 tests/qtest/libqtest.c          |   3 -
 ui/vnc.c                        |  10 +-
 21 files changed, 53 insertions(+), 678 deletions(-)
 delete mode 100644 audio/audio_legacy.c

-- 
2.38.1

[PATCH 1/9] audio: don't check qemu_add_vm_change_state_handler failure

[Daniel P. Berrangé]

This function cannot fail since g_malloc0 aborts on OOM.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 audio/audio.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index d849a94a81..7b4b957945 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1712,7 +1712,6 @@ static AudioState *audio_init(Audiodev *dev, const char *name)
     size_t i;
     int done = 0;
     const char *drvname = NULL;
-    VMChangeStateEntry *e;
     AudioState *s;
     struct audio_driver *driver;
     /* silence gcc warning about uninitialized variable */
@@ -1830,11 +1829,7 @@ static AudioState *audio_init(Audiodev *dev, const char *name)
         s->period_ticks = dev->timer_period * (int64_t)SCALE_US;
     }
 
-    e = qemu_add_vm_change_state_handler (audio_vm_change_state_handler, s);
-    if (!e) {
-        dolog ("warning: Could not register change state handler\n"
-               "(Audio can continue looping even after stopping the VM)\n");
-    }
+    qemu_add_vm_change_state_handler (audio_vm_change_state_handler, s);
 
     QTAILQ_INSERT_TAIL(&audio_states, s, list);
     QLIST_INIT (&s->card_head);
-- 
2.38.1

[PATCH 2/9] audio: remove special audio_calloc function

[Daniel P. Berrangé]

The audio_calloc function does various checks on the size and
nmembers parameters to detect various error conditions. There
are only 5 callers

 * alsa_poll_helper: the pollfd count is small and bounded,
 * audio_pcm_create_voice_pair_: allocating a single fixed
   size struct
 * audio_pcm_sw_alloc_resources_: samples could be negative
   zero, or overflow, so needs a check
 * audio_pcm_hw_add_new_: voice size could be zero for
   backends that don't support audio input
 * st_rate_start: allocating a single fixed size struct

IOW, only two of the callers need special error checks and
it is clearer if their respective checks are inlined. Thus
audio_calloc can be eliminated.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 audio/alsaaudio.c            |  6 +-----
 audio/audio.c                | 20 --------------------
 audio/audio_int.h            |  1 -
 audio/audio_template.h       | 28 ++++++++++++++--------------
 audio/mixeng.c               |  7 +------
 tests/qtest/fuzz-sb16-test.c |  6 ++++--
 6 files changed, 20 insertions(+), 48 deletions(-)

diff --git a/audio/alsaaudio.c b/audio/alsaaudio.c
index 714bfb6453..5f50dfa0bf 100644
--- a/audio/alsaaudio.c
+++ b/audio/alsaaudio.c
@@ -222,11 +222,7 @@ static int alsa_poll_helper (snd_pcm_t *handle, struct pollhlp *hlp, int mask)
         return -1;
     }
 
-    pfds = audio_calloc ("alsa_poll_helper", count, sizeof (*pfds));
-    if (!pfds) {
-        dolog ("Could not initialize poll mode\n");
-        return -1;
-    }
+    pfds = g_new0(struct pollfd, count);
 
     err = snd_pcm_poll_descriptors (handle, pfds, count);
     if (err < 0) {
diff --git a/audio/audio.c b/audio/audio.c
index 7b4b957945..f397072a1f 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -146,26 +146,6 @@ static inline int audio_bits_to_index (int bits)
     }
 }
 
-void *audio_calloc (const char *funcname, int nmemb, size_t size)
-{
-    int cond;
-    size_t len;
-
-    len = nmemb * size;
-    cond = !nmemb || !size;
-    cond |= nmemb < 0;
-    cond |= len < size;
-
-    if (audio_bug ("audio_calloc", cond)) {
-        AUD_log (NULL, "%s passed invalid arguments to audio_calloc\n",
-                 funcname);
-        AUD_log (NULL, "nmemb=%d size=%zu (len=%zu)\n", nmemb, size, len);
-        return NULL;
-    }
-
-    return g_malloc0 (len);
-}
-
 void AUD_vlog (const char *cap, const char *fmt, va_list ap)
 {
     if (cap) {
diff --git a/audio/audio_int.h b/audio/audio_int.h
index e87ce014a0..b0cc2cd390 100644
--- a/audio/audio_int.h
+++ b/audio/audio_int.h
@@ -251,7 +251,6 @@ void audio_pcm_init_info (struct audio_pcm_info *info, struct audsettings *as);
 void audio_pcm_info_clear_buf (struct audio_pcm_info *info, void *buf, int len);
 
 int audio_bug (const char *funcname, int cond);
-void *audio_calloc (const char *funcname, int nmemb, size_t size);
 
 void audio_run(AudioState *s, const char *msg);
 
diff --git a/audio/audio_template.h b/audio/audio_template.h
index 720a32e57e..564cbb1f01 100644
--- a/audio/audio_template.h
+++ b/audio/audio_template.h
@@ -116,13 +116,20 @@ static int glue (audio_pcm_sw_alloc_resources_, TYPE) (SW *sw)
     samples = (int64_t)sw->HWBUF->size * sw->ratio >> 32;
 #endif
 
-    sw->buf = audio_calloc(__func__, samples, sizeof(struct st_sample));
-    if (!sw->buf) {
-        dolog ("Could not allocate buffer for `%s' (%d samples)\n",
+    if (audio_bug(__func__, samples <= 0)) {
+        dolog ("Could not allocate buffer for '%s', samples %d <= 0\n",
                SW_NAME (sw), samples);
         return -1;
     }
 
+    if (audio_bug(__func__, (SIZE_MAX / sizeof(struct st_sample) < samples))) {
+        dolog ("Could not allocate buffer for '%s', samples %d overflows\n",
+               SW_NAME (sw), samples);
+        return -1;
+    }
+
+    sw->buf = g_new0(struct st_sample, samples);
+
 #ifdef DAC
     sw->rate = st_rate_start (sw->info.freq, sw->hw->info.freq);
 #else
@@ -264,13 +271,12 @@ static HW *glue(audio_pcm_hw_add_new_, TYPE)(AudioState *s,
         return NULL;
     }
 
-    hw = audio_calloc(__func__, 1, glue(drv->voice_size_, TYPE));
-    if (!hw) {
-        dolog ("Can not allocate voice `%s' size %d\n",
-               drv->name, glue (drv->voice_size_, TYPE));
+    if (audio_bug(__func__, glue(drv->voice_size_, TYPE) == 0)) {
+        dolog ("Voice size is zero");
         return NULL;
     }
 
+    hw = g_malloc0(glue(drv->voice_size_, TYPE));
     hw->s = s;
     hw->pcm_ops = drv->pcm_ops;
 
@@ -398,12 +404,7 @@ static SW *glue(audio_pcm_create_voice_pair_, TYPE)(
         hw_as = *as;
     }
 
-    sw = audio_calloc(__func__, 1, sizeof(*sw));
-    if (!sw) {
-        dolog ("Could not allocate soft voice `%s' (%zu bytes)\n",
-               sw_name ? sw_name : "unknown", sizeof (*sw));
-        goto err1;
-    }
+    sw = g_new0(SW, 1);
     sw->s = s;
 
     hw = glue(audio_pcm_hw_add_, TYPE)(s, &hw_as);
@@ -424,7 +425,6 @@ err3:
     glue (audio_pcm_hw_gc_, TYPE) (&hw);
 err2:
     g_free (sw);
-err1:
     return NULL;
 }
 
diff --git a/audio/mixeng.c b/audio/mixeng.c
index 100a306d6f..fe454e0725 100644
--- a/audio/mixeng.c
+++ b/audio/mixeng.c
@@ -414,12 +414,7 @@ struct rate {
  */
 void *st_rate_start (int inrate, int outrate)
 {
-    struct rate *rate = audio_calloc(__func__, 1, sizeof(*rate));
-
-    if (!rate) {
-        dolog ("Could not allocate resampler (%zu bytes)\n", sizeof (*rate));
-        return NULL;
-    }
+    struct rate *rate = g_new0(struct rate, 1);
 
     rate->opos = 0;
 
diff --git a/tests/qtest/fuzz-sb16-test.c b/tests/qtest/fuzz-sb16-test.c
index fc445b1871..a28b93be3a 100644
--- a/tests/qtest/fuzz-sb16-test.c
+++ b/tests/qtest/fuzz-sb16-test.c
@@ -10,7 +10,8 @@
 #include "libqtest.h"
 
 /*
- * This used to trigger the assert in audio_calloc
+ * This used to trigger the audio_bug calls in
+ * audio_pcm_sw_alloc_resources
  * https://bugs.launchpad.net/qemu/+bug/1910603
  */
 static void test_fuzz_sb16_0x1c(void)
@@ -38,7 +39,8 @@ static void test_fuzz_sb16_0x91(void)
 }
 
 /*
- * This used to trigger the assert in audio_calloc
+ * This used to trigger the audio_bug calls in
+ * audio_pcm_sw_alloc_resources
  * through command 0xd4
  */
 static void test_fuzz_sb16_0xd4(void)
-- 
2.38.1

[PATCH 3/9] audio: remove unused 'name' in QEMUSoundCard struct

[Daniel P. Berrangé]

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 audio/audio.c | 2 --
 audio/audio.h | 1 -
 2 files changed, 3 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index f397072a1f..94a16c2dda 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1833,7 +1833,6 @@ void AUD_register_card (const char *name, QEMUSoundCard *card)
         card->state = audio_init(NULL, name);
     }
 
-    card->name = g_strdup (name);
     memset (&card->entries, 0, sizeof (card->entries));
     QLIST_INSERT_HEAD(&card->state->card_head, card, entries);
 }
@@ -1841,7 +1840,6 @@ void AUD_register_card (const char *name, QEMUSoundCard *card)
 void AUD_remove_card (QEMUSoundCard *card)
 {
     QLIST_REMOVE (card, entries);
-    g_free (card->name);
 }
 
 static struct audio_pcm_ops capture_pcm_ops;
diff --git a/audio/audio.h b/audio/audio.h
index 01bdc567fb..eefb809a54 100644
--- a/audio/audio.h
+++ b/audio/audio.h
@@ -82,7 +82,6 @@ typedef struct SWVoiceIn SWVoiceIn;
 
 typedef struct AudioState AudioState;
 typedef struct QEMUSoundCard {
-    char *name;
     AudioState *state;
     QLIST_ENTRY (QEMUSoundCard) entries;
 } QEMUSoundCard;
-- 
2.38.1

[PATCH 4/9] audio: remove QEMUSoundCard linked list

[Daniel P. Berrangé]

No code ever iterates over the list

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 audio/audio.c     | 5 -----
 audio/audio.h     | 1 -
 audio/audio_int.h | 1 -
 3 files changed, 7 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index 94a16c2dda..217095306f 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1812,7 +1812,6 @@ static AudioState *audio_init(Audiodev *dev, const char *name)
     qemu_add_vm_change_state_handler (audio_vm_change_state_handler, s);
 
     QTAILQ_INSERT_TAIL(&audio_states, s, list);
-    QLIST_INIT (&s->card_head);
     vmstate_register (NULL, 0, &vmstate_audio, s);
     return s;
 }
@@ -1832,14 +1831,10 @@ void AUD_register_card (const char *name, QEMUSoundCard *card)
     if (!card->state) {
         card->state = audio_init(NULL, name);
     }
-
-    memset (&card->entries, 0, sizeof (card->entries));
-    QLIST_INSERT_HEAD(&card->state->card_head, card, entries);
 }
 
 void AUD_remove_card (QEMUSoundCard *card)
 {
-    QLIST_REMOVE (card, entries);
 }
 
 static struct audio_pcm_ops capture_pcm_ops;
diff --git a/audio/audio.h b/audio/audio.h
index eefb809a54..ebcc540431 100644
--- a/audio/audio.h
+++ b/audio/audio.h
@@ -83,7 +83,6 @@ typedef struct SWVoiceIn SWVoiceIn;
 typedef struct AudioState AudioState;
 typedef struct QEMUSoundCard {
     AudioState *state;
-    QLIST_ENTRY (QEMUSoundCard) entries;
 } QEMUSoundCard;
 
 typedef struct QEMUAudioTimeStamp {
diff --git a/audio/audio_int.h b/audio/audio_int.h
index b0cc2cd390..ca62e49ee5 100644
--- a/audio/audio_int.h
+++ b/audio/audio_int.h
@@ -225,7 +225,6 @@ typedef struct AudioState {
     void *drv_opaque;
 
     QEMUTimer *ts;
-    QLIST_HEAD (card_listhead, QEMUSoundCard) card_head;
     QLIST_HEAD (hw_in_listhead, HWVoiceIn) hw_head_in;
     QLIST_HEAD (hw_out_listhead, HWVoiceOut) hw_head_out;
     QLIST_HEAD (cap_listhead, CaptureVoiceOut) cap_head;
-- 
2.38.1

[PATCH 5/9] audio: remove empty AUD_remove_card method

[Daniel P. Berrangé]

Since the linked list of QEMUSoundCard structs was removed,
AUD_remove_card does nothing useful.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 audio/audio.c        | 4 ----
 audio/audio.h        | 1 -
 hw/audio/ac97.c      | 1 -
 hw/audio/adlib.c     | 1 -
 hw/audio/es1370.c    | 1 -
 hw/audio/gus.c       | 1 -
 hw/audio/hda-codec.c | 1 -
 hw/audio/wm8750.c    | 1 -
 hw/usb/dev-audio.c   | 1 -
 9 files changed, 12 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index 217095306f..00128c2ad7 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1833,10 +1833,6 @@ void AUD_register_card (const char *name, QEMUSoundCard *card)
     }
 }
 
-void AUD_remove_card (QEMUSoundCard *card)
-{
-}
-
 static struct audio_pcm_ops capture_pcm_ops;
 
 CaptureVoiceOut *AUD_add_capture(
diff --git a/audio/audio.h b/audio/audio.h
index ebcc540431..8ee0e2159a 100644
--- a/audio/audio.h
+++ b/audio/audio.h
@@ -93,7 +93,6 @@ void AUD_vlog (const char *cap, const char *fmt, va_list ap) G_GNUC_PRINTF(2, 0)
 void AUD_log (const char *cap, const char *fmt, ...) G_GNUC_PRINTF(2, 3);
 
 void AUD_register_card (const char *name, QEMUSoundCard *card);
-void AUD_remove_card (QEMUSoundCard *card);
 CaptureVoiceOut *AUD_add_capture(
     AudioState *s,
     struct audsettings *as,
diff --git a/hw/audio/ac97.c b/hw/audio/ac97.c
index 364cdfa733..fd8d3abba4 100644
--- a/hw/audio/ac97.c
+++ b/hw/audio/ac97.c
@@ -1358,7 +1358,6 @@ static void ac97_exit(PCIDevice *dev)
     AUD_close_in(&s->card, s->voice_pi);
     AUD_close_out(&s->card, s->voice_po);
     AUD_close_in(&s->card, s->voice_mc);
-    AUD_remove_card(&s->card);
 }
 
 static Property ac97_properties[] = {
diff --git a/hw/audio/adlib.c b/hw/audio/adlib.c
index 5f979b1487..79b1b8e271 100644
--- a/hw/audio/adlib.c
+++ b/hw/audio/adlib.c
@@ -240,7 +240,6 @@ static void Adlib_fini (AdlibState *s)
 
     s->active = 0;
     s->enabled = 0;
-    AUD_remove_card (&s->card);
 }
 
 static MemoryRegionPortio adlib_portio_list[] = {
diff --git a/hw/audio/es1370.c b/hw/audio/es1370.c
index 54cc19a637..9a504db37e 100644
--- a/hw/audio/es1370.c
+++ b/hw/audio/es1370.c
@@ -882,7 +882,6 @@ static void es1370_exit(PCIDevice *dev)
     }
 
     AUD_close_in(&s->card, s->adc_voice);
-    AUD_remove_card(&s->card);
 }
 
 static Property es1370_properties[] = {
diff --git a/hw/audio/gus.c b/hw/audio/gus.c
index 42f010b671..c086502708 100644
--- a/hw/audio/gus.c
+++ b/hw/audio/gus.c
@@ -263,7 +263,6 @@ static void gus_realizefn (DeviceState *dev, Error **errp)
         );
 
     if (!s->voice) {
-        AUD_remove_card (&s->card);
         error_setg(errp, "No voice");
         return;
     }
diff --git a/hw/audio/hda-codec.c b/hw/audio/hda-codec.c
index feb8f9e2bb..f70a6798df 100644
--- a/hw/audio/hda-codec.c
+++ b/hw/audio/hda-codec.c
@@ -743,7 +743,6 @@ static void hda_audio_exit(HDACodecDevice *hda)
             AUD_close_in(&a->card, st->voice.in);
         }
     }
-    AUD_remove_card(&a->card);
 }
 
 static int hda_audio_post_load(void *opaque, int version)
diff --git a/hw/audio/wm8750.c b/hw/audio/wm8750.c
index b5722b37c3..b63943dd3e 100644
--- a/hw/audio/wm8750.c
+++ b/hw/audio/wm8750.c
@@ -634,7 +634,6 @@ static void wm8750_fini(I2CSlave *i2c)
     WM8750State *s = WM8750(i2c);
 
     wm8750_reset(I2C_SLAVE(s));
-    AUD_remove_card(&s->card);
     g_free(s);
 }
 #endif
diff --git a/hw/usb/dev-audio.c b/hw/usb/dev-audio.c
index 8748c1ba04..72cc89548e 100644
--- a/hw/usb/dev-audio.c
+++ b/hw/usb/dev-audio.c
@@ -934,7 +934,6 @@ static void usb_audio_unrealize(USBDevice *dev)
 
     usb_audio_set_output_altset(s, ALTSET_OFF);
     AUD_close_out(&s->card, s->out.voice);
-    AUD_remove_card(&s->card);
 
     streambuf_fini(&s->out.buf);
 }
-- 
2.38.1

[PATCH 6/9] docs: split the deprecation warning for soundcards vs VNC

[Daniel P. Berrangé]

Both soundcards and VNC will require the audiodev= property
but lets split the deprecation message since these are
distinct functional areas.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 docs/about/deprecated.rst | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 68d29642d7..f8b4e19a4c 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -31,13 +31,17 @@ backend settings instead of environment variables.  To ease migration to
 the new format, the ``-audiodev-help`` option can be used to convert
 the current values of the environment variables to ``-audiodev`` options.
 
-Creating sound card devices and vnc without ``audiodev=`` property (since 4.2)
-''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+Creating sound card devices without ``audiodev=`` property (since 4.2)
+''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
 
 When not using the deprecated legacy audio config, each sound card
-should specify an ``audiodev=`` property.  Additionally, when using
-vnc, you should specify an ``audiodev=`` property if you plan to
-transmit audio through the VNC protocol.
+should specify an ``audiodev=`` property.
+
+Supporting audio transfer over vnc without ``audiodev=`` property (since 4.2)
+'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+When using vnc, you should specify an ``audiodev=`` property if you
+intend to allow clients to request audio transfer through the VNC protocol.
 
 Short-form boolean options (since 6.0)
 ''''''''''''''''''''''''''''''''''''''
-- 
2.38.1

[PATCH 7/9] ui/vnc: don't accept VNC_ENCODING_AUDIO without audiodev

[Daniel P. Berrangé]

If we have no audio state configured, then we don't want to
advertize the VNC_ENCODING_AUDIO feature. If a client attempts
to use it despite being disabled, we should also reject it.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 docs/about/deprecated.rst       |  6 ------
 docs/about/removed-features.rst |  6 ++++++
 ui/vnc.c                        | 10 +++++++++-
 3 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index f8b4e19a4c..09269f55e6 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -37,12 +37,6 @@ Creating sound card devices without ``audiodev=`` property (since 4.2)
 When not using the deprecated legacy audio config, each sound card
 should specify an ``audiodev=`` property.
 
-Supporting audio transfer over vnc without ``audiodev=`` property (since 4.2)
-'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
-
-When using vnc, you should specify an ``audiodev=`` property if you
-intend to allow clients to request audio transfer through the VNC protocol.
-
 Short-form boolean options (since 6.0)
 ''''''''''''''''''''''''''''''''''''''
 
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index c918cabd1a..8a8e0faff0 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -422,6 +422,12 @@ the value is hexadecimal.  That is, '0x20M' should be written either as
 ``tty`` and ``parport`` used to be aliases for ``serial`` and ``parallel``
 respectively. The actual backend names should be used instead.
 
+Supporting audio transfer over vnc without ``audiodev=`` property (removed in 8.0)
+''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+When using vnc, you should specify an ``audiodev=`` property if you
+intend to allow clients to request audio transfer through the VNC protocol.
+
 QEMU Machine Protocol (QMP) commands
 ------------------------------------
 
diff --git a/ui/vnc.c b/ui/vnc.c
index d9eacad759..6b3cbf365e 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2199,7 +2199,9 @@ static void set_encodings(VncState *vs, int32_t *encodings, size_t n_encodings)
             send_ext_key_event_ack(vs);
             break;
         case VNC_ENCODING_AUDIO:
-            send_ext_audio_ack(vs);
+            if (vs->vd->audio_state != NULL) {
+                send_ext_audio_ack(vs);
+            }
             break;
         case VNC_ENCODING_WMVi:
             vs->features |= VNC_FEATURE_WMVI_MASK;
@@ -2506,6 +2508,12 @@ static int protocol_client_msg(VncState *vs, uint8_t *data, size_t len)
                           read_u32(data, 4), read_u32(data, 8));
             break;
         case VNC_MSG_CLIENT_QEMU_AUDIO:
+            if (vs->vd->audio_state == NULL) {
+                error_report("vnc: QEMU audio client message while disabled");
+                vnc_client_error(vs);
+                break;
+            }
+
             if (len == 2)
                 return 4;
 
-- 
2.38.1

[PATCH 8/9] audio: audio state is now mandatory for capture

[Daniel P. Berrangé]

Both callers of AUD_add_capture will now ensure that the
audio state is non-NULL.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 audio/audio.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index 00128c2ad7..64b75cdf94 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1845,13 +1845,6 @@ CaptureVoiceOut *AUD_add_capture(
     CaptureVoiceOut *cap;
     struct capture_callback *cb;
 
-    if (!s) {
-        if (!legacy_config) {
-            dolog("Capturing without setting an audiodev is deprecated\n");
-        }
-        s = audio_init(NULL, NULL);
-    }
-
     if (!audio_get_pdo_out(s->dev)->mixing_engine) {
         dolog("Can't capture with mixeng disabled\n");
         return NULL;
-- 
2.38.1

[PATCH 9/9] audio: remove support for QEMU_AUDIO_ env variables

[Daniel P. Berrangé]

All user created devices and the builtin pcspk can be given
a audiodev property. A few devices using audiodevs though
cannot be configured directly as they are built-in devices
created programmatically by the machine type.

To enable those to continue to be used we leave in the
logic that picks the first audiodev if none is set
explicitly. Ideally this will be removed once there is
a way to configure all built-in devices.

Even for devices which can be configured explicitly,
though, if the 'audiodev' property is mandated to be
set to a valid ID at time of realize(), it will break
several of our test suites which expect to be able to
instantiate any device without setting any properties.

So as a final fallback, we have logic that creates an
audiodev with the 'none' backend type, which will be used
in no -audiodev devices are present at all.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 audio/audio.c                   |  26 +-
 audio/audio.h                   |   1 -
 audio/audio_legacy.c            | 552 --------------------------------
 audio/meson.build               |   1 -
 docs/about/deprecated.rst       |   8 -
 docs/about/removed-features.rst |   8 +
 softmmu/vl.c                    |   4 -
 tests/qtest/libqtest.c          |   3 -
 8 files changed, 14 insertions(+), 589 deletions(-)
 delete mode 100644 audio/audio_legacy.c

diff --git a/audio/audio.c b/audio/audio.c
index 64b75cdf94..f70075c7e4 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -108,8 +108,6 @@ const struct mixeng_volume nominal_volume = {
 #endif
 };
 
-static bool legacy_config = true;
-
 int audio_bug (const char *funcname, int cond)
 {
     if (cond) {
@@ -1684,7 +1682,7 @@ static AudiodevListEntry *audiodev_find(
  * if we have dev, this function was called because of an -audiodev argument =>
  *   initialize a new state with it
  * if dev == NULL => legacy implicit initialization, return the already created
- *   state or create a new one
+ *   state or create a new no-op one.
  */
 static AudioState *audio_init(Audiodev *dev, const char *name)
 {
@@ -1716,27 +1714,15 @@ static AudioState *audio_init(Audiodev *dev, const char *name)
 
     if (dev) {
         /* -audiodev option */
-        legacy_config = false;
         drvname = AudiodevDriver_str(dev->driver);
     } else if (!QTAILQ_EMPTY(&audio_states)) {
-        if (!legacy_config) {
-            dolog("Device %s: audiodev default parameter is deprecated, please "
-                  "specify audiodev=%s\n", name,
-                  QTAILQ_FIRST(&audio_states)->dev->id);
-        }
         return QTAILQ_FIRST(&audio_states);
     } else {
-        /* legacy implicit initialization */
-        head = audio_handle_legacy_opts();
-        /*
-         * In case of legacy initialization, all Audiodevs in the list will have
-         * the same configuration (except the driver), so it doesn't matter which
-         * one we chose.  We need an Audiodev to set up AudioState before we can
-         * init a driver.  Also note that dev at this point is still in the
-         * list.
-         */
-        dev = QSIMPLEQ_FIRST(&head)->dev;
-        audio_validate_opts(dev, &error_abort);
+        dev = g_new0(Audiodev, 1);
+        dev->id = g_strdup("none");
+        dev->driver = AUDIODEV_DRIVER_NONE;
+
+        audio_create_pdos(dev);
     }
 
     s = g_new0(AudioState, 1);
diff --git a/audio/audio.h b/audio/audio.h
index 8ee0e2159a..9f7ad056da 100644
--- a/audio/audio.h
+++ b/audio/audio.h
@@ -169,7 +169,6 @@ void audio_define(Audiodev *audio);
 void audio_parse_option(const char *opt);
 bool audio_init_audiodevs(void);
 void audio_help(void);
-void audio_legacy_help(void);
 
 AudioState *audio_state_by_name(const char *name);
 const char *audio_get_id(QEMUSoundCard *card);
diff --git a/audio/audio_legacy.c b/audio/audio_legacy.c
deleted file mode 100644
index 18a89ffffb..0000000000
--- a/audio/audio_legacy.c
+++ /dev/null
@@ -1,552 +0,0 @@
-/*
- * QEMU Audio subsystem: legacy configuration handling
- *
- * Copyright (c) 2015-2019 Zoltán Kővágó <DirtY.iCE.hu@gmail.com>
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
- * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
- * THE SOFTWARE.
- */
-#include "qemu/osdep.h"
-#include "audio.h"
-#include "audio_int.h"
-#include "qemu/cutils.h"
-#include "qemu/timer.h"
-#include "qapi/error.h"
-#include "qapi/qapi-visit-audio.h"
-#include "qapi/visitor-impl.h"
-
-#define AUDIO_CAP "audio-legacy"
-#include "audio_int.h"
-
-static uint32_t toui32(const char *str)
-{
-    unsigned long long ret;
-    if (parse_uint_full(str, &ret, 10) || ret > UINT32_MAX) {
-        dolog("Invalid integer value `%s'\n", str);
-        exit(1);
-    }
-    return ret;
-}
-
-/* helper functions to convert env variables */
-static void get_bool(const char *env, bool *dst, bool *has_dst)
-{
-    const char *val = getenv(env);
-    if (val) {
-        *dst = toui32(val) != 0;
-        *has_dst = true;
-    }
-}
-
-static void get_int(const char *env, uint32_t *dst, bool *has_dst)
-{
-    const char *val = getenv(env);
-    if (val) {
-        *dst = toui32(val);
-        *has_dst = true;
-    }
-}
-
-static void get_str(const char *env, char **dst)
-{
-    const char *val = getenv(env);
-    if (val) {
-        g_free(*dst);
-        *dst = g_strdup(val);
-    }
-}
-
-static void get_fmt(const char *env, AudioFormat *dst, bool *has_dst)
-{
-    const char *val = getenv(env);
-    if (val) {
-        size_t i;
-        for (i = 0; AudioFormat_lookup.size; ++i) {
-            if (strcasecmp(val, AudioFormat_lookup.array[i]) == 0) {
-                *dst = i;
-                *has_dst = true;
-                return;
-            }
-        }
-
-        dolog("Invalid audio format `%s'\n", val);
-        exit(1);
-    }
-}
-
-
-static void get_millis_to_usecs(const char *env, uint32_t *dst, bool *has_dst)
-{
-    const char *val = getenv(env);
-    if (val) {
-        *dst = toui32(val) * 1000;
-        *has_dst = true;
-    }
-}
-
-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;
-}
-
-
-static void get_frames_to_usecs(const char *env, uint32_t *dst, bool *has_dst,
-                                AudiodevPerDirectionOptions *pdo)
-{
-    const char *val = getenv(env);
-    if (val) {
-        *dst = frames_to_usecs(toui32(val), pdo);
-        *has_dst = true;
-    }
-}
-
-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);
-}
-
-static void get_samples_to_usecs(const char *env, uint32_t *dst, bool *has_dst,
-                                 AudiodevPerDirectionOptions *pdo)
-{
-    const char *val = getenv(env);
-    if (val) {
-        *dst = samples_to_usecs(toui32(val), pdo);
-        *has_dst = true;
-    }
-}
-
-static uint32_t bytes_to_usecs(uint32_t bytes, AudiodevPerDirectionOptions *pdo)
-{
-    AudioFormat fmt = pdo->has_format ? pdo->format : AUDIO_FORMAT_S16;
-    uint32_t bytes_per_sample = audioformat_bytes_per_sample(fmt);
-    return samples_to_usecs(bytes / bytes_per_sample, pdo);
-}
-
-static void get_bytes_to_usecs(const char *env, uint32_t *dst, bool *has_dst,
-                               AudiodevPerDirectionOptions *pdo)
-{
-    const char *val = getenv(env);
-    if (val) {
-        *dst = bytes_to_usecs(toui32(val), pdo);
-        *has_dst = true;
-    }
-}
-
-/* backend specific functions */
-/* ALSA */
-static void handle_alsa_per_direction(
-    AudiodevAlsaPerDirectionOptions *apdo, const char *prefix)
-{
-    char buf[64];
-    size_t len = strlen(prefix);
-    bool size_in_usecs = false;
-    bool dummy;
-
-    memcpy(buf, prefix, len);
-    strcpy(buf + len, "TRY_POLL");
-    get_bool(buf, &apdo->try_poll, &apdo->has_try_poll);
-
-    strcpy(buf + len, "DEV");
-    get_str(buf, &apdo->dev);
-
-    strcpy(buf + len, "SIZE_IN_USEC");
-    get_bool(buf, &size_in_usecs, &dummy);
-
-    strcpy(buf + len, "PERIOD_SIZE");
-    get_int(buf, &apdo->period_length, &apdo->has_period_length);
-    if (apdo->has_period_length && !size_in_usecs) {
-        apdo->period_length = frames_to_usecs(
-            apdo->period_length,
-            qapi_AudiodevAlsaPerDirectionOptions_base(apdo));
-    }
-
-    strcpy(buf + len, "BUFFER_SIZE");
-    get_int(buf, &apdo->buffer_length, &apdo->has_buffer_length);
-    if (apdo->has_buffer_length && !size_in_usecs) {
-        apdo->buffer_length = frames_to_usecs(
-            apdo->buffer_length,
-            qapi_AudiodevAlsaPerDirectionOptions_base(apdo));
-    }
-}
-
-static void handle_alsa(Audiodev *dev)
-{
-    AudiodevAlsaOptions *aopt = &dev->u.alsa;
-    handle_alsa_per_direction(aopt->in, "QEMU_ALSA_ADC_");
-    handle_alsa_per_direction(aopt->out, "QEMU_ALSA_DAC_");
-
-    get_millis_to_usecs("QEMU_ALSA_THRESHOLD",
-                        &aopt->threshold, &aopt->has_threshold);
-}
-
-/* coreaudio */
-static void handle_coreaudio(Audiodev *dev)
-{
-    get_frames_to_usecs(
-        "QEMU_COREAUDIO_BUFFER_SIZE",
-        &dev->u.coreaudio.out->buffer_length,
-        &dev->u.coreaudio.out->has_buffer_length,
-        qapi_AudiodevCoreaudioPerDirectionOptions_base(dev->u.coreaudio.out));
-    get_int("QEMU_COREAUDIO_BUFFER_COUNT",
-            &dev->u.coreaudio.out->buffer_count,
-            &dev->u.coreaudio.out->has_buffer_count);
-}
-
-/* dsound */
-static void handle_dsound(Audiodev *dev)
-{
-    get_millis_to_usecs("QEMU_DSOUND_LATENCY_MILLIS",
-                        &dev->u.dsound.latency, &dev->u.dsound.has_latency);
-    get_bytes_to_usecs("QEMU_DSOUND_BUFSIZE_OUT",
-                       &dev->u.dsound.out->buffer_length,
-                       &dev->u.dsound.out->has_buffer_length,
-                       dev->u.dsound.out);
-    get_bytes_to_usecs("QEMU_DSOUND_BUFSIZE_IN",
-                       &dev->u.dsound.in->buffer_length,
-                       &dev->u.dsound.in->has_buffer_length,
-                       dev->u.dsound.in);
-}
-
-/* OSS */
-static void handle_oss_per_direction(
-    AudiodevOssPerDirectionOptions *opdo, const char *try_poll_env,
-    const char *dev_env)
-{
-    get_bool(try_poll_env, &opdo->try_poll, &opdo->has_try_poll);
-    get_str(dev_env, &opdo->dev);
-
-    get_bytes_to_usecs("QEMU_OSS_FRAGSIZE",
-                       &opdo->buffer_length, &opdo->has_buffer_length,
-                       qapi_AudiodevOssPerDirectionOptions_base(opdo));
-    get_int("QEMU_OSS_NFRAGS", &opdo->buffer_count,
-            &opdo->has_buffer_count);
-}
-
-static void handle_oss(Audiodev *dev)
-{
-    AudiodevOssOptions *oopt = &dev->u.oss;
-    handle_oss_per_direction(oopt->in, "QEMU_AUDIO_ADC_TRY_POLL",
-                             "QEMU_OSS_ADC_DEV");
-    handle_oss_per_direction(oopt->out, "QEMU_AUDIO_DAC_TRY_POLL",
-                             "QEMU_OSS_DAC_DEV");
-
-    get_bool("QEMU_OSS_MMAP", &oopt->try_mmap, &oopt->has_try_mmap);
-    get_bool("QEMU_OSS_EXCLUSIVE", &oopt->exclusive, &oopt->has_exclusive);
-    get_int("QEMU_OSS_POLICY", &oopt->dsp_policy, &oopt->has_dsp_policy);
-}
-
-/* pulseaudio */
-static void handle_pa_per_direction(
-    AudiodevPaPerDirectionOptions *ppdo, const char *env)
-{
-    get_str(env, &ppdo->name);
-}
-
-static void handle_pa(Audiodev *dev)
-{
-    handle_pa_per_direction(dev->u.pa.in, "QEMU_PA_SOURCE");
-    handle_pa_per_direction(dev->u.pa.out, "QEMU_PA_SINK");
-
-    get_samples_to_usecs(
-        "QEMU_PA_SAMPLES", &dev->u.pa.in->buffer_length,
-        &dev->u.pa.in->has_buffer_length,
-        qapi_AudiodevPaPerDirectionOptions_base(dev->u.pa.in));
-    get_samples_to_usecs(
-        "QEMU_PA_SAMPLES", &dev->u.pa.out->buffer_length,
-        &dev->u.pa.out->has_buffer_length,
-        qapi_AudiodevPaPerDirectionOptions_base(dev->u.pa.out));
-
-    get_str("QEMU_PA_SERVER", &dev->u.pa.server);
-}
-
-/* SDL */
-static void handle_sdl(Audiodev *dev)
-{
-    /* SDL is output only */
-    get_samples_to_usecs("QEMU_SDL_SAMPLES", &dev->u.sdl.out->buffer_length,
-        &dev->u.sdl.out->has_buffer_length,
-        qapi_AudiodevSdlPerDirectionOptions_base(dev->u.sdl.out));
-}
-
-/* wav */
-static void handle_wav(Audiodev *dev)
-{
-    get_int("QEMU_WAV_FREQUENCY",
-            &dev->u.wav.out->frequency, &dev->u.wav.out->has_frequency);
-    get_fmt("QEMU_WAV_FORMAT", &dev->u.wav.out->format,
-            &dev->u.wav.out->has_format);
-    get_int("QEMU_WAV_DAC_FIXED_CHANNELS",
-            &dev->u.wav.out->channels, &dev->u.wav.out->has_channels);
-    get_str("QEMU_WAV_PATH", &dev->u.wav.path);
-}
-
-/* general */
-static void handle_per_direction(
-    AudiodevPerDirectionOptions *pdo, const char *prefix)
-{
-    char buf[64];
-    size_t len = strlen(prefix);
-
-    memcpy(buf, prefix, len);
-    strcpy(buf + len, "FIXED_SETTINGS");
-    get_bool(buf, &pdo->fixed_settings, &pdo->has_fixed_settings);
-
-    strcpy(buf + len, "FIXED_FREQ");
-    get_int(buf, &pdo->frequency, &pdo->has_frequency);
-
-    strcpy(buf + len, "FIXED_FMT");
-    get_fmt(buf, &pdo->format, &pdo->has_format);
-
-    strcpy(buf + len, "FIXED_CHANNELS");
-    get_int(buf, &pdo->channels, &pdo->has_channels);
-
-    strcpy(buf + len, "VOICES");
-    get_int(buf, &pdo->voices, &pdo->has_voices);
-}
-
-static AudiodevListEntry *legacy_opt(const char *drvname)
-{
-    AudiodevListEntry *e = g_new0(AudiodevListEntry, 1);
-    e->dev = g_new0(Audiodev, 1);
-    e->dev->id = g_strdup(drvname);
-    e->dev->driver = qapi_enum_parse(
-        &AudiodevDriver_lookup, drvname, -1, &error_abort);
-
-    audio_create_pdos(e->dev);
-
-    handle_per_direction(audio_get_pdo_in(e->dev), "QEMU_AUDIO_ADC_");
-    handle_per_direction(audio_get_pdo_out(e->dev), "QEMU_AUDIO_DAC_");
-
-    /* Original description: Timer period in HZ (0 - use lowest possible) */
-    get_int("QEMU_AUDIO_TIMER_PERIOD",
-            &e->dev->timer_period, &e->dev->has_timer_period);
-    if (e->dev->has_timer_period && e->dev->timer_period) {
-        e->dev->timer_period = NANOSECONDS_PER_SECOND / 1000 /
-                               e->dev->timer_period;
-    }
-
-    switch (e->dev->driver) {
-    case AUDIODEV_DRIVER_ALSA:
-        handle_alsa(e->dev);
-        break;
-
-    case AUDIODEV_DRIVER_COREAUDIO:
-        handle_coreaudio(e->dev);
-        break;
-
-    case AUDIODEV_DRIVER_DSOUND:
-        handle_dsound(e->dev);
-        break;
-
-    case AUDIODEV_DRIVER_OSS:
-        handle_oss(e->dev);
-        break;
-
-    case AUDIODEV_DRIVER_PA:
-        handle_pa(e->dev);
-        break;
-
-    case AUDIODEV_DRIVER_SDL:
-        handle_sdl(e->dev);
-        break;
-
-    case AUDIODEV_DRIVER_WAV:
-        handle_wav(e->dev);
-        break;
-
-    default:
-        break;
-    }
-
-    return e;
-}
-
-AudiodevListHead audio_handle_legacy_opts(void)
-{
-    const char *drvname = getenv("QEMU_AUDIO_DRV");
-    AudiodevListHead head = QSIMPLEQ_HEAD_INITIALIZER(head);
-
-    if (drvname) {
-        AudiodevListEntry *e;
-        audio_driver *driver = audio_driver_lookup(drvname);
-        if (!driver) {
-            dolog("Unknown audio driver `%s'\n", drvname);
-            exit(1);
-        }
-        e = legacy_opt(drvname);
-        QSIMPLEQ_INSERT_TAIL(&head, e, next);
-    } else {
-        for (int i = 0; audio_prio_list[i]; i++) {
-            audio_driver *driver = audio_driver_lookup(audio_prio_list[i]);
-            if (driver && driver->can_be_default) {
-                AudiodevListEntry *e = legacy_opt(driver->name);
-                QSIMPLEQ_INSERT_TAIL(&head, e, next);
-            }
-        }
-        if (QSIMPLEQ_EMPTY(&head)) {
-            dolog("Internal error: no default audio driver available\n");
-            exit(1);
-        }
-    }
-
-    return head;
-}
-
-/* visitor to print -audiodev option */
-typedef struct {
-    Visitor visitor;
-
-    bool comma;
-    GList *path;
-} LegacyPrintVisitor;
-
-static bool lv_start_struct(Visitor *v, const char *name, void **obj,
-                            size_t size, Error **errp)
-{
-    LegacyPrintVisitor *lv = (LegacyPrintVisitor *) v;
-    lv->path = g_list_append(lv->path, g_strdup(name));
-    return true;
-}
-
-static void lv_end_struct(Visitor *v, void **obj)
-{
-    LegacyPrintVisitor *lv = (LegacyPrintVisitor *) v;
-    lv->path = g_list_delete_link(lv->path, g_list_last(lv->path));
-}
-
-static void lv_print_key(Visitor *v, const char *name)
-{
-    GList *e;
-    LegacyPrintVisitor *lv = (LegacyPrintVisitor *) v;
-    if (lv->comma) {
-        putchar(',');
-    } else {
-        lv->comma = true;
-    }
-
-    for (e = lv->path; e; e = e->next) {
-        if (e->data) {
-            printf("%s.", (const char *) e->data);
-        }
-    }
-
-    printf("%s=", name);
-}
-
-static bool lv_type_int64(Visitor *v, const char *name, int64_t *obj,
-                          Error **errp)
-{
-    lv_print_key(v, name);
-    printf("%" PRIi64, *obj);
-    return true;
-}
-
-static bool lv_type_uint64(Visitor *v, const char *name, uint64_t *obj,
-                           Error **errp)
-{
-    lv_print_key(v, name);
-    printf("%" PRIu64, *obj);
-    return true;
-}
-
-static bool lv_type_bool(Visitor *v, const char *name, bool *obj, Error **errp)
-{
-    lv_print_key(v, name);
-    printf("%s", *obj ? "on" : "off");
-    return true;
-}
-
-static bool lv_type_str(Visitor *v, const char *name, char **obj, Error **errp)
-{
-    const char *str = *obj;
-    lv_print_key(v, name);
-
-    while (*str) {
-        if (*str == ',') {
-            putchar(',');
-        }
-        putchar(*str++);
-    }
-    return true;
-}
-
-static void lv_complete(Visitor *v, void *opaque)
-{
-    LegacyPrintVisitor *lv = (LegacyPrintVisitor *) v;
-    assert(lv->path == NULL);
-}
-
-static void lv_free(Visitor *v)
-{
-    LegacyPrintVisitor *lv = (LegacyPrintVisitor *) v;
-
-    g_list_free_full(lv->path, g_free);
-    g_free(lv);
-}
-
-static Visitor *legacy_visitor_new(void)
-{
-    LegacyPrintVisitor *lv = g_new0(LegacyPrintVisitor, 1);
-
-    lv->visitor.start_struct = lv_start_struct;
-    lv->visitor.end_struct = lv_end_struct;
-    /* lists not supported */
-    lv->visitor.type_int64 = lv_type_int64;
-    lv->visitor.type_uint64 = lv_type_uint64;
-    lv->visitor.type_bool = lv_type_bool;
-    lv->visitor.type_str = lv_type_str;
-
-    lv->visitor.type = VISITOR_OUTPUT;
-    lv->visitor.complete = lv_complete;
-    lv->visitor.free = lv_free;
-
-    return &lv->visitor;
-}
-
-void audio_legacy_help(void)
-{
-    AudiodevListHead head;
-    AudiodevListEntry *e;
-
-    printf("Environment variable based configuration deprecated.\n");
-    printf("Please use the new -audiodev option.\n");
-
-    head = audio_handle_legacy_opts();
-    printf("\nEquivalent -audiodev to your current environment variables:\n");
-    if (!getenv("QEMU_AUDIO_DRV")) {
-        printf("(Since you didn't specify QEMU_AUDIO_DRV, I'll list all "
-               "possibilities)\n");
-    }
-
-    QSIMPLEQ_FOREACH(e, &head, next) {
-        Visitor *v;
-        Audiodev *dev = e->dev;
-        printf("-audiodev ");
-
-        v = legacy_visitor_new();
-        visit_type_Audiodev(v, NULL, &dev, &error_abort);
-        visit_free(v);
-
-        printf("\n");
-    }
-    audio_free_audiodev_list(&head);
-}
diff --git a/audio/meson.build b/audio/meson.build
index 34aed78342..95cacc20db 100644
--- a/audio/meson.build
+++ b/audio/meson.build
@@ -1,6 +1,5 @@
 softmmu_ss.add([spice_headers, files('audio.c')])
 softmmu_ss.add(files(
-  'audio_legacy.c',
   'mixeng.c',
   'noaudio.c',
   'wavaudio.c',
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 09269f55e6..f118105652 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -23,14 +23,6 @@ deprecated.
 System emulator command line arguments
 --------------------------------------
 
-``QEMU_AUDIO_`` environment variables and ``-audio-help`` (since 4.0)
-'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
-
-The ``-audiodev`` argument is now the preferred way to specify audio
-backend settings instead of environment variables.  To ease migration to
-the new format, the ``-audiodev-help`` option can be used to convert
-the current values of the environment variables to ``-audiodev`` options.
-
 Creating sound card devices without ``audiodev=`` property (since 4.2)
 ''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
 
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index 8a8e0faff0..ffdd4a6d5d 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -428,6 +428,14 @@ Supporting audio transfer over vnc without ``audiodev=`` property (removed in 8.
 When using vnc, you should specify an ``audiodev=`` property if you
 intend to allow clients to request audio transfer through the VNC protocol.
 
+``QEMU_AUDIO_`` environment variables and ``-audio-help`` (rwemoved in 8.0)
+'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+The ``-audiodev`` argument is now the preferred way to specify audio
+backend settings instead of environment variables.  To ease migration to
+the new format, the ``-audiodev-help`` option can be used to convert
+the current values of the environment variables to ``-audiodev`` options.
+
 QEMU Machine Protocol (QMP) commands
 ------------------------------------
 
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 9bd0e52d01..f7759ce367 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2852,10 +2852,6 @@ void qemu_init(int argc, char **argv)
                 }
                 break;
 #endif
-            case QEMU_OPTION_audio_help:
-                audio_legacy_help();
-                exit (0);
-                break;
             case QEMU_OPTION_audiodev:
                 audio_parse_option(optarg);
                 break;
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 5cb38f90da..24bae43bed 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -413,9 +413,6 @@ QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
          */
         prctl(PR_SET_PDEATHSIG, SIGKILL, 0, 0, 0);
 #endif /* __linux__ */
-        if (!g_setenv("QEMU_AUDIO_DRV", "none", true)) {
-            exit(1);
-        }
         execlp("/bin/sh", "sh", "-c", command, NULL);
         exit(1);
     }
-- 
2.38.1

Re: [PATCH 2/9] audio: remove special audio_calloc function

[Volker Rümelin]

Am 13.01.23 um 17:21 schrieb Daniel P. Berrangé:
> The audio_calloc function does various checks on the size and
> nmembers parameters to detect various error conditions. There
> are only 5 callers
>
>   * alsa_poll_helper: the pollfd count is small and bounded,
>   * audio_pcm_create_voice_pair_: allocating a single fixed
>     size struct
>   * audio_pcm_sw_alloc_resources_: samples could be negative
>     zero, or overflow, so needs a check
>   * audio_pcm_hw_add_new_: voice size could be zero for
>     backends that don't support audio input
>   * st_rate_start: allocating a single fixed size struct
>
> IOW, only two of the callers need special error checks and
> it is clearer if their respective checks are inlined. Thus
> audio_calloc can be eliminated.

Hi Daniel,

my patch series at 
https://lists.nongnu.org/archive/html/qemu-devel/2022-12/msg02895.html 
also removes audio_calloc(). There will be merge conflicts.

With best regards,
Volker

>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   audio/alsaaudio.c            |  6 +-----
>   audio/audio.c                | 20 --------------------
>   audio/audio_int.h            |  1 -
>   audio/audio_template.h       | 28 ++++++++++++++--------------
>   audio/mixeng.c               |  7 +------
>   tests/qtest/fuzz-sb16-test.c |  6 ++++--
>   6 files changed, 20 insertions(+), 48 deletions(-)
>
> diff --git a/audio/alsaaudio.c b/audio/alsaaudio.c
> index 714bfb6453..5f50dfa0bf 100644
> --- a/audio/alsaaudio.c
> +++ b/audio/alsaaudio.c
> @@ -222,11 +222,7 @@ static int alsa_poll_helper (snd_pcm_t *handle, struct pollhlp *hlp, int mask)
>           return -1;
>       }
>   
> -    pfds = audio_calloc ("alsa_poll_helper", count, sizeof (*pfds));
> -    if (!pfds) {
> -        dolog ("Could not initialize poll mode\n");
> -        return -1;
> -    }
> +    pfds = g_new0(struct pollfd, count);
>   
>       err = snd_pcm_poll_descriptors (handle, pfds, count);
>       if (err < 0) {
> diff --git a/audio/audio.c b/audio/audio.c
> index 7b4b957945..f397072a1f 100644
> --- a/audio/audio.c
> +++ b/audio/audio.c
> @@ -146,26 +146,6 @@ static inline int audio_bits_to_index (int bits)
>       }
>   }
>   
> -void *audio_calloc (const char *funcname, int nmemb, size_t size)
> -{
> -    int cond;
> -    size_t len;
> -
> -    len = nmemb * size;
> -    cond = !nmemb || !size;
> -    cond |= nmemb < 0;
> -    cond |= len < size;
> -
> -    if (audio_bug ("audio_calloc", cond)) {
> -        AUD_log (NULL, "%s passed invalid arguments to audio_calloc\n",
> -                 funcname);
> -        AUD_log (NULL, "nmemb=%d size=%zu (len=%zu)\n", nmemb, size, len);
> -        return NULL;
> -    }
> -
> -    return g_malloc0 (len);
> -}
> -
>   void AUD_vlog (const char *cap, const char *fmt, va_list ap)
>   {
>       if (cap) {
> diff --git a/audio/audio_int.h b/audio/audio_int.h
> index e87ce014a0..b0cc2cd390 100644
> --- a/audio/audio_int.h
> +++ b/audio/audio_int.h
> @@ -251,7 +251,6 @@ void audio_pcm_init_info (struct audio_pcm_info *info, struct audsettings *as);
>   void audio_pcm_info_clear_buf (struct audio_pcm_info *info, void *buf, int len);
>   
>   int audio_bug (const char *funcname, int cond);
> -void *audio_calloc (const char *funcname, int nmemb, size_t size);
>   
>   void audio_run(AudioState *s, const char *msg);
>   
> diff --git a/audio/audio_template.h b/audio/audio_template.h
> index 720a32e57e..564cbb1f01 100644
> --- a/audio/audio_template.h
> +++ b/audio/audio_template.h
> @@ -116,13 +116,20 @@ static int glue (audio_pcm_sw_alloc_resources_, TYPE) (SW *sw)
>       samples = (int64_t)sw->HWBUF->size * sw->ratio >> 32;
>   #endif
>   
> -    sw->buf = audio_calloc(__func__, samples, sizeof(struct st_sample));
> -    if (!sw->buf) {
> -        dolog ("Could not allocate buffer for `%s' (%d samples)\n",
> +    if (audio_bug(__func__, samples <= 0)) {
> +        dolog ("Could not allocate buffer for '%s', samples %d <= 0\n",
>                  SW_NAME (sw), samples);
>           return -1;
>       }
>   
> +    if (audio_bug(__func__, (SIZE_MAX / sizeof(struct st_sample) < samples))) {
> +        dolog ("Could not allocate buffer for '%s', samples %d overflows\n",
> +               SW_NAME (sw), samples);
> +        return -1;
> +    }
> +
> +    sw->buf = g_new0(struct st_sample, samples);
> +
>   #ifdef DAC
>       sw->rate = st_rate_start (sw->info.freq, sw->hw->info.freq);
>   #else
> @@ -264,13 +271,12 @@ static HW *glue(audio_pcm_hw_add_new_, TYPE)(AudioState *s,
>           return NULL;
>       }
>   
> -    hw = audio_calloc(__func__, 1, glue(drv->voice_size_, TYPE));
> -    if (!hw) {
> -        dolog ("Can not allocate voice `%s' size %d\n",
> -               drv->name, glue (drv->voice_size_, TYPE));
> +    if (audio_bug(__func__, glue(drv->voice_size_, TYPE) == 0)) {
> +        dolog ("Voice size is zero");
>           return NULL;
>       }
>   
> +    hw = g_malloc0(glue(drv->voice_size_, TYPE));
>       hw->s = s;
>       hw->pcm_ops = drv->pcm_ops;
>   
> @@ -398,12 +404,7 @@ static SW *glue(audio_pcm_create_voice_pair_, TYPE)(
>           hw_as = *as;
>       }
>   
> -    sw = audio_calloc(__func__, 1, sizeof(*sw));
> -    if (!sw) {
> -        dolog ("Could not allocate soft voice `%s' (%zu bytes)\n",
> -               sw_name ? sw_name : "unknown", sizeof (*sw));
> -        goto err1;
> -    }
> +    sw = g_new0(SW, 1);
>       sw->s = s;
>   
>       hw = glue(audio_pcm_hw_add_, TYPE)(s, &hw_as);
> @@ -424,7 +425,6 @@ err3:
>       glue (audio_pcm_hw_gc_, TYPE) (&hw);
>   err2:
>       g_free (sw);
> -err1:
>       return NULL;
>   }
>   
> diff --git a/audio/mixeng.c b/audio/mixeng.c
> index 100a306d6f..fe454e0725 100644
> --- a/audio/mixeng.c
> +++ b/audio/mixeng.c
> @@ -414,12 +414,7 @@ struct rate {
>    */
>   void *st_rate_start (int inrate, int outrate)
>   {
> -    struct rate *rate = audio_calloc(__func__, 1, sizeof(*rate));
> -
> -    if (!rate) {
> -        dolog ("Could not allocate resampler (%zu bytes)\n", sizeof (*rate));
> -        return NULL;
> -    }
> +    struct rate *rate = g_new0(struct rate, 1);
>   
>       rate->opos = 0;
>   
> diff --git a/tests/qtest/fuzz-sb16-test.c b/tests/qtest/fuzz-sb16-test.c
> index fc445b1871..a28b93be3a 100644
> --- a/tests/qtest/fuzz-sb16-test.c
> +++ b/tests/qtest/fuzz-sb16-test.c
> @@ -10,7 +10,8 @@
>   #include "libqtest.h"
>   
>   /*
> - * This used to trigger the assert in audio_calloc
> + * This used to trigger the audio_bug calls in
> + * audio_pcm_sw_alloc_resources
>    * https://bugs.launchpad.net/qemu/+bug/1910603
>    */
>   static void test_fuzz_sb16_0x1c(void)
> @@ -38,7 +39,8 @@ static void test_fuzz_sb16_0x91(void)
>   }
>   
>   /*
> - * This used to trigger the assert in audio_calloc
> + * This used to trigger the audio_bug calls in
> + * audio_pcm_sw_alloc_resources
>    * through command 0xd4
>    */
>   static void test_fuzz_sb16_0xd4(void)

Re: [PATCH 1/9] audio: don't check qemu_add_vm_change_state_handler failure

[Volker Rümelin]

Am 13.01.23 um 17:21 schrieb Daniel P. Berrangé:
> This function cannot fail since g_malloc0 aborts on OOM.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   audio/audio.c | 7 +------
>   1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/audio/audio.c b/audio/audio.c
> index d849a94a81..7b4b957945 100644
> --- a/audio/audio.c
> +++ b/audio/audio.c
> @@ -1712,7 +1712,6 @@ static AudioState *audio_init(Audiodev *dev, const char *name)
>       size_t i;
>       int done = 0;
>       const char *drvname = NULL;
> -    VMChangeStateEntry *e;
>       AudioState *s;
>       struct audio_driver *driver;
>       /* silence gcc warning about uninitialized variable */
> @@ -1830,11 +1829,7 @@ static AudioState *audio_init(Audiodev *dev, const char *name)
>           s->period_ticks = dev->timer_period * (int64_t)SCALE_US;
>       }
>   
> -    e = qemu_add_vm_change_state_handler (audio_vm_change_state_handler, s);
> -    if (!e) {
> -        dolog ("warning: Could not register change state handler\n"
> -               "(Audio can continue looping even after stopping the VM)\n");
> -    }
> +    qemu_add_vm_change_state_handler (audio_vm_change_state_handler, s);

checkpatch.pl doesn't work properly here. It should report:
ERROR: space prohibited between function name and open parenthesis '('

With this changed,

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

>   
>       QTAILQ_INSERT_TAIL(&audio_states, s, list);
>       QLIST_INIT (&s->card_head);

Re: [PATCH 2/9] audio: remove special audio_calloc function

[Daniel P. Berrangé]

On Sun, Jan 15, 2023 at 03:03:29PM +0100, Volker Rümelin wrote:
> Am 13.01.23 um 17:21 schrieb Daniel P. Berrangé:
> > The audio_calloc function does various checks on the size and
> > nmembers parameters to detect various error conditions. There
> > are only 5 callers
> > 
> >   * alsa_poll_helper: the pollfd count is small and bounded,
> >   * audio_pcm_create_voice_pair_: allocating a single fixed
> >     size struct
> >   * audio_pcm_sw_alloc_resources_: samples could be negative
> >     zero, or overflow, so needs a check
> >   * audio_pcm_hw_add_new_: voice size could be zero for
> >     backends that don't support audio input
> >   * st_rate_start: allocating a single fixed size struct
> > 
> > IOW, only two of the callers need special error checks and
> > it is clearer if their respective checks are inlined. Thus
> > audio_calloc can be eliminated.
> 
> Hi Daniel,
> 
> my patch series at
> https://lists.nongnu.org/archive/html/qemu-devel/2022-12/msg02895.html also
> removes audio_calloc(). There will be merge conflicts.

Ah, yes, sorry I missed that.  I've sent a few comments on your
impl. Consider this patch dropped.


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