[PATCH 00/18] RFC: Remove deprecated audio features

[PATCH 00/18] RFC: Remove deprecated audio features

[Martin Kletzander]

I wanted to deal with https://bugzilla.redhat.com/2043498 and I got a
suggesstion that removing deprecated features could actually make it
easier to propagate the error.  In the end (last patch) it turns out the
error is still just reported with error_fatal, so it probably is not
really needed, but I really wanted to dig into QEMU more and learn some
of the internals for quite some time now.  So I used the opportunity.
The one-liner ended up being an 18 patch series which was, for someone
who has just one commit in QEMU codebase, a pretty challenging task.
Although I tried my best to do things properly, I am not sure whether I
handled everything correctly, hence the RFC.

Any comments are very much appreciated.  Thanks and have a nice day ;)

Martin Kletzander (18):
  hw/audio: Remove -soundhw support
  hw/input/tsc210x: Extract common init code into new function
  hw/audio: Simplify hda audio init
  hw/audio/lm4549: Add errp error reporting to init function
  tests/qtest: Specify audiodev= and -audiodev
  ui/vnc: Require audiodev=
  Introduce machine's default-audiodev property
  audio: Add easy dummy audio initialiser
  hw/display/xlnx_dp.c: Add audiodev property
  hw/input/tsc210x.c: Support machine-default audiodev with fallback
  hw/arm: Support machine-default audiodev with fallback
  hw/ppc: Support machine-default audiodev with fallback
  audio: Make AUD_register_card fallible and require audiodev=
  audio: Require AudioState in AUD_add_capture
  audio: Be more strict during audio backend initialisation
  audio: Remove legacy audio environment variables and options
  audio: Remove unused can_be_default
  audio/spiceaudio: Fail initialisation when not using spice

 audio/alsaaudio.c                             |   1 -
 audio/audio.c                                 | 204 +++----
 audio/audio.h                                 |   5 +-
 audio/audio_int.h                             |   1 -
 audio/audio_legacy.c                          | 555 ------------------
 audio/coreaudio.m                             |   1 -
 audio/dbusaudio.c                             |   1 -
 audio/dsoundaudio.c                           |   1 -
 audio/jackaudio.c                             |   1 -
 audio/meson.build                             |   1 -
 audio/noaudio.c                               |   1 -
 audio/ossaudio.c                              |   1 -
 audio/paaudio.c                               |   1 -
 audio/sdlaudio.c                              |   1 -
 audio/spiceaudio.c                            |   3 +-
 audio/wavaudio.c                              |   1 -
 docs/about/deprecated.rst                     |  24 -
 docs/about/removed-features.rst               |  27 +
 docs/qdev-device-use.txt                      |  21 +-
 docs/replay.txt                               |   2 +-
 hw/arm/integratorcp.c                         |   8 +-
 hw/arm/musicpal.c                             |   8 +-
 hw/arm/omap2.c                                |  11 +-
 hw/arm/realview.c                             |   3 +
 hw/arm/spitz.c                                |  10 +-
 hw/arm/versatilepb.c                          |   3 +
 hw/arm/vexpress.c                             |   3 +
 hw/arm/xlnx-zcu102.c                          |   4 +
 hw/arm/z2.c                                   |  12 +-
 hw/audio/ac97.c                               |   9 +-
 hw/audio/adlib.c                              |   9 +-
 hw/audio/cs4231a.c                            |   8 +-
 hw/audio/es1370.c                             |   8 +-
 hw/audio/gus.c                                |   6 +-
 hw/audio/hda-codec.c                          |  37 +-
 hw/audio/intel-hda.c                          |  25 +-
 hw/audio/intel-hda.h                          |   2 +-
 hw/audio/lm4549.c                             |   7 +-
 hw/audio/lm4549.h                             |   3 +-
 hw/audio/meson.build                          |   1 -
 hw/audio/pcspk.c                              |  15 +-
 hw/audio/pl041.c                              |   2 +-
 hw/audio/sb16.c                               |   9 +-
 hw/audio/soundhw.c                            | 177 ------
 hw/audio/wm8750.c                             |   5 +-
 hw/core/machine.c                             |  23 +
 hw/display/xlnx_dp.c                          |  12 +-
 hw/input/tsc210x.c                            |  79 ++-
 hw/ppc/prep.c                                 |   4 +
 hw/usb/dev-audio.c                            |   5 +-
 include/hw/audio/soundhw.h                    |  15 -
 include/hw/boards.h                           |   1 +
 qemu-options.hx                               |  37 --
 .../codeconverter/test_regexps.py             |   1 -
 softmmu/qdev-monitor.c                        |   2 -
 softmmu/vl.c                                  |  10 -
 tests/qtest/ac97-test.c                       |   3 +-
 tests/qtest/es1370-test.c                     |   3 +-
 tests/qtest/fuzz/generic_fuzz_configs.h       |   6 +-
 tests/qtest/intel-hda-test.c                  |  15 +-
 ui/vnc.c                                      |  15 +-
 61 files changed, 329 insertions(+), 1140 deletions(-)
 delete mode 100644 audio/audio_legacy.c
 delete mode 100644 hw/audio/soundhw.c
 delete mode 100644 include/hw/audio/soundhw.h

-- 
2.35.1

[PATCH 01/18] hw/audio: Remove -soundhw support

[Martin Kletzander]

One thing I am not sure about is whether to keep the aliases of ac97 and
es1370 in the qdev_alias_table.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
 docs/about/deprecated.rst                     |   9 -
 docs/about/removed-features.rst               |  10 +
 docs/qdev-device-use.txt                      |  21 +--
 docs/replay.txt                               |   2 +-
 hw/audio/ac97.c                               |   3 -
 hw/audio/adlib.c                              |   2 -
 hw/audio/cs4231a.c                            |   2 -
 hw/audio/es1370.c                             |   3 -
 hw/audio/gus.c                                |   2 -
 hw/audio/intel-hda.c                          |  21 ---
 hw/audio/meson.build                          |   1 -
 hw/audio/pcspk.c                              |  11 --
 hw/audio/sb16.c                               |   3 -
 hw/audio/soundhw.c                            | 177 ------------------
 include/hw/audio/soundhw.h                    |  15 --
 qemu-options.hx                               |  27 ---
 .../codeconverter/test_regexps.py             |   1 -
 softmmu/qdev-monitor.c                        |   2 -
 softmmu/vl.c                                  |   6 -
 19 files changed, 19 insertions(+), 299 deletions(-)
 delete mode 100644 hw/audio/soundhw.c
 delete mode 100644 include/hw/audio/soundhw.h

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index cf02ef6821e4..7ba71ebd3435 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -39,15 +39,6 @@ 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.
 
-Creating sound card devices using ``-soundhw`` (since 5.1)
-''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
-
-Sound card devices should be created using ``-device`` instead.  The
-names are the same for most devices.  The exceptions are ``hda`` which
-needs two devices (``-device intel-hda -device hda-duplex``) and
-``pcspk`` which can be activated using ``-machine
-pcspk-audiodev=<name>``.
-
 ``-chardev`` backend aliases ``tty`` and ``parport`` (since 6.0)
 ''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
 
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index 4b831ea29176..086ba3edb042 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -336,6 +336,16 @@ for the RISC-V ``virt`` machine and ``sifive_u`` machine.
 The ``-no-quit`` was a synonym for ``-display ...,window-close=off`` which
 should be used instead.
 
+Creating sound card devices using ``-soundhw`` (removed in 7.1)
+'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+Sound card devices should be created using ``-device`` instead.  The
+names are the same for most devices.  The exceptions are ``hda`` which
+needs two devices (``-device intel-hda -device hda-duplex``) and
+``pcspk`` which can be activated using ``-machine
+pcspk-audiodev=<name>``.  And ``AC97`` and ``ES1370`` now have to be
+specified in uppercase.
+
 
 QEMU Machine Protocol (QMP) commands
 ------------------------------------
diff --git a/docs/qdev-device-use.txt b/docs/qdev-device-use.txt
index 240888933482..30e7eaa3e66d 100644
--- a/docs/qdev-device-use.txt
+++ b/docs/qdev-device-use.txt
@@ -311,21 +311,16 @@ constraints.
 
 Host and guest part of audio devices have always been separate.
 
-The old way to define guest audio devices is -soundhw C1,...
+Host side (backend) is defined using -audiodev with a specific driver:
 
-The new way is to define each guest audio device separately with
--device.
+    spice
+    pa
+    none
 
-Map from -soundhw sound card name to -device:
-
-    ac97        -device AC97
-    cs4231a     -device cs4231a,iobase=IOADDR,irq=IRQ,dma=DMA
-    es1370      -device ES1370
-    gus         -device gus,iobase=IOADDR,irq=IRQ,dma=DMA,freq=F
-    hda         -device intel-hda,msi=MSI -device hda-duplex
-    sb16        -device sb16,iobase=IOADDR,irq=IRQ,dma=DMA,dma16=DMA16,version=V
-    adlib       not yet available with -device
-    pcspk       not yet available with -device
+And each guest audio device is then defined with -device with
+audiodev=AUDIODEV_ID that refers to the audio backend above.  Exceptions are
+pcspk and adlib which are note yet available with -device and are part of a
+machine type.
 
 For PCI devices, you can add bus=PCI-BUS,addr=DEVFN to control the PCI
 device address, as usual.
diff --git a/docs/replay.txt b/docs/replay.txt
index 5b008ca4911f..c329767c148a 100644
--- a/docs/replay.txt
+++ b/docs/replay.txt
@@ -294,7 +294,7 @@ Audio devices
 
 Audio data is recorded and replay automatically. The command line for recording
 and replaying must contain identical specifications of audio hardware, e.g.:
- -soundhw ac97
+ -audiodev driver=pa -device ac97,audiodev=audio0
 
 Serial ports
 ------------
diff --git a/hw/audio/ac97.c b/hw/audio/ac97.c
index 3cb81310607f..fd0b3b97d5b5 100644
--- a/hw/audio/ac97.c
+++ b/hw/audio/ac97.c
@@ -18,7 +18,6 @@
  */
 
 #include "qemu/osdep.h"
-#include "hw/audio/soundhw.h"
 #include "audio/audio.h"
 #include "hw/pci/pci.h"
 #include "hw/qdev-properties.h"
@@ -1430,8 +1429,6 @@ static const TypeInfo ac97_info = {
 static void ac97_register_types (void)
 {
     type_register_static (&ac97_info);
-    deprecated_register_soundhw("ac97", "Intel 82801AA AC97 Audio",
-                                0, TYPE_AC97);
 }
 
 type_init (ac97_register_types)
diff --git a/hw/audio/adlib.c b/hw/audio/adlib.c
index 5f979b1487d1..ba1be6c8378d 100644
--- a/hw/audio/adlib.c
+++ b/hw/audio/adlib.c
@@ -25,7 +25,6 @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "qemu/module.h"
-#include "hw/audio/soundhw.h"
 #include "audio/audio.h"
 #include "hw/isa/isa.h"
 #include "hw/qdev-properties.h"
@@ -322,7 +321,6 @@ static const TypeInfo adlib_info = {
 static void adlib_register_types (void)
 {
     type_register_static (&adlib_info);
-    deprecated_register_soundhw("adlib", ADLIB_DESC, 1, TYPE_ADLIB);
 }
 
 type_init (adlib_register_types)
diff --git a/hw/audio/cs4231a.c b/hw/audio/cs4231a.c
index 0723e3943044..f510b862efbe 100644
--- a/hw/audio/cs4231a.c
+++ b/hw/audio/cs4231a.c
@@ -23,7 +23,6 @@
  */
 
 #include "qemu/osdep.h"
-#include "hw/audio/soundhw.h"
 #include "audio/audio.h"
 #include "hw/irq.h"
 #include "hw/isa/isa.h"
@@ -717,7 +716,6 @@ static const TypeInfo cs4231a_info = {
 static void cs4231a_register_types (void)
 {
     type_register_static (&cs4231a_info);
-    deprecated_register_soundhw("cs4231a", "CS4231A", 1, TYPE_CS4231A);
 }
 
 type_init (cs4231a_register_types)
diff --git a/hw/audio/es1370.c b/hw/audio/es1370.c
index 690458981471..62359b84f279 100644
--- a/hw/audio/es1370.c
+++ b/hw/audio/es1370.c
@@ -27,7 +27,6 @@
 #define SILENT_ES1370
 
 #include "qemu/osdep.h"
-#include "hw/audio/soundhw.h"
 #include "audio/audio.h"
 #include "hw/pci/pci.h"
 #include "migration/vmstate.h"
@@ -923,8 +922,6 @@ static const TypeInfo es1370_info = {
 static void es1370_register_types (void)
 {
     type_register_static (&es1370_info);
-    deprecated_register_soundhw("es1370", "ENSONIQ AudioPCI ES1370",
-                                0, TYPE_ES1370);
 }
 
 type_init (es1370_register_types)
diff --git a/hw/audio/gus.c b/hw/audio/gus.c
index 42f010b67193..c7710a684b88 100644
--- a/hw/audio/gus.c
+++ b/hw/audio/gus.c
@@ -25,7 +25,6 @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "qemu/module.h"
-#include "hw/audio/soundhw.h"
 #include "audio/audio.h"
 #include "hw/irq.h"
 #include "hw/isa/isa.h"
@@ -317,7 +316,6 @@ static const TypeInfo gus_info = {
 static void gus_register_types (void)
 {
     type_register_static (&gus_info);
-    deprecated_register_soundhw("gus", "Gravis Ultrasound GF1", 1, TYPE_GUS);
 }
 
 type_init (gus_register_types)
diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
index bc77e3d8c9dc..e77552363a4c 100644
--- a/hw/audio/intel-hda.c
+++ b/hw/audio/intel-hda.c
@@ -26,7 +26,6 @@
 #include "qemu/log.h"
 #include "qemu/module.h"
 #include "qemu/error-report.h"
-#include "hw/audio/soundhw.h"
 #include "intel-hda.h"
 #include "migration/vmstate.h"
 #include "intel-hda-defs.h"
@@ -1307,25 +1306,6 @@ static const TypeInfo hda_codec_device_type_info = {
     .class_init = hda_codec_device_class_init,
 };
 
-/*
- * create intel hda controller with codec attached to it,
- * so '-soundhw hda' works.
- */
-static int intel_hda_and_codec_init(PCIBus *bus)
-{
-    DeviceState *controller;
-    BusState *hdabus;
-    DeviceState *codec;
-
-    warn_report("'-soundhw hda' is deprecated, "
-                "please use '-device intel-hda -device hda-duplex' instead");
-    controller = DEVICE(pci_create_simple(bus, -1, "intel-hda"));
-    hdabus = QLIST_FIRST(&controller->child_bus);
-    codec = qdev_new("hda-duplex");
-    qdev_realize_and_unref(codec, hdabus, &error_fatal);
-    return 0;
-}
-
 static void intel_hda_register_types(void)
 {
     type_register_static(&hda_codec_bus_info);
@@ -1333,7 +1313,6 @@ static void intel_hda_register_types(void)
     type_register_static(&intel_hda_info_ich6);
     type_register_static(&intel_hda_info_ich9);
     type_register_static(&hda_codec_device_type_info);
-    pci_register_soundhw("hda", "Intel HD Audio", intel_hda_and_codec_init);
 }
 
 type_init(intel_hda_register_types)
diff --git a/hw/audio/meson.build b/hw/audio/meson.build
index e48a9fc73d57..746b5b98d198 100644
--- a/hw/audio/meson.build
+++ b/hw/audio/meson.build
@@ -1,4 +1,3 @@
-softmmu_ss.add(files('soundhw.c'))
 softmmu_ss.add(when: 'CONFIG_AC97', if_true: files('ac97.c'))
 softmmu_ss.add(when: 'CONFIG_ADLIB', if_true: files('fmopl.c', 'adlib.c'))
 softmmu_ss.add(when: 'CONFIG_CS4231', if_true: files('cs4231.c'))
diff --git a/hw/audio/pcspk.c b/hw/audio/pcspk.c
index dfc7ebca4e17..72bea5fb202a 100644
--- a/hw/audio/pcspk.c
+++ b/hw/audio/pcspk.c
@@ -24,7 +24,6 @@
 
 #include "qemu/osdep.h"
 #include "hw/isa/isa.h"
-#include "hw/audio/soundhw.h"
 #include "audio/audio.h"
 #include "qemu/module.h"
 #include "qemu/timer.h"
@@ -245,18 +244,8 @@ static const TypeInfo pcspk_info = {
     .class_init     = pcspk_class_initfn,
 };
 
-static int pcspk_audio_init_soundhw(ISABus *bus)
-{
-    PCSpkState *s = pcspk_state;
-
-    warn_report("'-soundhw pcspk' is deprecated, "
-                "please set a backend using '-machine pcspk-audiodev=<name>' instead");
-    return pcspk_audio_init(s);
-}
-
 static void pcspk_register(void)
 {
     type_register_static(&pcspk_info);
-    isa_register_soundhw("pcspk", "PC speaker", pcspk_audio_init_soundhw);
 }
 type_init(pcspk_register)
diff --git a/hw/audio/sb16.c b/hw/audio/sb16.c
index 2215386ddb2e..593da2478c14 100644
--- a/hw/audio/sb16.c
+++ b/hw/audio/sb16.c
@@ -23,7 +23,6 @@
  */
 
 #include "qemu/osdep.h"
-#include "hw/audio/soundhw.h"
 #include "audio/audio.h"
 #include "hw/irq.h"
 #include "hw/isa/isa.h"
@@ -1469,8 +1468,6 @@ static const TypeInfo sb16_info = {
 static void sb16_register_types (void)
 {
     type_register_static (&sb16_info);
-    deprecated_register_soundhw("sb16", "Creative Sound Blaster 16",
-                                1, TYPE_SB16);
 }
 
 type_init (sb16_register_types)
diff --git a/hw/audio/soundhw.c b/hw/audio/soundhw.c
deleted file mode 100644
index 173b674ff53a..000000000000
--- a/hw/audio/soundhw.c
+++ /dev/null
@@ -1,177 +0,0 @@
-/*
- * QEMU System Emulator
- *
- * Copyright (c) 2003-2008 Fabrice Bellard
- *
- * 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 "qemu/option.h"
-#include "qemu/help_option.h"
-#include "qemu/error-report.h"
-#include "qom/object.h"
-#include "hw/isa/isa.h"
-#include "hw/pci/pci.h"
-#include "hw/audio/soundhw.h"
-
-struct soundhw {
-    const char *name;
-    const char *descr;
-    const char *typename;
-    int enabled;
-    int isa;
-    union {
-        int (*init_isa) (ISABus *bus);
-        int (*init_pci) (PCIBus *bus);
-    } init;
-};
-
-static struct soundhw soundhw[9];
-static int soundhw_count;
-
-void isa_register_soundhw(const char *name, const char *descr,
-                          int (*init_isa)(ISABus *bus))
-{
-    assert(soundhw_count < ARRAY_SIZE(soundhw) - 1);
-    soundhw[soundhw_count].name = name;
-    soundhw[soundhw_count].descr = descr;
-    soundhw[soundhw_count].isa = 1;
-    soundhw[soundhw_count].init.init_isa = init_isa;
-    soundhw_count++;
-}
-
-void pci_register_soundhw(const char *name, const char *descr,
-                          int (*init_pci)(PCIBus *bus))
-{
-    assert(soundhw_count < ARRAY_SIZE(soundhw) - 1);
-    soundhw[soundhw_count].name = name;
-    soundhw[soundhw_count].descr = descr;
-    soundhw[soundhw_count].isa = 0;
-    soundhw[soundhw_count].init.init_pci = init_pci;
-    soundhw_count++;
-}
-
-void deprecated_register_soundhw(const char *name, const char *descr,
-                                 int isa, const char *typename)
-{
-    assert(soundhw_count < ARRAY_SIZE(soundhw) - 1);
-    soundhw[soundhw_count].name = name;
-    soundhw[soundhw_count].descr = descr;
-    soundhw[soundhw_count].isa = isa;
-    soundhw[soundhw_count].typename = typename;
-    soundhw_count++;
-}
-
-void select_soundhw(const char *optarg)
-{
-    struct soundhw *c;
-
-    if (is_help_option(optarg)) {
-    show_valid_cards:
-
-        if (soundhw_count) {
-             printf("Valid sound card names (comma separated):\n");
-             for (c = soundhw; c->name; ++c) {
-                 printf ("%-11s %s\n", c->name, c->descr);
-             }
-             printf("\n-soundhw all will enable all of the above\n");
-        } else {
-             printf("Machine has no user-selectable audio hardware "
-                    "(it may or may not have always-present audio hardware).\n");
-        }
-        exit(!is_help_option(optarg));
-    }
-    else {
-        size_t l;
-        const char *p;
-        char *e;
-        int bad_card = 0;
-
-        if (!strcmp(optarg, "all")) {
-            for (c = soundhw; c->name; ++c) {
-                c->enabled = 1;
-            }
-            return;
-        }
-
-        p = optarg;
-        while (*p) {
-            e = strchr(p, ',');
-            l = !e ? strlen(p) : (size_t) (e - p);
-
-            for (c = soundhw; c->name; ++c) {
-                if (!strncmp(c->name, p, l) && !c->name[l]) {
-                    c->enabled = 1;
-                    break;
-                }
-            }
-
-            if (!c->name) {
-                if (l > 80) {
-                    error_report("Unknown sound card name (too big to show)");
-                }
-                else {
-                    error_report("Unknown sound card name `%.*s'",
-                                 (int) l, p);
-                }
-                bad_card = 1;
-            }
-            p += l + (e != NULL);
-        }
-
-        if (bad_card) {
-            goto show_valid_cards;
-        }
-    }
-}
-
-void soundhw_init(void)
-{
-    struct soundhw *c;
-    ISABus *isa_bus = (ISABus *) object_resolve_path_type("", TYPE_ISA_BUS, NULL);
-    PCIBus *pci_bus = (PCIBus *) object_resolve_path_type("", TYPE_PCI_BUS, NULL);
-
-    for (c = soundhw; c->name; ++c) {
-        if (c->enabled) {
-            if (c->typename) {
-                warn_report("'-soundhw %s' is deprecated, "
-                            "please use '-device %s' instead",
-                            c->name, c->typename);
-                if (c->isa) {
-                    isa_create_simple(isa_bus, c->typename);
-                } else {
-                    pci_create_simple(pci_bus, -1, c->typename);
-                }
-            } else if (c->isa) {
-                if (!isa_bus) {
-                    error_report("ISA bus not available for %s", c->name);
-                    exit(1);
-                }
-                c->init.init_isa(isa_bus);
-            } else {
-                if (!pci_bus) {
-                    error_report("PCI bus not available for %s", c->name);
-                    exit(1);
-                }
-                c->init.init_pci(pci_bus);
-            }
-        }
-    }
-}
-
diff --git a/include/hw/audio/soundhw.h b/include/hw/audio/soundhw.h
deleted file mode 100644
index f09a297854af..000000000000
--- a/include/hw/audio/soundhw.h
+++ /dev/null
@@ -1,15 +0,0 @@
-#ifndef HW_SOUNDHW_H
-#define HW_SOUNDHW_H
-
-void isa_register_soundhw(const char *name, const char *descr,
-                          int (*init_isa)(ISABus *bus));
-
-void pci_register_soundhw(const char *name, const char *descr,
-                          int (*init_pci)(PCIBus *bus));
-void deprecated_register_soundhw(const char *name, const char *descr,
-                                 int isa, const char *typename);
-
-void soundhw_init(void);
-void select_soundhw(const char *optarg);
-
-#endif
diff --git a/qemu-options.hx b/qemu-options.hx
index 34e9b32a5c00..47bbd86a4533 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -892,33 +892,6 @@ SRST
         ``qemu.wav``.
 ERST
 
-DEF("soundhw", HAS_ARG, QEMU_OPTION_soundhw,
-    "-soundhw c1,... enable audio support\n"
-    "                and only specified sound cards (comma separated list)\n"
-    "                use '-soundhw help' to get the list of supported cards\n"
-    "                use '-soundhw all' to enable all of them\n", QEMU_ARCH_ALL)
-SRST
-``-soundhw card1[,card2,...] or -soundhw all``
-    Enable audio and selected sound hardware. Use 'help' to print all
-    available sound hardware. For example:
-
-    .. parsed-literal::
-
-        |qemu_system_x86| -soundhw sb16,adlib disk.img
-        |qemu_system_x86| -soundhw es1370 disk.img
-        |qemu_system_x86| -soundhw ac97 disk.img
-        |qemu_system_x86| -soundhw hda disk.img
-        |qemu_system_x86| -soundhw all disk.img
-        |qemu_system_x86| -soundhw help
-
-    Note that Linux's i810\_audio OSS kernel (for AC97) module might
-    require manually specifying clocking.
-
-    ::
-
-        modprobe i810_audio clocking=48000
-ERST
-
 DEF("device", HAS_ARG, QEMU_OPTION_device,
     "-device driver[,prop[=value][,...]]\n"
     "                add device (based on driver)\n"
diff --git a/scripts/codeconverter/codeconverter/test_regexps.py b/scripts/codeconverter/codeconverter/test_regexps.py
index a445634d88ac..0161cadffee6 100644
--- a/scripts/codeconverter/codeconverter/test_regexps.py
+++ b/scripts/codeconverter/codeconverter/test_regexps.py
@@ -264,7 +264,6 @@ def test_initial_includes():
 #define SILENT_ES1370
 
 #include "qemu/osdep.h"
-#include "hw/audio/soundhw.h"
 #include "audio/audio.h"
 #include "hw/pci/pci.h"
 #include "migration/vmstate.h"
diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
index 12fe60c4670d..5c80d135ebf0 100644
--- a/softmmu/qdev-monitor.c
+++ b/softmmu/qdev-monitor.c
@@ -66,9 +66,7 @@ typedef struct QDevAlias
 
 /* Please keep this table sorted by typename. */
 static const QDevAlias qdev_alias_table[] = {
-    { "AC97", "ac97" }, /* -soundhw name */
     { "e1000", "e1000-82540em" },
-    { "ES1370", "es1370" }, /* -soundhw name */
     { "ich9-ahci", "ahci" },
     { "lsi53c895a", "lsi" },
     { "virtio-9p-device", "virtio-9p", QEMU_ARCH_VIRTIO_MMIO },
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 06a0e342fe9e..2f7158574e4f 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -80,7 +80,6 @@
 #include "migration/snapshot.h"
 #include "sysemu/tpm.h"
 #include "sysemu/dma.h"
-#include "hw/audio/soundhw.h"
 #include "audio/audio.h"
 #include "sysemu/cpus.h"
 #include "sysemu/cpu-timers.h"
@@ -2669,8 +2668,6 @@ static void qemu_create_cli_devices(void)
 {
     DeviceOption *opt;
 
-    soundhw_init();
-
     qemu_opts_foreach(qemu_find_opts("fw_cfg"),
                       parse_fw_cfg, fw_cfg_find(), &error_fatal);
 
@@ -3018,9 +3015,6 @@ void qemu_init(int argc, char **argv, char **envp)
             case QEMU_OPTION_audiodev:
                 audio_parse_option(optarg);
                 break;
-            case QEMU_OPTION_soundhw:
-                select_soundhw (optarg);
-                break;
             case QEMU_OPTION_h:
                 help(0);
                 break;
-- 
2.35.1

[PATCH 02/18] hw/input/tsc210x: Extract common init code into new function

[Martin Kletzander]

This deduplicates several lines and will make future changes more
concise.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
 hw/input/tsc210x.c | 68 ++++++++++++++++------------------------------
 1 file changed, 24 insertions(+), 44 deletions(-)

diff --git a/hw/input/tsc210x.c b/hw/input/tsc210x.c
index df7313db5d7f..f16a8090b7c7 100644
--- a/hw/input/tsc210x.c
+++ b/hw/input/tsc210x.c
@@ -31,6 +31,7 @@
 #include "hw/input/tsc2xxx.h"
 #include "hw/irq.h"
 #include "migration/vmstate.h"
+#include "qapi/error.h"
 
 #define TSC_DATA_REGISTERS_PAGE		0x0
 #define TSC_CONTROL_REGISTERS_PAGE	0x1
@@ -1070,20 +1071,10 @@ static const VMStateDescription vmstate_tsc2301 = {
     .fields = vmstatefields_tsc210x,
 };
 
-uWireSlave *tsc2102_init(qemu_irq pint)
+static void tsc210x_init(TSC210xState *s,
+                         const char *name,
+                         const VMStateDescription *vmsd)
 {
-    TSC210xState *s;
-
-    s = g_new0(TSC210xState, 1);
-    s->x = 160;
-    s->y = 160;
-    s->pressure = 0;
-    s->precision = s->nextprecision = 0;
-    s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, tsc210x_timer_tick, s);
-    s->pint = pint;
-    s->model = 0x2102;
-    s->name = "tsc2102";
-
     s->tr[0] = 0;
     s->tr[1] = 1;
     s->tr[2] = 1;
@@ -1105,13 +1096,29 @@ uWireSlave *tsc2102_init(qemu_irq pint)
 
     tsc210x_reset(s);
 
-    qemu_add_mouse_event_handler(tsc210x_touchscreen_event, s, 1,
-                    "QEMU TSC2102-driven Touchscreen");
+    qemu_add_mouse_event_handler(tsc210x_touchscreen_event, s, 1, name);
 
     AUD_register_card(s->name, &s->card);
 
     qemu_register_reset((void *) tsc210x_reset, s);
-    vmstate_register(NULL, 0, &vmstate_tsc2102, s);
+    vmstate_register(NULL, 0, vmsd, s);
+}
+
+uWireSlave *tsc2102_init(qemu_irq pint)
+{
+    TSC210xState *s;
+
+    s = g_new0(TSC210xState, 1);
+    s->x = 160;
+    s->y = 160;
+    s->pressure = 0;
+    s->precision = s->nextprecision = 0;
+    s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, tsc210x_timer_tick, s);
+    s->pint = pint;
+    s->model = 0x2102;
+    s->name = "tsc2102";
+
+    tsc210x_init(s, "QEMU TSC2102-driven Touchscreen", &vmstate_tsc2102);
 
     return &s->chip;
 }
@@ -1132,34 +1139,7 @@ uWireSlave *tsc2301_init(qemu_irq penirq, qemu_irq kbirq, qemu_irq dav)
     s->model = 0x2301;
     s->name = "tsc2301";
 
-    s->tr[0] = 0;
-    s->tr[1] = 1;
-    s->tr[2] = 1;
-    s->tr[3] = 0;
-    s->tr[4] = 1;
-    s->tr[5] = 0;
-    s->tr[6] = 1;
-    s->tr[7] = 0;
-
-    s->chip.opaque = s;
-    s->chip.send = (void *) tsc210x_write;
-    s->chip.receive = (void *) tsc210x_read;
-
-    s->codec.opaque = s;
-    s->codec.tx_swallow = (void *) tsc210x_i2s_swallow;
-    s->codec.set_rate = (void *) tsc210x_i2s_set_rate;
-    s->codec.in.fifo = s->in_fifo;
-    s->codec.out.fifo = s->out_fifo;
-
-    tsc210x_reset(s);
-
-    qemu_add_mouse_event_handler(tsc210x_touchscreen_event, s, 1,
-                    "QEMU TSC2301-driven Touchscreen");
-
-    AUD_register_card(s->name, &s->card);
-
-    qemu_register_reset((void *) tsc210x_reset, s);
-    vmstate_register(NULL, 0, &vmstate_tsc2301, s);
+    tsc210x_init(s, "QEMU TSC2301-driven Touchscreen", &vmstate_tsc2301);
 
     return &s->chip;
 }
-- 
2.35.1

[PATCH 03/18] hw/audio: Simplify hda audio init

[Martin Kletzander]

No return values are used anywhere, so switch the functions to be void
and add support for error reporting using errp for use in next patches.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
 hw/audio/hda-codec.c | 32 ++++++++++++++++++--------------
 hw/audio/intel-hda.c |  4 +---
 hw/audio/intel-hda.h |  2 +-
 3 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/hw/audio/hda-codec.c b/hw/audio/hda-codec.c
index feb8f9e2bb7a..e86a2adf31a0 100644
--- a/hw/audio/hda-codec.c
+++ b/hw/audio/hda-codec.c
@@ -676,7 +676,9 @@ static void hda_audio_stream(HDACodecDevice *hda, uint32_t stnr, bool running, b
     }
 }
 
-static int hda_audio_init(HDACodecDevice *hda, const struct desc_codec *desc)
+static void hda_audio_init(HDACodecDevice *hda,
+                           const struct desc_codec *desc,
+                           Error **errp)
 {
     HDAAudioState *a = HDA_AUDIO(hda);
     HDAAudioStream *st;
@@ -719,7 +721,6 @@ static int hda_audio_init(HDACodecDevice *hda, const struct desc_codec *desc)
             break;
         }
     }
-    return 0;
 }
 
 static void hda_audio_exit(HDACodecDevice *hda)
@@ -849,37 +850,40 @@ static Property hda_audio_properties[] = {
     DEFINE_PROP_END_OF_LIST(),
 };
 
-static int hda_audio_init_output(HDACodecDevice *hda)
+static void hda_audio_init_output(HDACodecDevice *hda, Error **errp)
 {
     HDAAudioState *a = HDA_AUDIO(hda);
+    const struct desc_codec *desc = &output_nomixemu;
 
     if (!a->mixer) {
-        return hda_audio_init(hda, &output_nomixemu);
-    } else {
-        return hda_audio_init(hda, &output_mixemu);
+        desc = &output_mixemu;
     }
+
+    hda_audio_init(hda, desc, errp);
 }
 
-static int hda_audio_init_duplex(HDACodecDevice *hda)
+static void hda_audio_init_duplex(HDACodecDevice *hda, Error **errp)
 {
     HDAAudioState *a = HDA_AUDIO(hda);
+    const struct desc_codec *desc = &duplex_nomixemu;
 
     if (!a->mixer) {
-        return hda_audio_init(hda, &duplex_nomixemu);
-    } else {
-        return hda_audio_init(hda, &duplex_mixemu);
+        desc = &duplex_mixemu;
     }
+
+    hda_audio_init(hda, desc, errp);
 }
 
-static int hda_audio_init_micro(HDACodecDevice *hda)
+static void hda_audio_init_micro(HDACodecDevice *hda, Error **errp)
 {
     HDAAudioState *a = HDA_AUDIO(hda);
+    const struct desc_codec *desc = &micro_nomixemu;
 
     if (!a->mixer) {
-        return hda_audio_init(hda, &micro_nomixemu);
-    } else {
-        return hda_audio_init(hda, &micro_mixemu);
+        desc = &micro_mixemu;
     }
+
+    hda_audio_init(hda, desc, errp);
 }
 
 static void hda_audio_base_class_init(ObjectClass *klass, void *data)
diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
index e77552363a4c..a17002812240 100644
--- a/hw/audio/intel-hda.c
+++ b/hw/audio/intel-hda.c
@@ -70,9 +70,7 @@ static void hda_codec_dev_realize(DeviceState *qdev, Error **errp)
         return;
     }
     bus->next_cad = dev->cad + 1;
-    if (cdc->init(dev) != 0) {
-        error_setg(errp, "HDA audio init failed");
-    }
+    cdc->init(dev, errp);
 }
 
 static void hda_codec_dev_unrealize(DeviceState *qdev)
diff --git a/hw/audio/intel-hda.h b/hw/audio/intel-hda.h
index f78c1833e341..8d710eee5d66 100644
--- a/hw/audio/intel-hda.h
+++ b/hw/audio/intel-hda.h
@@ -31,7 +31,7 @@ struct HDACodecBus {
 struct HDACodecDeviceClass {
     DeviceClass parent_class;
 
-    int (*init)(HDACodecDevice *dev);
+    void (*init)(HDACodecDevice *dev, Error **errp);
     void (*exit)(HDACodecDevice *dev);
     void (*command)(HDACodecDevice *dev, uint32_t nid, uint32_t data);
     void (*stream)(HDACodecDevice *dev, uint32_t stnr, bool running, bool output);
-- 
2.35.1

[PATCH 04/18] hw/audio/lm4549: Add errp error reporting to init function

[Martin Kletzander]

This will be used in future commit.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
 hw/audio/lm4549.c | 3 ++-
 hw/audio/lm4549.h | 3 ++-
 hw/audio/pl041.c  | 2 +-
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/audio/lm4549.c b/hw/audio/lm4549.c
index 32b1481b5614..418041bc9c6c 100644
--- a/hw/audio/lm4549.c
+++ b/hw/audio/lm4549.c
@@ -276,7 +276,8 @@ static int lm4549_post_load(void *opaque, int version_id)
     return 0;
 }
 
-void lm4549_init(lm4549_state *s, lm4549_callback data_req_cb, void* opaque)
+void lm4549_init(lm4549_state *s, lm4549_callback data_req_cb, void* opaque,
+                 Error **errp)
 {
     struct audsettings as;
 
diff --git a/hw/audio/lm4549.h b/hw/audio/lm4549.h
index aba9bb5b077c..61c3ab12dd33 100644
--- a/hw/audio/lm4549.h
+++ b/hw/audio/lm4549.h
@@ -36,7 +36,8 @@ typedef struct {
 extern const VMStateDescription vmstate_lm4549_state;
 
 
-void lm4549_init(lm4549_state *s, lm4549_callback data_req, void *opaque);
+void lm4549_init(lm4549_state *s, lm4549_callback data_req, void *opaque,
+                 Error **errp);
 uint32_t lm4549_read(lm4549_state *s, hwaddr offset);
 void lm4549_write(lm4549_state *s, hwaddr offset, uint32_t value);
 uint32_t lm4549_write_samples(lm4549_state *s, uint32_t left, uint32_t right);
diff --git a/hw/audio/pl041.c b/hw/audio/pl041.c
index 03acd4fe344b..868dffbfd321 100644
--- a/hw/audio/pl041.c
+++ b/hw/audio/pl041.c
@@ -564,7 +564,7 @@ static void pl041_realize(DeviceState *dev, Error **errp)
     }
 
     /* Init the codec */
-    lm4549_init(&s->codec, &pl041_request_data, (void *)s);
+    lm4549_init(&s->codec, &pl041_request_data, (void *)s, errp);
 }
 
 static const VMStateDescription vmstate_pl041_regfile = {
-- 
2.35.1

[PATCH 05/18] tests/qtest: Specify audiodev= and -audiodev

[Martin Kletzander]

This will enable removing deprecated default audiodev support.

I did not figure out how to make the audiodev represented as an
interface node, so this is a workaround.  I am not sure what would be
the proper way.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
 tests/qtest/ac97-test.c                 |  3 ++-
 tests/qtest/es1370-test.c               |  3 ++-
 tests/qtest/fuzz/generic_fuzz_configs.h |  6 ++++--
 tests/qtest/intel-hda-test.c            | 15 ++++++++++-----
 4 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/tests/qtest/ac97-test.c b/tests/qtest/ac97-test.c
index e09f2495d24d..9711f1f6d966 100644
--- a/tests/qtest/ac97-test.c
+++ b/tests/qtest/ac97-test.c
@@ -45,7 +45,8 @@ static void *ac97_create(void *pci_bus, QGuestAllocator *alloc, void *addr)
 static void ac97_register_nodes(void)
 {
     QOSGraphEdgeOptions opts = {
-        .extra_device_opts = "addr=04.0",
+        .extra_device_opts = "addr=04.0,audiodev=audio0",
+        .before_cmd_line = "-audiodev driver=none,id=audio0",
     };
     add_qpci_address(&opts, &(QPCIAddress) { .devfn = QPCI_DEVFN(4, 0) });
 
diff --git a/tests/qtest/es1370-test.c b/tests/qtest/es1370-test.c
index 2fd7fd2d3d30..5facda8d0d8d 100644
--- a/tests/qtest/es1370-test.c
+++ b/tests/qtest/es1370-test.c
@@ -46,7 +46,8 @@ static void *es1370_create(void *pci_bus, QGuestAllocator *alloc, void *addr)
 static void es1370_register_nodes(void)
 {
     QOSGraphEdgeOptions opts = {
-        .extra_device_opts = "addr=04.0",
+        .extra_device_opts = "addr=04.0,audiodev=audio0",
+        .before_cmd_line = "-audiodev driver=none,id=audio0",
     };
     add_qpci_address(&opts, &(QPCIAddress) { .devfn = QPCI_DEVFN(4, 0) });
 
diff --git a/tests/qtest/fuzz/generic_fuzz_configs.h b/tests/qtest/fuzz/generic_fuzz_configs.h
index 004c701915e1..84a93b3c350b 100644
--- a/tests/qtest/fuzz/generic_fuzz_configs.h
+++ b/tests/qtest/fuzz/generic_fuzz_configs.h
@@ -101,8 +101,10 @@ const generic_fuzz_config predefined_configs[] = {
     },{
         .name = "intel-hda",
         .args = "-machine q35 -nodefaults -device intel-hda,id=hda0 "
-        "-device hda-output,bus=hda0.0 -device hda-micro,bus=hda0.0 "
-        "-device hda-duplex,bus=hda0.0",
+        "-audiodev driver=none,id=audio0",
+        "-device hda-output,bus=hda0.0,audiodev=audio0 "
+        "-device hda-micro,bus=hda0.0,audiodev=audio0 "
+        "-device hda-duplex,bus=hda0.0,audiodev=audio0",
         .objects = "intel-hda",
     },{
         .name = "ide-hd",
diff --git a/tests/qtest/intel-hda-test.c b/tests/qtest/intel-hda-test.c
index a58c98e4d11b..39ced2bc6ac6 100644
--- a/tests/qtest/intel-hda-test.c
+++ b/tests/qtest/intel-hda-test.c
@@ -11,20 +11,24 @@
 #include "libqtest-single.h"
 
 #define HDA_ID "hda0"
-#define CODEC_DEVICES " -device hda-output,bus=" HDA_ID ".0" \
-                      " -device hda-micro,bus=" HDA_ID ".0" \
-                      " -device hda-duplex,bus=" HDA_ID ".0"
+#define AUDIODEV " -audiodev driver=none,id=audio0 "
+#define AUDIODEV_REF "audiodev=audio0"
+#define CODEC_DEVICES " -device hda-output,bus=" HDA_ID ".0," AUDIODEV_REF \
+                      " -device hda-micro,bus=" HDA_ID ".0," AUDIODEV_REF \
+                      " -device hda-duplex,bus=" HDA_ID ".0," AUDIODEV_REF
 
 /* Tests only initialization so far. TODO: Replace with functional tests */
 static void ich6_test(void)
 {
-    qtest_start("-device intel-hda,id=" HDA_ID CODEC_DEVICES);
+    qtest_start(AUDIODEV "-device intel-hda,id=" HDA_ID CODEC_DEVICES);
     qtest_end();
 }
 
 static void ich9_test(void)
 {
-    qtest_start("-machine q35 -device ich9-intel-hda,bus=pcie.0,addr=1b.0,id="
+    qtest_start("-machine q35"
+                AUDIODEV
+                "-device ich9-intel-hda,bus=pcie.0,addr=1b.0,id="
                 HDA_ID CODEC_DEVICES);
     qtest_end();
 }
@@ -39,6 +43,7 @@ static void test_issue542_ich6(void)
     QTestState *s;
 
     s = qtest_init("-nographic -nodefaults -M pc-q35-6.2 "
+                   AUDIODEV
                    "-device intel-hda,id=" HDA_ID CODEC_DEVICES);
 
     qtest_outl(s, 0xcf8, 0x80000804);
-- 
2.35.1

[PATCH 06/18] ui/vnc: Require audiodev=

[Martin Kletzander]

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
 ui/vnc.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index badf1d7664fe..2e7af139b030 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -4188,12 +4188,15 @@ void vnc_display_open(const char *id, Error **errp)
     vd->ledstate = 0;
 
     audiodev = qemu_opt_get(opts, "audiodev");
-    if (audiodev) {
-        vd->audio_state = audio_state_by_name(audiodev);
-        if (!vd->audio_state) {
-            error_setg(errp, "Audiodev '%s' not found", audiodev);
-            goto fail;
-        }
+    if (!audiodev) {
+        error_setg(errp, "Audiodev parameter for vnc required");
+        goto fail;
+    }
+
+    vd->audio_state = audio_state_by_name(audiodev);
+    if (!vd->audio_state) {
+        error_setg(errp, "Audiodev '%s' not found", audiodev);
+        goto fail;
     }
 
     device_id = qemu_opt_get(opts, "display");
-- 
2.35.1

[PATCH 07/18] Introduce machine's default-audiodev property

[Martin Kletzander]

Many machine types have default audio devices with no way to set the underlying
audiodev.  Instead of adding an option for each and every one of them this new
property can be used as a default during machine initialisation when creating
such devices.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
 hw/core/machine.c   | 23 +++++++++++++++++++++++
 include/hw/boards.h |  1 +
 2 files changed, 24 insertions(+)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index cb9bbc844d24..d055a126d398 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -596,6 +596,22 @@ static void machine_set_memdev(Object *obj, const char *value, Error **errp)
     ms->ram_memdev_id = g_strdup(value);
 }
 
+static char *machine_get_default_audiodev(Object *obj, Error **errp)
+{
+    MachineState *ms = MACHINE(obj);
+
+    return g_strdup(ms->default_audiodev);
+}
+
+static void machine_set_default_audiodev(Object *obj, const char *value,
+                                         Error **errp)
+{
+    MachineState *ms = MACHINE(obj);
+
+    g_free(ms->default_audiodev);
+    ms->default_audiodev = g_strdup(value);
+}
+
 HotpluggableCPUList *machine_query_hotpluggable_cpus(MachineState *machine)
 {
     int i;
@@ -867,6 +883,12 @@ static void machine_class_init(ObjectClass *oc, void *data)
     object_class_property_set_description(oc, "confidential-guest-support",
                                           "Set confidential guest scheme to support");
 
+    object_class_property_add_str(oc, "default-audiodev",
+                                  machine_get_default_audiodev,
+                                  machine_set_default_audiodev);
+    object_class_property_set_description(oc, "default-audiodev",
+                                          "Audiodev to use for default machine devices");
+
     /* For compatibility */
     object_class_property_add_str(oc, "memory-encryption",
         machine_get_memory_encryption, machine_set_memory_encryption);
@@ -961,6 +983,7 @@ static void machine_finalize(Object *obj)
     g_free(ms->device_memory);
     g_free(ms->nvdimms_state);
     g_free(ms->numa_state);
+    g_free(ms->default_audiodev);
 }
 
 bool machine_usb(MachineState *machine)
diff --git a/include/hw/boards.h b/include/hw/boards.h
index d64b5481e834..5be1de50af03 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -346,6 +346,7 @@ struct MachineState {
      */
     MemoryRegion *ram;
     DeviceMemoryState *device_memory;
+    char *default_audiodev;
 
     ram_addr_t ram_size;
     ram_addr_t maxram_size;
-- 
2.35.1

[PATCH 08/18] audio: Add easy dummy audio initialiser

[Martin Kletzander]

In case of sound devices which are created by default (some of them even with
-nodefaults) it would not be nice to users if qemu required explicit:

  -audiodev driver=none,id=audio0 -machine default-audiodev=audio0

when they just want to run a VM and not care about any audio output.  Instead
this little helper makes it easy to create a dummy audiodev (driver=none) in
case there is no default-audiodev specified for the machine.  To make sure users
are not surprised by no sound output a helping message is also printed out.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
 audio/audio.c | 34 ++++++++++++++++++++++++++++++++++
 audio/audio.h |  2 ++
 2 files changed, 36 insertions(+)

diff --git a/audio/audio.c b/audio/audio.c
index 9e91a5a4f2b8..671845c65d18 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -37,6 +37,7 @@
 #include "sysemu/runstate.h"
 #include "ui/qemu-spice.h"
 #include "trace.h"
+#include "hw/boards.h"
 
 #define AUDIO_CAP "audio"
 #include "audio_int.h"
@@ -2122,6 +2123,39 @@ void audio_init_audiodevs(void)
     }
 }
 
+static void audio_init_dummy(const char *id)
+{
+    Audiodev *dev = g_new0(Audiodev, 1);
+    AudiodevListEntry *e = g_new0(AudiodevListEntry, 1);
+
+    dev->driver = AUDIODEV_DRIVER_NONE;
+    dev->id = g_strdup(id);
+
+    audio_validate_opts(dev, &error_abort);
+    audio_init(dev, NULL);
+
+    e->dev = dev;
+    QSIMPLEQ_INSERT_TAIL(&audiodevs, e, next);
+}
+
+const char *audio_maybe_init_dummy(const char *default_id)
+{
+    MachineState *ms = MACHINE(qdev_get_machine());
+
+    if (ms->default_audiodev) {
+        return ms->default_audiodev;
+    }
+
+    dolog("warning: No audiodev specified for implicit machine devices, "
+          "no audio output will be available for these. "
+          "For setting a backend audiodev for such devices try using "
+          "the default-audiodev machine option.\n");
+
+    audio_init_dummy(default_id);
+
+    return default_id;
+}
+
 audsettings audiodev_to_audsettings(AudiodevPerDirectionOptions *pdo)
 {
     return (audsettings) {
diff --git a/audio/audio.h b/audio/audio.h
index 3d5ecdecd5c1..335704a4ddb1 100644
--- a/audio/audio.h
+++ b/audio/audio.h
@@ -175,6 +175,8 @@ void audio_legacy_help(void);
 AudioState *audio_state_by_name(const char *name);
 const char *audio_get_id(QEMUSoundCard *card);
 
+const char *audio_maybe_init_dummy(const char *default_id);
+
 #define DEFINE_AUDIO_PROPERTIES(_s, _f)         \
     DEFINE_PROP_AUDIODEV("audiodev", _s, _f)
 
-- 
2.35.1

[PATCH 09/18] hw/display/xlnx_dp.c: Add audiodev property

[Martin Kletzander]

There was no way to set this and we need that for it to be able to properly
initialise.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
 hw/display/xlnx_dp.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/hw/display/xlnx_dp.c b/hw/display/xlnx_dp.c
index 9bb781e31254..b16d6be2b5cc 100644
--- a/hw/display/xlnx_dp.c
+++ b/hw/display/xlnx_dp.c
@@ -1357,6 +1357,11 @@ static void xlnx_dp_reset(DeviceState *dev)
     xlnx_dp_update_irq(s);
 }
 
+static Property xlnx_dp_device_properties[] = {
+    DEFINE_AUDIO_PROPERTIES(XlnxDPState, aud_card),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 static void xlnx_dp_class_init(ObjectClass *oc, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(oc);
@@ -1364,6 +1369,7 @@ static void xlnx_dp_class_init(ObjectClass *oc, void *data)
     dc->realize = xlnx_dp_realize;
     dc->vmsd = &vmstate_dp;
     dc->reset = xlnx_dp_reset;
+    device_class_set_props(dc, xlnx_dp_device_properties);
 }
 
 static const TypeInfo xlnx_dp_info = {
-- 
2.35.1

[PATCH 10/18] hw/input/tsc210x.c: Support machine-default audiodev with fallback

[Martin Kletzander]

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
 hw/input/tsc210x.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/hw/input/tsc210x.c b/hw/input/tsc210x.c
index f16a8090b7c7..f0b02bc72280 100644
--- a/hw/input/tsc210x.c
+++ b/hw/input/tsc210x.c
@@ -1098,6 +1098,14 @@ static void tsc210x_init(TSC210xState *s,
 
     qemu_add_mouse_event_handler(tsc210x_touchscreen_event, s, 1, name);
 
+    const char *audiodev_id = audio_maybe_init_dummy("tsc.defaudio");
+    s->card.name = g_strdup(audiodev_id);
+    s->card.state = audio_state_by_name(s->card.name);
+    if (!s->card.state) {
+        error_setg(&error_fatal, "Cannot find audiodev with id '%s'",
+                   s->card.name);
+    }
+
     AUD_register_card(s->name, &s->card);
 
     qemu_register_reset((void *) tsc210x_reset, s);
-- 
2.35.1

[PATCH 11/18] hw/arm: Support machine-default audiodev with fallback

[Martin Kletzander]

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
 hw/arm/integratorcp.c |  8 +++++++-
 hw/arm/musicpal.c     |  8 +++++++-
 hw/arm/omap2.c        |  8 ++++++++
 hw/arm/realview.c     |  3 +++
 hw/arm/spitz.c        | 10 +++++++---
 hw/arm/versatilepb.c  |  3 +++
 hw/arm/vexpress.c     |  3 +++
 hw/arm/xlnx-zcu102.c  |  4 ++++
 hw/arm/z2.c           | 12 +++++++++++-
 9 files changed, 53 insertions(+), 6 deletions(-)

diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c
index b109ece3ae02..0a6d4186dad6 100644
--- a/hw/arm/integratorcp.c
+++ b/hw/arm/integratorcp.c
@@ -27,6 +27,7 @@
 #include "hw/irq.h"
 #include "hw/sd/sd.h"
 #include "qom/object.h"
+#include "audio/audio.h"
 
 #define TYPE_INTEGRATOR_CM "integrator_core"
 OBJECT_DECLARE_SIMPLE_TYPE(IntegratorCMState, INTEGRATOR_CM)
@@ -660,7 +661,12 @@ static void integratorcp_init(MachineState *machine)
                                &error_fatal);
     }
 
-    sysbus_create_varargs("pl041", 0x1d000000, pic[25], NULL);
+    dev = qdev_new("pl041");
+    qdev_prop_set_string(dev, "audiodev",
+                         audio_maybe_init_dummy("integrator.defaudio"));
+    sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
+    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0x1d000000);
+    sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[25]);
 
     if (nd_table[0].used)
         smc91c111_init(&nd_table[0], 0xc8000000, pic[27]);
diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c
index 7c840fb4283e..5d53ed3a8709 100644
--- a/hw/arm/musicpal.c
+++ b/hw/arm/musicpal.c
@@ -35,6 +35,7 @@
 #include "qemu/cutils.h"
 #include "qom/object.h"
 #include "hw/net/mv88w8618_eth.h"
+#include "audio/audio.h"
 
 #define MP_MISC_BASE            0x80002000
 #define MP_MISC_SIZE            0x00001000
@@ -1324,7 +1325,12 @@ static void musicpal_init(MachineState *machine)
         qdev_connect_gpio_out(key_dev, i, qdev_get_gpio_in(dev, i + 15));
     }
 
-    wm8750_dev = i2c_slave_create_simple(i2c, TYPE_WM8750, MP_WM_ADDR);
+    wm8750_dev = i2c_slave_new(TYPE_WM8750, MP_WM_ADDR);
+
+    qdev_prop_set_string(DEVICE(wm8750_dev), "audiodev",
+                         audio_maybe_init_dummy("musicpal.defaudio"));
+    i2c_slave_realize_and_unref(wm8750_dev, i2c, &error_abort);
+
     dev = qdev_new(TYPE_MV88W8618_AUDIO);
     s = SYS_BUS_DEVICE(dev);
     object_property_set_link(OBJECT(dev), "wm8750", OBJECT(wm8750_dev),
diff --git a/hw/arm/omap2.c b/hw/arm/omap2.c
index 02b1aa8c974b..4ae524a1a1a6 100644
--- a/hw/arm/omap2.c
+++ b/hw/arm/omap2.c
@@ -604,12 +604,20 @@ static struct omap_eac_s *omap_eac_init(struct omap_target_agent_s *ta,
                 qemu_irq irq, qemu_irq *drq, omap_clk fclk, omap_clk iclk)
 {
     struct omap_eac_s *s = g_new0(struct omap_eac_s, 1);
+    const char *audiodev_id = audio_maybe_init_dummy("eac.defaudio");
 
     s->irq = irq;
     s->codec.rxdrq = *drq ++;
     s->codec.txdrq = *drq;
     omap_eac_reset(s);
 
+    s->codec.card.name = g_strdup(audiodev_id);
+    s->codec.card.state = audio_state_by_name(s->codec.card.name);
+    if (!s->codec.card.state) {
+        error_setg(&error_fatal, "Cannot find audiodev with id '%s'",
+                   s->codec.card.name);
+    }
+
     AUD_register_card("OMAP EAC", &s->codec.card);
 
     memory_region_init_io(&s->iomem, NULL, &omap_eac_ops, s, "omap.eac",
diff --git a/hw/arm/realview.c b/hw/arm/realview.c
index d2dc8a895258..4fcdd821789a 100644
--- a/hw/arm/realview.c
+++ b/hw/arm/realview.c
@@ -29,6 +29,7 @@
 #include "hw/irq.h"
 #include "hw/i2c/arm_sbcon_i2c.h"
 #include "hw/sd/sd.h"
+#include "audio/audio.h"
 
 #define SMP_BOOT_ADDR 0xe0000000
 #define SMP_BOOTREG_ADDR 0x10000030
@@ -207,6 +208,8 @@ static void realview_init(MachineState *machine,
 
     pl041 = qdev_new("pl041");
     qdev_prop_set_uint32(pl041, "nc_fifo_depth", 512);
+    qdev_prop_set_string(pl041, "audiodev",
+                         audio_maybe_init_dummy("realview.defaudio"));
     sysbus_realize_and_unref(SYS_BUS_DEVICE(pl041), &error_fatal);
     sysbus_mmio_map(SYS_BUS_DEVICE(pl041), 0, 0x10004000);
     sysbus_connect_irq(SYS_BUS_DEVICE(pl041), 0, pic[19]);
diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
index 5aab0b856574..94fff2f460ca 100644
--- a/hw/arm/spitz.c
+++ b/hw/arm/spitz.c
@@ -35,6 +35,7 @@
 #include "exec/address-spaces.h"
 #include "cpu.h"
 #include "qom/object.h"
+#include "audio/audio.h"
 
 enum spitz_model_e { spitz, akita, borzoi, terrier };
 
@@ -779,10 +780,13 @@ static void spitz_i2c_setup(PXA2xxState *cpu)
     /* Attach the CPU on one end of our I2C bus.  */
     I2CBus *bus = pxa2xx_i2c_bus(cpu->i2c[0]);
 
-    DeviceState *wm;
-
     /* Attach a WM8750 to the bus */
-    wm = DEVICE(i2c_slave_create_simple(bus, TYPE_WM8750, 0));
+    I2CSlave *i2c_dev = i2c_slave_new(TYPE_WM8750, 0);
+    DeviceState *wm = DEVICE(i2c_dev);
+
+    qdev_prop_set_string(wm, "audiodev",
+                         audio_maybe_init_dummy("spitz.defaudio"));
+    i2c_slave_realize_and_unref(i2c_dev, bus, &error_abort);
 
     spitz_wm8750_addr(wm, 0, 0);
     qdev_connect_gpio_out(cpu->gpio, SPITZ_GPIO_WM,
diff --git a/hw/arm/versatilepb.c b/hw/arm/versatilepb.c
index ecc1f6cf74ff..e068f592b134 100644
--- a/hw/arm/versatilepb.c
+++ b/hw/arm/versatilepb.c
@@ -26,6 +26,7 @@
 #include "hw/char/pl011.h"
 #include "hw/sd/sd.h"
 #include "qom/object.h"
+#include "audio/audio.h"
 
 #define VERSATILE_FLASH_ADDR 0x34000000
 #define VERSATILE_FLASH_SIZE (64 * 1024 * 1024)
@@ -343,6 +344,8 @@ static void versatile_init(MachineState *machine, int board_id)
     /* Add PL041 AACI Interface to the LM4549 codec */
     pl041 = qdev_new("pl041");
     qdev_prop_set_uint32(pl041, "nc_fifo_depth", 512);
+    qdev_prop_set_string(pl041, "audiodev",
+                         audio_maybe_init_dummy("versatilepb.defaudio"));
     sysbus_realize_and_unref(SYS_BUS_DEVICE(pl041), &error_fatal);
     sysbus_mmio_map(SYS_BUS_DEVICE(pl041), 0, 0x10004000);
     sysbus_connect_irq(SYS_BUS_DEVICE(pl041), 0, sic[24]);
diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
index e1d1983ae657..f312a4d5315e 100644
--- a/hw/arm/vexpress.c
+++ b/hw/arm/vexpress.c
@@ -44,6 +44,7 @@
 #include "hw/i2c/arm_sbcon_i2c.h"
 #include "hw/sd/sd.h"
 #include "qom/object.h"
+#include "audio/audio.h"
 
 #define VEXPRESS_BOARD_ID 0x8e0
 #define VEXPRESS_FLASH_SIZE (64 * 1024 * 1024)
@@ -614,6 +615,8 @@ static void vexpress_common_init(MachineState *machine)
 
     pl041 = qdev_new("pl041");
     qdev_prop_set_uint32(pl041, "nc_fifo_depth", 512);
+    qdev_prop_set_string(pl041, "audiodev",
+                         audio_maybe_init_dummy("vexpress.defaudio"));
     sysbus_realize_and_unref(SYS_BUS_DEVICE(pl041), &error_fatal);
     sysbus_mmio_map(SYS_BUS_DEVICE(pl041), 0, map[VE_PL041]);
     sysbus_connect_irq(SYS_BUS_DEVICE(pl041), 0, pic[11]);
diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c
index 4c84bb932aa0..e4c32e158b61 100644
--- a/hw/arm/xlnx-zcu102.c
+++ b/hw/arm/xlnx-zcu102.c
@@ -24,6 +24,7 @@
 #include "sysemu/device_tree.h"
 #include "qom/object.h"
 #include "net/can_emu.h"
+#include "audio/audio.h"
 
 struct XlnxZCU102 {
     MachineState parent_obj;
@@ -143,6 +144,9 @@ static void xlnx_zcu102_init(MachineState *machine)
 
     object_initialize_child(OBJECT(machine), "soc", &s->soc, TYPE_XLNX_ZYNQMP);
 
+    qdev_prop_set_string(DEVICE(&s->soc.dp), "audiodev",
+                         audio_maybe_init_dummy("zcu102.defaudio"));
+
     object_property_set_link(OBJECT(&s->soc), "ddr-ram", OBJECT(machine->ram),
                              &error_abort);
     object_property_set_bool(OBJECT(&s->soc), "secure", s->secure,
diff --git a/hw/arm/z2.c b/hw/arm/z2.c
index 9c1e876207b3..8d9141998870 100644
--- a/hw/arm/z2.c
+++ b/hw/arm/z2.c
@@ -26,6 +26,7 @@
 #include "exec/address-spaces.h"
 #include "cpu.h"
 #include "qom/object.h"
+#include "qapi/error.h"
 
 #ifdef DEBUG_Z2
 #define DPRINTF(fmt, ...) \
@@ -306,6 +307,7 @@ static void z2_init(MachineState *machine)
     void *z2_lcd;
     I2CBus *bus;
     DeviceState *wm;
+    I2CSlave *i2c_dev;
 
     /* Setup CPU & memory */
     mpu = pxa270_init(address_space_mem, z2_binfo.ram_size, machine->cpu_type);
@@ -330,8 +332,16 @@ static void z2_init(MachineState *machine)
     type_register_static(&aer915_info);
     z2_lcd = ssi_create_peripheral(mpu->ssp[1], TYPE_ZIPIT_LCD);
     bus = pxa2xx_i2c_bus(mpu->i2c[0]);
+
     i2c_slave_create_simple(bus, TYPE_AER915, 0x55);
-    wm = DEVICE(i2c_slave_create_simple(bus, TYPE_WM8750, 0x1b));
+
+    i2c_dev = i2c_slave_new(TYPE_WM8750, 0x1b);
+    wm = DEVICE(i2c_dev);
+
+    qdev_prop_set_string(wm, "audiodev",
+                         audio_maybe_init_dummy("z2.defaudio"));
+    i2c_slave_realize_and_unref(i2c_dev, bus, &error_abort);
+
     mpu->i2s->opaque = wm;
     mpu->i2s->codec_out = wm8750_dac_dat;
     mpu->i2s->codec_in = wm8750_adc_dat;
-- 
2.35.1

[PATCH 12/18] hw/ppc: Support machine-default audiodev with fallback

[Martin Kletzander]

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
 hw/ppc/prep.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index bf622aa38fab..52d801afb307 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -46,6 +46,7 @@
 #include "elf.h"
 #include "qemu/units.h"
 #include "kvm_ppc.h"
+#include "audio/audio.h"
 
 /* SMP is not enabled, for now */
 #define MAX_CPUS 1
@@ -304,6 +305,9 @@ static void ibm_40p_init(MachineState *machine)
         dev = DEVICE(isa_dev);
         qdev_prop_set_uint32(dev, "iobase", 0x830);
         qdev_prop_set_uint32(dev, "irq", 10);
+
+        qdev_prop_set_string(dev, "audiodev",
+                             audio_maybe_init_dummy("ppc.defaudio"));
         isa_realize_and_unref(isa_dev, isa_bus, &error_fatal);
 
         isa_dev = isa_new("pc87312");
-- 
2.35.1

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

[Martin Kletzander]

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;
     }
 
     card->name = g_strdup (name);
     memset (&card->entries, 0, sizeof (card->entries));
     QLIST_INSERT_HEAD(&card->state->card_head, card, entries);
+
+    return true;
 }
 
 void AUD_remove_card (QEMUSoundCard *card)
diff --git a/audio/audio.h b/audio/audio.h
index 335704a4ddb1..9deed8ed6830 100644
--- a/audio/audio.h
+++ b/audio/audio.h
@@ -94,7 +94,7 @@ typedef struct QEMUAudioTimeStamp {
 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);
+bool AUD_register_card (const char *name, QEMUSoundCard *card, Error **errp);
 void AUD_remove_card (QEMUSoundCard *card);
 CaptureVoiceOut *AUD_add_capture(
     AudioState *s,
diff --git a/hw/arm/omap2.c b/hw/arm/omap2.c
index 4ae524a1a1a6..407c24551c84 100644
--- a/hw/arm/omap2.c
+++ b/hw/arm/omap2.c
@@ -618,7 +618,8 @@ static struct omap_eac_s *omap_eac_init(struct omap_target_agent_s *ta,
                    s->codec.card.name);
     }
 
-    AUD_register_card("OMAP EAC", &s->codec.card);
+    /* We can quit here because this only gets called from machine class init */
+    AUD_register_card("OMAP EAC", &s->codec.card, &error_fatal);
 
     memory_region_init_io(&s->iomem, NULL, &omap_eac_ops, s, "omap.eac",
                           omap_l4_region_size(ta, 0));
diff --git a/hw/audio/ac97.c b/hw/audio/ac97.c
index fd0b3b97d5b5..8242ddb0f93d 100644
--- a/hw/audio/ac97.c
+++ b/hw/audio/ac97.c
@@ -1345,6 +1345,10 @@ static void ac97_realize(PCIDevice *dev, Error **errp)
     AC97LinkState *s = AC97(dev);
     uint8_t *c = s->dev.config;
 
+    if (!AUD_register_card ("ac97", &s->card, errp)) {
+        return;
+    }
+
     /* TODO: no need to override */
     c[PCI_COMMAND] = 0x00;      /* pcicmd pci command rw, ro */
     c[PCI_COMMAND + 1] = 0x00;
@@ -1378,7 +1382,7 @@ static void ac97_realize(PCIDevice *dev, Error **errp)
                            "ac97-nabm", 256);
     pci_register_bar (&s->dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &s->io_nam);
     pci_register_bar (&s->dev, 1, PCI_BASE_ADDRESS_SPACE_IO, &s->io_nabm);
-    AUD_register_card ("ac97", &s->card);
+
     ac97_on_reset(DEVICE(s));
 }
 
diff --git a/hw/audio/adlib.c b/hw/audio/adlib.c
index ba1be6c8378d..39932654f7f5 100644
--- a/hw/audio/adlib.c
+++ b/hw/audio/adlib.c
@@ -269,8 +269,6 @@ static void adlib_realizefn (DeviceState *dev, Error **errp)
     as.fmt = AUDIO_FORMAT_S16;
     as.endianness = AUDIO_HOST_ENDIANNESS;
 
-    AUD_register_card ("adlib", &s->card);
-
     s->voice = AUD_open_out (
         &s->card,
         s->voice,
@@ -285,6 +283,11 @@ static void adlib_realizefn (DeviceState *dev, Error **errp)
         return;
     }
 
+    if (!AUD_register_card ("adlib", &s->card, errp)) {
+        Adlib_fini (s);
+        return;
+    }
+
     s->samples = AUD_get_buffer_size_out (s->voice) >> SHIFT;
     s->mixbuf = g_malloc0 (s->samples << SHIFT);
 
diff --git a/hw/audio/cs4231a.c b/hw/audio/cs4231a.c
index f510b862efbe..d9353a51ec66 100644
--- a/hw/audio/cs4231a.c
+++ b/hw/audio/cs4231a.c
@@ -676,13 +676,15 @@ static void cs4231a_realizefn (DeviceState *dev, Error **errp)
         return;
     }
 
+    if (!AUD_register_card ("cs4231a", &s->card, errp)) {
+        return;
+    }
+
     s->pic = isa_get_irq(d, s->irq);
     k = ISADMA_GET_CLASS(s->isa_dma);
     k->register_channel(s->isa_dma, s->dma, cs_dma_read, s);
 
     isa_register_ioport (d, &s->ioports, s->port);
-
-    AUD_register_card ("cs4231a", &s->card);
 }
 
 static Property cs4231a_properties[] = {
diff --git a/hw/audio/es1370.c b/hw/audio/es1370.c
index 62359b84f279..77a84f83060b 100644
--- a/hw/audio/es1370.c
+++ b/hw/audio/es1370.c
@@ -867,7 +867,10 @@ static void es1370_realize(PCIDevice *dev, Error **errp)
     memory_region_init_io (&s->io, OBJECT(s), &es1370_io_ops, s, "es1370", 256);
     pci_register_bar (&s->dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &s->io);
 
-    AUD_register_card ("es1370", &s->card);
+    if (!AUD_register_card ("es1370", &s->card, errp)) {
+        return;
+    }
+
     es1370_reset (s);
 }
 
diff --git a/hw/audio/gus.c b/hw/audio/gus.c
index c7710a684b88..f890042baa97 100644
--- a/hw/audio/gus.c
+++ b/hw/audio/gus.c
@@ -245,7 +245,9 @@ static void gus_realizefn (DeviceState *dev, Error **errp)
         return;
     }
 
-    AUD_register_card ("gus", &s->card);
+    if (!AUD_register_card ("gus", &s->card, errp)) {
+        return;
+    }
 
     as.freq = s->freq;
     as.nchannels = 2;
diff --git a/hw/audio/hda-codec.c b/hw/audio/hda-codec.c
index e86a2adf31a0..7f8a7fa7ca1b 100644
--- a/hw/audio/hda-codec.c
+++ b/hw/audio/hda-codec.c
@@ -690,7 +690,10 @@ static void hda_audio_init(HDACodecDevice *hda,
     a->name = object_get_typename(OBJECT(a));
     dprint(a, 1, "%s: cad %d\n", __func__, a->hda.cad);
 
-    AUD_register_card("hda", &a->card);
+    if (!AUD_register_card("hda", &a->card, errp)) {
+        return;
+    }
+
     for (i = 0; i < a->desc->nnodes; i++) {
         node = a->desc->nodes + i;
         param = hda_codec_find_param(node, AC_PAR_AUDIO_WIDGET_CAP);
diff --git a/hw/audio/lm4549.c b/hw/audio/lm4549.c
index 418041bc9c6c..9afb81517e8f 100644
--- a/hw/audio/lm4549.c
+++ b/hw/audio/lm4549.c
@@ -289,7 +289,9 @@ void lm4549_init(lm4549_state *s, lm4549_callback data_req_cb, void* opaque,
     lm4549_reset(s);
 
     /* Register an audio card */
-    AUD_register_card("lm4549", &s->card);
+    if (!AUD_register_card("lm4549", &s->card, errp)) {
+        return;
+    }
 
     /* Open a default voice */
     as.freq = 48000;
diff --git a/hw/audio/pcspk.c b/hw/audio/pcspk.c
index 72bea5fb202a..41bf6a5cfdb3 100644
--- a/hw/audio/pcspk.c
+++ b/hw/audio/pcspk.c
@@ -122,7 +122,9 @@ static int pcspk_audio_init(PCSpkState *s)
         return 0;
     }
 
-    AUD_register_card(s_spk, &s->card);
+    if (!AUD_register_card(s_spk, &s->card, NULL)) {
+        return -1;
+    }
 
     s->voice = AUD_open_out(&s->card, s->voice, s_spk, s, pcspk_callback, &as);
     if (!s->voice) {
diff --git a/hw/audio/sb16.c b/hw/audio/sb16.c
index 593da2478c14..dd2131426cd3 100644
--- a/hw/audio/sb16.c
+++ b/hw/audio/sb16.c
@@ -1400,6 +1400,10 @@ static void sb16_realizefn (DeviceState *dev, Error **errp)
     SB16State *s = SB16 (dev);
     IsaDmaClass *k;
 
+    if (!AUD_register_card ("sb16", &s->card, errp)) {
+        return;
+    }
+
     s->isa_hdma = isa_get_dma(isa_bus_from_device(isadev), s->hdma);
     s->isa_dma = isa_get_dma(isa_bus_from_device(isadev), s->dma);
     if (!s->isa_dma || !s->isa_hdma) {
@@ -1432,8 +1436,6 @@ static void sb16_realizefn (DeviceState *dev, Error **errp)
     k->register_channel(s->isa_dma, s->dma, SB_read_DMA, s);
 
     s->can_write = 1;
-
-    AUD_register_card ("sb16", &s->card);
 }
 
 static Property sb16_properties[] = {
diff --git a/hw/audio/wm8750.c b/hw/audio/wm8750.c
index b5722b37c36b..57954a631442 100644
--- a/hw/audio/wm8750.c
+++ b/hw/audio/wm8750.c
@@ -624,7 +624,10 @@ static void wm8750_realize(DeviceState *dev, Error **errp)
 {
     WM8750State *s = WM8750(dev);
 
-    AUD_register_card(CODEC, &s->card);
+    if (!AUD_register_card(CODEC, &s->card, errp)) {
+        return;
+    }
+
     wm8750_reset(I2C_SLAVE(s));
 }
 
diff --git a/hw/display/xlnx_dp.c b/hw/display/xlnx_dp.c
index b16d6be2b5cc..d6699f00d89e 100644
--- a/hw/display/xlnx_dp.c
+++ b/hw/display/xlnx_dp.c
@@ -1278,6 +1278,10 @@ static void xlnx_dp_realize(DeviceState *dev, Error **errp)
     DisplaySurface *surface;
     struct audsettings as;
 
+    if (!AUD_register_card("xlnx_dp.audio", &s->aud_card, errp)) {
+        return;
+    }
+
     aux_bus_realize(s->aux_bus);
 
     qdev_realize(DEVICE(s->dpcd), BUS(s->aux_bus), &error_fatal);
@@ -1296,8 +1300,6 @@ static void xlnx_dp_realize(DeviceState *dev, Error **errp)
     as.fmt = AUDIO_FORMAT_S16;
     as.endianness = 0;
 
-    AUD_register_card("xlnx_dp.audio", &s->aud_card);
-
     s->amixer_output_stream = AUD_open_out(&s->aud_card,
                                            s->amixer_output_stream,
                                            "xlnx_dp.audio.out",
diff --git a/hw/input/tsc210x.c b/hw/input/tsc210x.c
index f0b02bc72280..1ebf0199bfc7 100644
--- a/hw/input/tsc210x.c
+++ b/hw/input/tsc210x.c
@@ -1106,7 +1106,8 @@ static void tsc210x_init(TSC210xState *s,
                    s->card.name);
     }
 
-    AUD_register_card(s->name, &s->card);
+    /* We can quit here because this only gets called from machine class init */
+    AUD_register_card(s->name, &s->card, &error_fatal);
 
     qemu_register_reset((void *) tsc210x_reset, s);
     vmstate_register(NULL, 0, vmsd, s);
diff --git a/hw/usb/dev-audio.c b/hw/usb/dev-audio.c
index 8748c1ba0401..d5ac1f8962e3 100644
--- a/hw/usb/dev-audio.c
+++ b/hw/usb/dev-audio.c
@@ -944,12 +944,15 @@ static void usb_audio_realize(USBDevice *dev, Error **errp)
     USBAudioState *s = USB_AUDIO(dev);
     int i;
 
+    if (!AUD_register_card(TYPE_USB_AUDIO, &s->card, errp)) {
+        return;
+    }
+
     dev->usb_desc = s->multi ? &desc_audio_multi : &desc_audio;
 
     usb_desc_create_serial(dev);
     usb_desc_init(dev);
     s->dev.opaque = s;
-    AUD_register_card(TYPE_USB_AUDIO, &s->card);
 
     s->out.altset        = ALTSET_OFF;
     s->out.vol.mute      = false;
-- 
2.35.1

[PATCH 14/18] audio: Require AudioState in AUD_add_capture

[Martin Kletzander]

Since all callers require a valid audiodev this function can now safely
abort in case of missing AudioState.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
 audio/audio.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index b95aca444382..97eb645764c1 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1855,10 +1855,8 @@ CaptureVoiceOut *AUD_add_capture(
     struct capture_callback *cb;
 
     if (!s) {
-        if (!legacy_config) {
-            dolog("Capturing without setting an audiodev is deprecated\n");
-        }
-        s = audio_init(NULL, NULL);
+        error_setg(&error_abort,
+                   "Capturing without setting an audiodev is not supported");
     }
 
     if (!audio_get_pdo_out(s->dev)->mixing_engine) {
-- 
2.35.1

[PATCH 15/18] audio: Be more strict during audio backend initialisation

[Martin Kletzander]

Now that audiodev= is required and audio_init() will not be called
without and AudioDev we can remove the fallback functionality and error
out in case audio drivers fail initialisation or when the driver does
not exist.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
 audio/audio.c                   | 146 ++++++--------------------------
 docs/about/deprecated.rst       |   8 --
 docs/about/removed-features.rst |   8 ++
 3 files changed, 34 insertions(+), 128 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index 97eb645764c1..c944cf817cf9 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -87,6 +87,8 @@ audio_driver *audio_driver_lookup(const char *name)
         }
     }
 
+    error_setg(&error_fatal, "Unknown audio driver `%s'", name);
+    /* Avoid compiler complaining that we do not return in non-void function */
     return NULL;
 }
 
@@ -104,8 +106,6 @@ const struct mixeng_volume nominal_volume = {
 #endif
 };
 
-static bool legacy_config = true;
-
 int audio_bug (const char *funcname, int cond)
 {
     if (cond) {
@@ -1532,31 +1532,27 @@ size_t audio_generic_read(HWVoiceIn *hw, void *buf, size_t size)
     return total;
 }
 
-static int audio_driver_init(AudioState *s, struct audio_driver *drv,
-                             bool msg, Audiodev *dev)
+static void audio_driver_init(AudioState *s, struct audio_driver *drv,
+                              Audiodev *dev)
 {
     s->drv_opaque = drv->init(dev);
 
-    if (s->drv_opaque) {
-        if (!drv->pcm_ops->get_buffer_in) {
-            drv->pcm_ops->get_buffer_in = audio_generic_get_buffer_in;
-            drv->pcm_ops->put_buffer_in = audio_generic_put_buffer_in;
-        }
-        if (!drv->pcm_ops->get_buffer_out) {
-            drv->pcm_ops->get_buffer_out = audio_generic_get_buffer_out;
-            drv->pcm_ops->put_buffer_out = audio_generic_put_buffer_out;
-        }
+    if (!s->drv_opaque) {
+        error_setg(&error_fatal, "Could not init `%s' audio driver", drv->name);
+    }
 
-        audio_init_nb_voices_out(s, drv);
-        audio_init_nb_voices_in(s, drv);
-        s->drv = drv;
-        return 0;
-    } else {
-        if (msg) {
-            dolog("Could not init `%s' audio driver\n", drv->name);
-        }
-        return -1;
+    if (!drv->pcm_ops->get_buffer_in) {
+        drv->pcm_ops->get_buffer_in = audio_generic_get_buffer_in;
+        drv->pcm_ops->put_buffer_in = audio_generic_put_buffer_in;
     }
+    if (!drv->pcm_ops->get_buffer_out) {
+        drv->pcm_ops->get_buffer_out = audio_generic_get_buffer_out;
+        drv->pcm_ops->put_buffer_out = audio_generic_put_buffer_out;
+    }
+
+    audio_init_nb_voices_out(s, drv);
+    audio_init_nb_voices_in(s, drv);
+    s->drv = drv;
 }
 
 static void audio_vm_change_state_handler (void *opaque, bool running,
@@ -1661,79 +1657,19 @@ static const VMStateDescription vmstate_audio = {
 
 static void audio_validate_opts(Audiodev *dev, Error **errp);
 
-static AudiodevListEntry *audiodev_find(
-    AudiodevListHead *head, const char *drvname)
-{
-    AudiodevListEntry *e;
-    QSIMPLEQ_FOREACH(e, head, next) {
-        if (strcmp(AudiodevDriver_str(e->dev->driver), drvname) == 0) {
-            return e;
-        }
-    }
-
-    return NULL;
-}
-
-/*
- * 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
- */
-static AudioState *audio_init(Audiodev *dev, const char *name)
+static AudioState *audio_init(Audiodev *dev)
 {
     static bool atexit_registered;
-    size_t i;
-    int done = 0;
     const char *drvname = NULL;
     VMChangeStateEntry *e;
     AudioState *s;
-    struct audio_driver *driver;
-    /* silence gcc warning about uninitialized variable */
-    AudiodevListHead head = QSIMPLEQ_HEAD_INITIALIZER(head);
-
-    if (using_spice) {
-        /*
-         * When using spice allow the spice audio driver being picked
-         * as default.
-         *
-         * Temporary hack.  Using audio devices without explicit
-         * audiodev= property is already deprecated.  Same goes for
-         * the -soundhw switch.  Once this support gets finally
-         * removed we can also drop the concept of a default audio
-         * backend and this can go away.
-         */
-        driver = audio_driver_lookup("spice");
-        if (driver) {
-            driver->can_be_default = 1;
-        }
-    }
 
-    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);
+    if (!dev) {
+        error_setg(&error_abort, "Mandatory audiodev parameter required");
     }
 
+    drvname = AudiodevDriver_str(dev->driver);
+
     s = g_new0(AudioState, 1);
     s->dev = dev;
 
@@ -1763,37 +1699,7 @@ static AudioState *audio_init(Audiodev *dev, const char *name)
         s->nb_hw_voices_in = 0;
     }
 
-    if (drvname) {
-        driver = audio_driver_lookup(drvname);
-        if (driver) {
-            done = !audio_driver_init(s, driver, true, dev);
-        } else {
-            dolog ("Unknown audio driver `%s'\n", drvname);
-        }
-    } else {
-        for (i = 0; audio_prio_list[i]; i++) {
-            AudiodevListEntry *e = audiodev_find(&head, audio_prio_list[i]);
-            driver = audio_driver_lookup(audio_prio_list[i]);
-
-            if (e && driver) {
-                s->dev = dev = e->dev;
-                audio_validate_opts(dev, &error_abort);
-                done = !audio_driver_init(s, driver, false, dev);
-                if (done) {
-                    e->dev = NULL;
-                    break;
-                }
-            }
-        }
-    }
-    audio_free_audiodev_list(&head);
-
-    if (!done) {
-        driver = audio_driver_lookup("none");
-        done = !audio_driver_init(s, driver, false, dev);
-        assert(done);
-        dolog("warning: Using timer based audio emulation\n");
-    }
+    audio_driver_init(s, audio_driver_lookup(drvname), dev);
 
     if (dev->timer_period <= 0) {
         s->period_ticks = 1;
@@ -2120,7 +2026,7 @@ void audio_init_audiodevs(void)
     AudiodevListEntry *e;
 
     QSIMPLEQ_FOREACH(e, &audiodevs, next) {
-        audio_init(e->dev, NULL);
+        audio_init(e->dev);
     }
 }
 
@@ -2133,7 +2039,7 @@ static void audio_init_dummy(const char *id)
     dev->id = g_strdup(id);
 
     audio_validate_opts(dev, &error_abort);
-    audio_init(dev, NULL);
+    audio_init(dev);
 
     e->dev = dev;
     QSIMPLEQ_INSERT_TAIL(&audiodevs, e, next);
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 7ba71ebd3435..b2255319e347 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -31,14 +31,6 @@ 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)
-''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
-
-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.
-
 ``-chardev`` backend aliases ``tty`` and ``parport`` (since 6.0)
 ''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
 
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index 086ba3edb042..f9eea4dda327 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -346,6 +346,14 @@ needs two devices (``-device intel-hda -device hda-duplex``) and
 pcspk-audiodev=<name>``.  And ``AC97`` and ``ES1370`` now have to be
 specified in uppercase.
 
+Creating sound card devices and vnc without ``audiodev=`` property (removed in 7.1)
+'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+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.
+
 
 QEMU Machine Protocol (QMP) commands
 ------------------------------------
-- 
2.35.1

[PATCH 16/18] audio: Remove legacy audio environment variables and options

[Martin Kletzander]

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
 audio/audio.c                   |  13 -
 audio/audio.h                   |   1 -
 audio/audio_legacy.c            | 555 --------------------------------
 audio/meson.build               |   1 -
 docs/about/deprecated.rst       |   7 -
 docs/about/removed-features.rst |   9 +
 qemu-options.hx                 |  10 -
 softmmu/vl.c                    |   4 -
 8 files changed, 9 insertions(+), 591 deletions(-)
 delete mode 100644 audio/audio_legacy.c

diff --git a/audio/audio.c b/audio/audio.c
index c944cf817cf9..b3ecc8fa6508 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -49,19 +49,6 @@
 
 #define SW_NAME(sw) (sw)->name ? (sw)->name : "unknown"
 
-
-/* Order of CONFIG_AUDIO_DRIVERS is import.
-   The 1st one is the one used by default, that is the reason
-    that we generate the list.
-*/
-const char *audio_prio_list[] = {
-    "spice",
-    CONFIG_AUDIO_DRIVERS
-    "none",
-    "wav",
-    NULL
-};
-
 static QLIST_HEAD(, audio_driver) audio_drivers;
 static AudiodevListHead audiodevs = QSIMPLEQ_HEAD_INITIALIZER(audiodevs);
 
diff --git a/audio/audio.h b/audio/audio.h
index 9deed8ed6830..ae10a7f0fa81 100644
--- a/audio/audio.h
+++ b/audio/audio.h
@@ -170,7 +170,6 @@ void audio_sample_from_uint64(void *samples, int pos,
 
 void audio_parse_option(const char *opt);
 void audio_init_audiodevs(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 595949f52cd4..000000000000
--- a/audio/audio_legacy.c
+++ /dev/null
@@ -1,555 +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, bool *has_dst)
-{
-    const char *val = getenv(env);
-    if (val) {
-        if (*has_dst) {
-            g_free(*dst);
-        }
-        *dst = g_strdup(val);
-        *has_dst = true;
-    }
-}
-
-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, &apdo->has_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, &opdo->has_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, &ppdo->has_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, &dev->u.pa.has_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, &dev->u.wav.has_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 94dab16891d5..d73155d2f2ae 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 b2255319e347..7cc44e611ebe 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -23,13 +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.
 
 ``-chardev`` backend aliases ``tty`` and ``parport`` (since 6.0)
 ''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index f9eea4dda327..e2a95baf9031 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -354,6 +354,15 @@ 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.
 
+``QEMU_AUDIO_`` environment variables and ``-audio-help`` (removed in 7.1)
+''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+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 ``-audio-help`` option can be used with older QEMU
+binaries to convert some of the old values of the environment variables
+to new ``-audiodev`` options.
+
 
 QEMU Machine Protocol (QMP) commands
 ------------------------------------
diff --git a/qemu-options.hx b/qemu-options.hx
index 47bbd86a4533..b45d488440fc 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -651,16 +651,6 @@ SRST
 ERST
 
 
-HXCOMM Deprecated by -audiodev
-DEF("audio-help", 0, QEMU_OPTION_audio_help,
-    "-audio-help     show -audiodev equivalent of the currently specified audio settings\n",
-    QEMU_ARCH_ALL)
-SRST
-``-audio-help``
-    Will show the -audiodev equivalent of the currently specified
-    (deprecated) environment variables.
-ERST
-
 DEF("audiodev", HAS_ARG, QEMU_OPTION_audiodev,
     "-audiodev [driver=]driver,id=id[,prop[=value][,...]]\n"
     "                specifies the audio backend to use\n"
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 2f7158574e4f..248bf8f66c4c 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -3008,10 +3008,6 @@ void qemu_init(int argc, char **argv, char **envp)
                 }
                 break;
 #endif
-            case QEMU_OPTION_audio_help:
-                audio_legacy_help();
-                exit (0);
-                break;
             case QEMU_OPTION_audiodev:
                 audio_parse_option(optarg);
                 break;
-- 
2.35.1

[PATCH 17/18] audio: Remove unused can_be_default

[Martin Kletzander]

Since there is no fallback mechanism and default-guessing this is now
not used and can be safely removed.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
 audio/alsaaudio.c   | 1 -
 audio/audio_int.h   | 1 -
 audio/coreaudio.m   | 1 -
 audio/dbusaudio.c   | 1 -
 audio/dsoundaudio.c | 1 -
 audio/jackaudio.c   | 1 -
 audio/noaudio.c     | 1 -
 audio/ossaudio.c    | 1 -
 audio/paaudio.c     | 1 -
 audio/sdlaudio.c    | 1 -
 audio/wavaudio.c    | 1 -
 11 files changed, 11 deletions(-)

diff --git a/audio/alsaaudio.c b/audio/alsaaudio.c
index 4a61378cd757..edbee63f97c5 100644
--- a/audio/alsaaudio.c
+++ b/audio/alsaaudio.c
@@ -933,7 +933,6 @@ static struct audio_driver alsa_audio_driver = {
     .init           = alsa_audio_init,
     .fini           = alsa_audio_fini,
     .pcm_ops        = &alsa_pcm_ops,
-    .can_be_default = 1,
     .max_voices_out = INT_MAX,
     .max_voices_in  = INT_MAX,
     .voice_size_out = sizeof (ALSAVoiceOut),
diff --git a/audio/audio_int.h b/audio/audio_int.h
index 2a6914d2aa65..082b13247657 100644
--- a/audio/audio_int.h
+++ b/audio/audio_int.h
@@ -148,7 +148,6 @@ struct audio_driver {
     void (*set_dbus_server) (AudioState *s, GDBusObjectManagerServer *manager);
 #endif
     struct audio_pcm_ops *pcm_ops;
-    int can_be_default;
     int max_voices_out;
     int max_voices_in;
     int voice_size_out;
diff --git a/audio/coreaudio.m b/audio/coreaudio.m
index 4695291621a3..e5612138a74b 100644
--- a/audio/coreaudio.m
+++ b/audio/coreaudio.m
@@ -673,7 +673,6 @@ static void coreaudio_audio_fini (void *opaque)
     .init           = coreaudio_audio_init,
     .fini           = coreaudio_audio_fini,
     .pcm_ops        = &coreaudio_pcm_ops,
-    .can_be_default = 1,
     .max_voices_out = 1,
     .max_voices_in  = 0,
     .voice_size_out = sizeof (coreaudioVoiceOut),
diff --git a/audio/dbusaudio.c b/audio/dbusaudio.c
index a3d656d3b017..bbcad7051413 100644
--- a/audio/dbusaudio.c
+++ b/audio/dbusaudio.c
@@ -638,7 +638,6 @@ static struct audio_driver dbus_audio_driver = {
     .fini            = dbus_audio_fini,
     .set_dbus_server = dbus_audio_set_server,
     .pcm_ops         = &dbus_pcm_ops,
-    .can_be_default  = 1,
     .max_voices_out  = INT_MAX,
     .max_voices_in   = INT_MAX,
     .voice_size_out  = sizeof(DBusVoiceOut),
diff --git a/audio/dsoundaudio.c b/audio/dsoundaudio.c
index 3fb67ec3eed4..311e34218465 100644
--- a/audio/dsoundaudio.c
+++ b/audio/dsoundaudio.c
@@ -721,7 +721,6 @@ static struct audio_driver dsound_audio_driver = {
     .init           = dsound_audio_init,
     .fini           = dsound_audio_fini,
     .pcm_ops        = &dsound_pcm_ops,
-    .can_be_default = 1,
     .max_voices_out = INT_MAX,
     .max_voices_in  = 1,
     .voice_size_out = sizeof (DSoundVoiceOut),
diff --git a/audio/jackaudio.c b/audio/jackaudio.c
index 5bdf3d7a78d6..fd2d2fd5acb7 100644
--- a/audio/jackaudio.c
+++ b/audio/jackaudio.c
@@ -669,7 +669,6 @@ static struct audio_driver jack_driver = {
     .init           = qjack_init,
     .fini           = qjack_fini,
     .pcm_ops        = &jack_pcm_ops,
-    .can_be_default = 1,
     .max_voices_out = INT_MAX,
     .max_voices_in  = INT_MAX,
     .voice_size_out = sizeof(QJackOut),
diff --git a/audio/noaudio.c b/audio/noaudio.c
index 84a6bfbb1c87..111aef4a24ce 100644
--- a/audio/noaudio.c
+++ b/audio/noaudio.c
@@ -135,7 +135,6 @@ static struct audio_driver no_audio_driver = {
     .init           = no_audio_init,
     .fini           = no_audio_fini,
     .pcm_ops        = &no_pcm_ops,
-    .can_be_default = 1,
     .max_voices_out = INT_MAX,
     .max_voices_in  = INT_MAX,
     .voice_size_out = sizeof (NoVoiceOut),
diff --git a/audio/ossaudio.c b/audio/ossaudio.c
index 8e075edb70d6..31b582e8e4b5 100644
--- a/audio/ossaudio.c
+++ b/audio/ossaudio.c
@@ -781,7 +781,6 @@ static struct audio_driver oss_audio_driver = {
     .init           = oss_audio_init,
     .fini           = oss_audio_fini,
     .pcm_ops        = &oss_pcm_ops,
-    .can_be_default = 1,
     .max_voices_out = INT_MAX,
     .max_voices_in  = INT_MAX,
     .voice_size_out = sizeof (OSSVoiceOut),
diff --git a/audio/paaudio.c b/audio/paaudio.c
index e91116f2396c..38f09017eb96 100644
--- a/audio/paaudio.c
+++ b/audio/paaudio.c
@@ -928,7 +928,6 @@ static struct audio_driver pa_audio_driver = {
     .init           = qpa_audio_init,
     .fini           = qpa_audio_fini,
     .pcm_ops        = &qpa_pcm_ops,
-    .can_be_default = 1,
     .max_voices_out = INT_MAX,
     .max_voices_in  = INT_MAX,
     .voice_size_out = sizeof (PAVoiceOut),
diff --git a/audio/sdlaudio.c b/audio/sdlaudio.c
index 68a237b76b45..5177e31d9b4a 100644
--- a/audio/sdlaudio.c
+++ b/audio/sdlaudio.c
@@ -493,7 +493,6 @@ static struct audio_driver sdl_audio_driver = {
     .init           = sdl_audio_init,
     .fini           = sdl_audio_fini,
     .pcm_ops        = &sdl_pcm_ops,
-    .can_be_default = 1,
     .max_voices_out = INT_MAX,
     .max_voices_in  = INT_MAX,
     .voice_size_out = sizeof(SDLVoiceOut),
diff --git a/audio/wavaudio.c b/audio/wavaudio.c
index ac666335c783..4e176500fc92 100644
--- a/audio/wavaudio.c
+++ b/audio/wavaudio.c
@@ -208,7 +208,6 @@ static struct audio_driver wav_audio_driver = {
     .init           = wav_audio_init,
     .fini           = wav_audio_fini,
     .pcm_ops        = &wav_pcm_ops,
-    .can_be_default = 0,
     .max_voices_out = 1,
     .max_voices_in  = 0,
     .voice_size_out = sizeof (WAVVoiceOut),
-- 
2.35.1

[PATCH 18/18] audio/spiceaudio: Fail initialisation when not using spice

[Martin Kletzander]

The caller would already fail, but this way the message can better
express the reason for the failure.

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

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
 audio/spiceaudio.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/audio/spiceaudio.c b/audio/spiceaudio.c
index a8d370fe6f31..fdbd7dc285ad 100644
--- a/audio/spiceaudio.c
+++ b/audio/spiceaudio.c
@@ -74,8 +74,9 @@ static const SpiceRecordInterface record_sif = {
 static void *spice_audio_init(Audiodev *dev)
 {
     if (!using_spice) {
-        return NULL;
+        error_setg(&error_fatal, "Cannot use spice audio without -spice");
     }
+
     return &spice_audio_init;
 }
 
-- 
2.35.1

Re: [PATCH 18/18] audio/spiceaudio: Fail initialisation when not using spice

[Daniel P. Berrangé]

On Mon, Apr 25, 2022 at 10:22:01AM +0200, Martin Kletzander wrote:
> The caller would already fail, but this way the message can better
> express the reason for the failure.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2043498
> 
> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> ---
>  audio/spiceaudio.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/audio/spiceaudio.c b/audio/spiceaudio.c
> index a8d370fe6f31..fdbd7dc285ad 100644
> --- a/audio/spiceaudio.c
> +++ b/audio/spiceaudio.c
> @@ -74,8 +74,9 @@ static const SpiceRecordInterface record_sif = {
>  static void *spice_audio_init(Audiodev *dev)
>  {
>      if (!using_spice) {
> -        return NULL;
> +        error_setg(&error_fatal, "Cannot use spice audio without -spice");

Typically one would not use error_fatal directly with a call
to error_setg(). The usual pattern would be for the method
calling error_setg() to have an 'Error **errp' parameter.
The caller would then pass in &error_fatal when calling the
method, or pass in a real error object if wishing to receive
the error.

If you don't want to plumb in an 'Error **errp' to the
spice_audio_init() method, then it would be sufficient to
instead just do

   error_report("Cannot use spice....")

Using 'Error **errp' is best practice in new code, but no one
will blame you for not refactoring existing code to support
this if looks like too much work.

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

Re: [PATCH 01/18] hw/audio: Remove -soundhw support

[Daniel P. Berrangé]

On Mon, Apr 25, 2022 at 10:21:44AM +0200, Martin Kletzander wrote:
> One thing I am not sure about is whether to keep the aliases of ac97 and
> es1370 in the qdev_alias_table.

I'm not sure we have any prior art in this respect. I feel it is a bit
odd-ball that AC97 and ES1370 are uppercase in qdev, so in that respect
the ac97/es1370 aliases are nice.

Libvirt uses uppercase form, so wouldn't be impacted if we removed the
aliases.

I don't have a strong preference either way.

I'd suggest splitting the removal of the aliases into a separate patch
since it isn't strictly neeeding to be done at the same time.

If you split off, then I'm happy for both bits to be

> 
> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> ---
>  docs/about/deprecated.rst                     |   9 -
>  docs/about/removed-features.rst               |  10 +
>  docs/qdev-device-use.txt                      |  21 +--
>  docs/replay.txt                               |   2 +-
>  hw/audio/ac97.c                               |   3 -
>  hw/audio/adlib.c                              |   2 -
>  hw/audio/cs4231a.c                            |   2 -
>  hw/audio/es1370.c                             |   3 -
>  hw/audio/gus.c                                |   2 -
>  hw/audio/intel-hda.c                          |  21 ---
>  hw/audio/meson.build                          |   1 -
>  hw/audio/pcspk.c                              |  11 --
>  hw/audio/sb16.c                               |   3 -
>  hw/audio/soundhw.c                            | 177 ------------------
>  include/hw/audio/soundhw.h                    |  15 --
>  qemu-options.hx                               |  27 ---
>  .../codeconverter/test_regexps.py             |   1 -
>  softmmu/qdev-monitor.c                        |   2 -
>  softmmu/vl.c                                  |   6 -
>  19 files changed, 19 insertions(+), 299 deletions(-)
>  delete mode 100644 hw/audio/soundhw.c
>  delete mode 100644 include/hw/audio/soundhw.h
> 
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index cf02ef6821e4..7ba71ebd3435 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -39,15 +39,6 @@ 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.
>  
> -Creating sound card devices using ``-soundhw`` (since 5.1)
> -''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
> -
> -Sound card devices should be created using ``-device`` instead.  The
> -names are the same for most devices.  The exceptions are ``hda`` which
> -needs two devices (``-device intel-hda -device hda-duplex``) and
> -``pcspk`` which can be activated using ``-machine
> -pcspk-audiodev=<name>``.
> -
>  ``-chardev`` backend aliases ``tty`` and ``parport`` (since 6.0)
>  ''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
>  
> diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
> index 4b831ea29176..086ba3edb042 100644
> --- a/docs/about/removed-features.rst
> +++ b/docs/about/removed-features.rst
> @@ -336,6 +336,16 @@ for the RISC-V ``virt`` machine and ``sifive_u`` machine.
>  The ``-no-quit`` was a synonym for ``-display ...,window-close=off`` which
>  should be used instead.
>  
> +Creating sound card devices using ``-soundhw`` (removed in 7.1)
> +'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
> +
> +Sound card devices should be created using ``-device`` instead.  The
> +names are the same for most devices.  The exceptions are ``hda`` which
> +needs two devices (``-device intel-hda -device hda-duplex``) and
> +``pcspk`` which can be activated using ``-machine
> +pcspk-audiodev=<name>``.  And ``AC97`` and ``ES1370`` now have to be
> +specified in uppercase.
> +
>  
>  QEMU Machine Protocol (QMP) commands
>  ------------------------------------
> diff --git a/docs/qdev-device-use.txt b/docs/qdev-device-use.txt
> index 240888933482..30e7eaa3e66d 100644
> --- a/docs/qdev-device-use.txt
> +++ b/docs/qdev-device-use.txt
> @@ -311,21 +311,16 @@ constraints.
>  
>  Host and guest part of audio devices have always been separate.
>  
> -The old way to define guest audio devices is -soundhw C1,...
> +Host side (backend) is defined using -audiodev with a specific driver:
>  
> -The new way is to define each guest audio device separately with
> --device.
> +    spice
> +    pa
> +    none
>  
> -Map from -soundhw sound card name to -device:
> -
> -    ac97        -device AC97
> -    cs4231a     -device cs4231a,iobase=IOADDR,irq=IRQ,dma=DMA
> -    es1370      -device ES1370
> -    gus         -device gus,iobase=IOADDR,irq=IRQ,dma=DMA,freq=F
> -    hda         -device intel-hda,msi=MSI -device hda-duplex
> -    sb16        -device sb16,iobase=IOADDR,irq=IRQ,dma=DMA,dma16=DMA16,version=V
> -    adlib       not yet available with -device
> -    pcspk       not yet available with -device
> +And each guest audio device is then defined with -device with
> +audiodev=AUDIODEV_ID that refers to the audio backend above.  Exceptions are
> +pcspk and adlib which are note yet available with -device and are part of a
> +machine type.
>  
>  For PCI devices, you can add bus=PCI-BUS,addr=DEVFN to control the PCI
>  device address, as usual.
> diff --git a/docs/replay.txt b/docs/replay.txt
> index 5b008ca4911f..c329767c148a 100644
> --- a/docs/replay.txt
> +++ b/docs/replay.txt
> @@ -294,7 +294,7 @@ Audio devices
>  
>  Audio data is recorded and replay automatically. The command line for recording
>  and replaying must contain identical specifications of audio hardware, e.g.:
> - -soundhw ac97
> + -audiodev driver=pa -device ac97,audiodev=audio0
>  
>  Serial ports
>  ------------
> diff --git a/hw/audio/ac97.c b/hw/audio/ac97.c
> index 3cb81310607f..fd0b3b97d5b5 100644
> --- a/hw/audio/ac97.c
> +++ b/hw/audio/ac97.c
> @@ -18,7 +18,6 @@
>   */
>  
>  #include "qemu/osdep.h"
> -#include "hw/audio/soundhw.h"
>  #include "audio/audio.h"
>  #include "hw/pci/pci.h"
>  #include "hw/qdev-properties.h"
> @@ -1430,8 +1429,6 @@ static const TypeInfo ac97_info = {
>  static void ac97_register_types (void)
>  {
>      type_register_static (&ac97_info);
> -    deprecated_register_soundhw("ac97", "Intel 82801AA AC97 Audio",
> -                                0, TYPE_AC97);
>  }
>  
>  type_init (ac97_register_types)
> diff --git a/hw/audio/adlib.c b/hw/audio/adlib.c
> index 5f979b1487d1..ba1be6c8378d 100644
> --- a/hw/audio/adlib.c
> +++ b/hw/audio/adlib.c
> @@ -25,7 +25,6 @@
>  #include "qemu/osdep.h"
>  #include "qapi/error.h"
>  #include "qemu/module.h"
> -#include "hw/audio/soundhw.h"
>  #include "audio/audio.h"
>  #include "hw/isa/isa.h"
>  #include "hw/qdev-properties.h"
> @@ -322,7 +321,6 @@ static const TypeInfo adlib_info = {
>  static void adlib_register_types (void)
>  {
>      type_register_static (&adlib_info);
> -    deprecated_register_soundhw("adlib", ADLIB_DESC, 1, TYPE_ADLIB);
>  }
>  
>  type_init (adlib_register_types)
> diff --git a/hw/audio/cs4231a.c b/hw/audio/cs4231a.c
> index 0723e3943044..f510b862efbe 100644
> --- a/hw/audio/cs4231a.c
> +++ b/hw/audio/cs4231a.c
> @@ -23,7 +23,6 @@
>   */
>  
>  #include "qemu/osdep.h"
> -#include "hw/audio/soundhw.h"
>  #include "audio/audio.h"
>  #include "hw/irq.h"
>  #include "hw/isa/isa.h"
> @@ -717,7 +716,6 @@ static const TypeInfo cs4231a_info = {
>  static void cs4231a_register_types (void)
>  {
>      type_register_static (&cs4231a_info);
> -    deprecated_register_soundhw("cs4231a", "CS4231A", 1, TYPE_CS4231A);
>  }
>  
>  type_init (cs4231a_register_types)
> diff --git a/hw/audio/es1370.c b/hw/audio/es1370.c
> index 690458981471..62359b84f279 100644
> --- a/hw/audio/es1370.c
> +++ b/hw/audio/es1370.c
> @@ -27,7 +27,6 @@
>  #define SILENT_ES1370
>  
>  #include "qemu/osdep.h"
> -#include "hw/audio/soundhw.h"
>  #include "audio/audio.h"
>  #include "hw/pci/pci.h"
>  #include "migration/vmstate.h"
> @@ -923,8 +922,6 @@ static const TypeInfo es1370_info = {
>  static void es1370_register_types (void)
>  {
>      type_register_static (&es1370_info);
> -    deprecated_register_soundhw("es1370", "ENSONIQ AudioPCI ES1370",
> -                                0, TYPE_ES1370);
>  }
>  
>  type_init (es1370_register_types)
> diff --git a/hw/audio/gus.c b/hw/audio/gus.c
> index 42f010b67193..c7710a684b88 100644
> --- a/hw/audio/gus.c
> +++ b/hw/audio/gus.c
> @@ -25,7 +25,6 @@
>  #include "qemu/osdep.h"
>  #include "qapi/error.h"
>  #include "qemu/module.h"
> -#include "hw/audio/soundhw.h"
>  #include "audio/audio.h"
>  #include "hw/irq.h"
>  #include "hw/isa/isa.h"
> @@ -317,7 +316,6 @@ static const TypeInfo gus_info = {
>  static void gus_register_types (void)
>  {
>      type_register_static (&gus_info);
> -    deprecated_register_soundhw("gus", "Gravis Ultrasound GF1", 1, TYPE_GUS);
>  }
>  
>  type_init (gus_register_types)
> diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
> index bc77e3d8c9dc..e77552363a4c 100644
> --- a/hw/audio/intel-hda.c
> +++ b/hw/audio/intel-hda.c
> @@ -26,7 +26,6 @@
>  #include "qemu/log.h"
>  #include "qemu/module.h"
>  #include "qemu/error-report.h"
> -#include "hw/audio/soundhw.h"
>  #include "intel-hda.h"
>  #include "migration/vmstate.h"
>  #include "intel-hda-defs.h"
> @@ -1307,25 +1306,6 @@ static const TypeInfo hda_codec_device_type_info = {
>      .class_init = hda_codec_device_class_init,
>  };
>  
> -/*
> - * create intel hda controller with codec attached to it,
> - * so '-soundhw hda' works.
> - */
> -static int intel_hda_and_codec_init(PCIBus *bus)
> -{
> -    DeviceState *controller;
> -    BusState *hdabus;
> -    DeviceState *codec;
> -
> -    warn_report("'-soundhw hda' is deprecated, "
> -                "please use '-device intel-hda -device hda-duplex' instead");
> -    controller = DEVICE(pci_create_simple(bus, -1, "intel-hda"));
> -    hdabus = QLIST_FIRST(&controller->child_bus);
> -    codec = qdev_new("hda-duplex");
> -    qdev_realize_and_unref(codec, hdabus, &error_fatal);
> -    return 0;
> -}
> -
>  static void intel_hda_register_types(void)
>  {
>      type_register_static(&hda_codec_bus_info);
> @@ -1333,7 +1313,6 @@ static void intel_hda_register_types(void)
>      type_register_static(&intel_hda_info_ich6);
>      type_register_static(&intel_hda_info_ich9);
>      type_register_static(&hda_codec_device_type_info);
> -    pci_register_soundhw("hda", "Intel HD Audio", intel_hda_and_codec_init);
>  }
>  
>  type_init(intel_hda_register_types)
> diff --git a/hw/audio/meson.build b/hw/audio/meson.build
> index e48a9fc73d57..746b5b98d198 100644
> --- a/hw/audio/meson.build
> +++ b/hw/audio/meson.build
> @@ -1,4 +1,3 @@
> -softmmu_ss.add(files('soundhw.c'))
>  softmmu_ss.add(when: 'CONFIG_AC97', if_true: files('ac97.c'))
>  softmmu_ss.add(when: 'CONFIG_ADLIB', if_true: files('fmopl.c', 'adlib.c'))
>  softmmu_ss.add(when: 'CONFIG_CS4231', if_true: files('cs4231.c'))
> diff --git a/hw/audio/pcspk.c b/hw/audio/pcspk.c
> index dfc7ebca4e17..72bea5fb202a 100644
> --- a/hw/audio/pcspk.c
> +++ b/hw/audio/pcspk.c
> @@ -24,7 +24,6 @@
>  
>  #include "qemu/osdep.h"
>  #include "hw/isa/isa.h"
> -#include "hw/audio/soundhw.h"
>  #include "audio/audio.h"
>  #include "qemu/module.h"
>  #include "qemu/timer.h"
> @@ -245,18 +244,8 @@ static const TypeInfo pcspk_info = {
>      .class_init     = pcspk_class_initfn,
>  };
>  
> -static int pcspk_audio_init_soundhw(ISABus *bus)
> -{
> -    PCSpkState *s = pcspk_state;
> -
> -    warn_report("'-soundhw pcspk' is deprecated, "
> -                "please set a backend using '-machine pcspk-audiodev=<name>' instead");
> -    return pcspk_audio_init(s);
> -}
> -
>  static void pcspk_register(void)
>  {
>      type_register_static(&pcspk_info);
> -    isa_register_soundhw("pcspk", "PC speaker", pcspk_audio_init_soundhw);
>  }
>  type_init(pcspk_register)
> diff --git a/hw/audio/sb16.c b/hw/audio/sb16.c
> index 2215386ddb2e..593da2478c14 100644
> --- a/hw/audio/sb16.c
> +++ b/hw/audio/sb16.c
> @@ -23,7 +23,6 @@
>   */
>  
>  #include "qemu/osdep.h"
> -#include "hw/audio/soundhw.h"
>  #include "audio/audio.h"
>  #include "hw/irq.h"
>  #include "hw/isa/isa.h"
> @@ -1469,8 +1468,6 @@ static const TypeInfo sb16_info = {
>  static void sb16_register_types (void)
>  {
>      type_register_static (&sb16_info);
> -    deprecated_register_soundhw("sb16", "Creative Sound Blaster 16",
> -                                1, TYPE_SB16);
>  }
>  
>  type_init (sb16_register_types)
> diff --git a/hw/audio/soundhw.c b/hw/audio/soundhw.c
> deleted file mode 100644
> index 173b674ff53a..000000000000
> --- a/hw/audio/soundhw.c
> +++ /dev/null
> @@ -1,177 +0,0 @@
> -/*
> - * QEMU System Emulator
> - *
> - * Copyright (c) 2003-2008 Fabrice Bellard
> - *
> - * 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 "qemu/option.h"
> -#include "qemu/help_option.h"
> -#include "qemu/error-report.h"
> -#include "qom/object.h"
> -#include "hw/isa/isa.h"
> -#include "hw/pci/pci.h"
> -#include "hw/audio/soundhw.h"
> -
> -struct soundhw {
> -    const char *name;
> -    const char *descr;
> -    const char *typename;
> -    int enabled;
> -    int isa;
> -    union {
> -        int (*init_isa) (ISABus *bus);
> -        int (*init_pci) (PCIBus *bus);
> -    } init;
> -};
> -
> -static struct soundhw soundhw[9];
> -static int soundhw_count;
> -
> -void isa_register_soundhw(const char *name, const char *descr,
> -                          int (*init_isa)(ISABus *bus))
> -{
> -    assert(soundhw_count < ARRAY_SIZE(soundhw) - 1);
> -    soundhw[soundhw_count].name = name;
> -    soundhw[soundhw_count].descr = descr;
> -    soundhw[soundhw_count].isa = 1;
> -    soundhw[soundhw_count].init.init_isa = init_isa;
> -    soundhw_count++;
> -}
> -
> -void pci_register_soundhw(const char *name, const char *descr,
> -                          int (*init_pci)(PCIBus *bus))
> -{
> -    assert(soundhw_count < ARRAY_SIZE(soundhw) - 1);
> -    soundhw[soundhw_count].name = name;
> -    soundhw[soundhw_count].descr = descr;
> -    soundhw[soundhw_count].isa = 0;
> -    soundhw[soundhw_count].init.init_pci = init_pci;
> -    soundhw_count++;
> -}
> -
> -void deprecated_register_soundhw(const char *name, const char *descr,
> -                                 int isa, const char *typename)
> -{
> -    assert(soundhw_count < ARRAY_SIZE(soundhw) - 1);
> -    soundhw[soundhw_count].name = name;
> -    soundhw[soundhw_count].descr = descr;
> -    soundhw[soundhw_count].isa = isa;
> -    soundhw[soundhw_count].typename = typename;
> -    soundhw_count++;
> -}
> -
> -void select_soundhw(const char *optarg)
> -{
> -    struct soundhw *c;
> -
> -    if (is_help_option(optarg)) {
> -    show_valid_cards:
> -
> -        if (soundhw_count) {
> -             printf("Valid sound card names (comma separated):\n");
> -             for (c = soundhw; c->name; ++c) {
> -                 printf ("%-11s %s\n", c->name, c->descr);
> -             }
> -             printf("\n-soundhw all will enable all of the above\n");
> -        } else {
> -             printf("Machine has no user-selectable audio hardware "
> -                    "(it may or may not have always-present audio hardware).\n");
> -        }
> -        exit(!is_help_option(optarg));
> -    }
> -    else {
> -        size_t l;
> -        const char *p;
> -        char *e;
> -        int bad_card = 0;
> -
> -        if (!strcmp(optarg, "all")) {
> -            for (c = soundhw; c->name; ++c) {
> -                c->enabled = 1;
> -            }
> -            return;
> -        }
> -
> -        p = optarg;
> -        while (*p) {
> -            e = strchr(p, ',');
> -            l = !e ? strlen(p) : (size_t) (e - p);
> -
> -            for (c = soundhw; c->name; ++c) {
> -                if (!strncmp(c->name, p, l) && !c->name[l]) {
> -                    c->enabled = 1;
> -                    break;
> -                }
> -            }
> -
> -            if (!c->name) {
> -                if (l > 80) {
> -                    error_report("Unknown sound card name (too big to show)");
> -                }
> -                else {
> -                    error_report("Unknown sound card name `%.*s'",
> -                                 (int) l, p);
> -                }
> -                bad_card = 1;
> -            }
> -            p += l + (e != NULL);
> -        }
> -
> -        if (bad_card) {
> -            goto show_valid_cards;
> -        }
> -    }
> -}
> -
> -void soundhw_init(void)
> -{
> -    struct soundhw *c;
> -    ISABus *isa_bus = (ISABus *) object_resolve_path_type("", TYPE_ISA_BUS, NULL);
> -    PCIBus *pci_bus = (PCIBus *) object_resolve_path_type("", TYPE_PCI_BUS, NULL);
> -
> -    for (c = soundhw; c->name; ++c) {
> -        if (c->enabled) {
> -            if (c->typename) {
> -                warn_report("'-soundhw %s' is deprecated, "
> -                            "please use '-device %s' instead",
> -                            c->name, c->typename);
> -                if (c->isa) {
> -                    isa_create_simple(isa_bus, c->typename);
> -                } else {
> -                    pci_create_simple(pci_bus, -1, c->typename);
> -                }
> -            } else if (c->isa) {
> -                if (!isa_bus) {
> -                    error_report("ISA bus not available for %s", c->name);
> -                    exit(1);
> -                }
> -                c->init.init_isa(isa_bus);
> -            } else {
> -                if (!pci_bus) {
> -                    error_report("PCI bus not available for %s", c->name);
> -                    exit(1);
> -                }
> -                c->init.init_pci(pci_bus);
> -            }
> -        }
> -    }
> -}
> -
> diff --git a/include/hw/audio/soundhw.h b/include/hw/audio/soundhw.h
> deleted file mode 100644
> index f09a297854af..000000000000
> --- a/include/hw/audio/soundhw.h
> +++ /dev/null
> @@ -1,15 +0,0 @@
> -#ifndef HW_SOUNDHW_H
> -#define HW_SOUNDHW_H
> -
> -void isa_register_soundhw(const char *name, const char *descr,
> -                          int (*init_isa)(ISABus *bus));
> -
> -void pci_register_soundhw(const char *name, const char *descr,
> -                          int (*init_pci)(PCIBus *bus));
> -void deprecated_register_soundhw(const char *name, const char *descr,
> -                                 int isa, const char *typename);
> -
> -void soundhw_init(void);
> -void select_soundhw(const char *optarg);
> -
> -#endif
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 34e9b32a5c00..47bbd86a4533 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -892,33 +892,6 @@ SRST
>          ``qemu.wav``.
>  ERST
>  
> -DEF("soundhw", HAS_ARG, QEMU_OPTION_soundhw,
> -    "-soundhw c1,... enable audio support\n"
> -    "                and only specified sound cards (comma separated list)\n"
> -    "                use '-soundhw help' to get the list of supported cards\n"
> -    "                use '-soundhw all' to enable all of them\n", QEMU_ARCH_ALL)
> -SRST
> -``-soundhw card1[,card2,...] or -soundhw all``
> -    Enable audio and selected sound hardware. Use 'help' to print all
> -    available sound hardware. For example:
> -
> -    .. parsed-literal::
> -
> -        |qemu_system_x86| -soundhw sb16,adlib disk.img
> -        |qemu_system_x86| -soundhw es1370 disk.img
> -        |qemu_system_x86| -soundhw ac97 disk.img
> -        |qemu_system_x86| -soundhw hda disk.img
> -        |qemu_system_x86| -soundhw all disk.img
> -        |qemu_system_x86| -soundhw help
> -
> -    Note that Linux's i810\_audio OSS kernel (for AC97) module might
> -    require manually specifying clocking.
> -
> -    ::
> -
> -        modprobe i810_audio clocking=48000
> -ERST
> -
>  DEF("device", HAS_ARG, QEMU_OPTION_device,
>      "-device driver[,prop[=value][,...]]\n"
>      "                add device (based on driver)\n"
> diff --git a/scripts/codeconverter/codeconverter/test_regexps.py b/scripts/codeconverter/codeconverter/test_regexps.py
> index a445634d88ac..0161cadffee6 100644
> --- a/scripts/codeconverter/codeconverter/test_regexps.py
> +++ b/scripts/codeconverter/codeconverter/test_regexps.py
> @@ -264,7 +264,6 @@ def test_initial_includes():
>  #define SILENT_ES1370
>  
>  #include "qemu/osdep.h"
> -#include "hw/audio/soundhw.h"
>  #include "audio/audio.h"
>  #include "hw/pci/pci.h"
>  #include "migration/vmstate.h"
> diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
> index 12fe60c4670d..5c80d135ebf0 100644
> --- a/softmmu/qdev-monitor.c
> +++ b/softmmu/qdev-monitor.c
> @@ -66,9 +66,7 @@ typedef struct QDevAlias
>  
>  /* Please keep this table sorted by typename. */
>  static const QDevAlias qdev_alias_table[] = {
> -    { "AC97", "ac97" }, /* -soundhw name */
>      { "e1000", "e1000-82540em" },
> -    { "ES1370", "es1370" }, /* -soundhw name */
>      { "ich9-ahci", "ahci" },
>      { "lsi53c895a", "lsi" },
>      { "virtio-9p-device", "virtio-9p", QEMU_ARCH_VIRTIO_MMIO },
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index 06a0e342fe9e..2f7158574e4f 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -80,7 +80,6 @@
>  #include "migration/snapshot.h"
>  #include "sysemu/tpm.h"
>  #include "sysemu/dma.h"
> -#include "hw/audio/soundhw.h"
>  #include "audio/audio.h"
>  #include "sysemu/cpus.h"
>  #include "sysemu/cpu-timers.h"
> @@ -2669,8 +2668,6 @@ static void qemu_create_cli_devices(void)
>  {
>      DeviceOption *opt;
>  
> -    soundhw_init();
> -
>      qemu_opts_foreach(qemu_find_opts("fw_cfg"),
>                        parse_fw_cfg, fw_cfg_find(), &error_fatal);
>  
> @@ -3018,9 +3015,6 @@ void qemu_init(int argc, char **argv, char **envp)
>              case QEMU_OPTION_audiodev:
>                  audio_parse_option(optarg);
>                  break;
> -            case QEMU_OPTION_soundhw:
> -                select_soundhw (optarg);
> -                break;
>              case QEMU_OPTION_h:
>                  help(0);
>                  break;
> -- 
> 2.35.1
> 

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

Re: [PATCH 02/18] hw/input/tsc210x: Extract common init code into new function

[Daniel P. Berrangé]

On Mon, Apr 25, 2022 at 10:21:45AM +0200, Martin Kletzander wrote:
> This deduplicates several lines and will make future changes more
> concise.
> 
> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> ---
>  hw/input/tsc210x.c | 68 ++++++++++++++++------------------------------
>  1 file changed, 24 insertions(+), 44 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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

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

Re: [PATCH 03/18] hw/audio: Simplify hda audio init

[Daniel P. Berrangé]

On Mon, Apr 25, 2022 at 10:21:46AM +0200, Martin Kletzander wrote:
> No return values are used anywhere, so switch the functions to be void
> and add support for error reporting using errp for use in next patches.

Making it 'void' makes sense, but I'm not sure we need the Error **errp
addition, given my comment on patch #13.

> 
> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> ---
>  hw/audio/hda-codec.c | 32 ++++++++++++++++++--------------
>  hw/audio/intel-hda.c |  4 +---
>  hw/audio/intel-hda.h |  2 +-
>  3 files changed, 20 insertions(+), 18 deletions(-)

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

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

Re: [PATCH 05/18] tests/qtest: Specify audiodev= and -audiodev

[Daniel P. Berrangé]

On Mon, Apr 25, 2022 at 10:21:48AM +0200, Martin Kletzander wrote:
> This will enable removing deprecated default audiodev support.
> 
> I did not figure out how to make the audiodev represented as an
> interface node, so this is a workaround.  I am not sure what would be
> the proper way.

Not sure I understand what you mean by this 'interface node' reference ?

The code looks fine though

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

> 
> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> ---
>  tests/qtest/ac97-test.c                 |  3 ++-
>  tests/qtest/es1370-test.c               |  3 ++-
>  tests/qtest/fuzz/generic_fuzz_configs.h |  6 ++++--
>  tests/qtest/intel-hda-test.c            | 15 ++++++++++-----
>  4 files changed, 18 insertions(+), 9 deletions(-)
> 
> diff --git a/tests/qtest/ac97-test.c b/tests/qtest/ac97-test.c
> index e09f2495d24d..9711f1f6d966 100644
> --- a/tests/qtest/ac97-test.c
> +++ b/tests/qtest/ac97-test.c
> @@ -45,7 +45,8 @@ static void *ac97_create(void *pci_bus, QGuestAllocator *alloc, void *addr)
>  static void ac97_register_nodes(void)
>  {
>      QOSGraphEdgeOptions opts = {
> -        .extra_device_opts = "addr=04.0",
> +        .extra_device_opts = "addr=04.0,audiodev=audio0",
> +        .before_cmd_line = "-audiodev driver=none,id=audio0",
>      };
>      add_qpci_address(&opts, &(QPCIAddress) { .devfn = QPCI_DEVFN(4, 0) });
>  
> diff --git a/tests/qtest/es1370-test.c b/tests/qtest/es1370-test.c
> index 2fd7fd2d3d30..5facda8d0d8d 100644
> --- a/tests/qtest/es1370-test.c
> +++ b/tests/qtest/es1370-test.c
> @@ -46,7 +46,8 @@ static void *es1370_create(void *pci_bus, QGuestAllocator *alloc, void *addr)
>  static void es1370_register_nodes(void)
>  {
>      QOSGraphEdgeOptions opts = {
> -        .extra_device_opts = "addr=04.0",
> +        .extra_device_opts = "addr=04.0,audiodev=audio0",
> +        .before_cmd_line = "-audiodev driver=none,id=audio0",
>      };
>      add_qpci_address(&opts, &(QPCIAddress) { .devfn = QPCI_DEVFN(4, 0) });
>  
> diff --git a/tests/qtest/fuzz/generic_fuzz_configs.h b/tests/qtest/fuzz/generic_fuzz_configs.h
> index 004c701915e1..84a93b3c350b 100644
> --- a/tests/qtest/fuzz/generic_fuzz_configs.h
> +++ b/tests/qtest/fuzz/generic_fuzz_configs.h
> @@ -101,8 +101,10 @@ const generic_fuzz_config predefined_configs[] = {
>      },{
>          .name = "intel-hda",
>          .args = "-machine q35 -nodefaults -device intel-hda,id=hda0 "
> -        "-device hda-output,bus=hda0.0 -device hda-micro,bus=hda0.0 "
> -        "-device hda-duplex,bus=hda0.0",
> +        "-audiodev driver=none,id=audio0",
> +        "-device hda-output,bus=hda0.0,audiodev=audio0 "
> +        "-device hda-micro,bus=hda0.0,audiodev=audio0 "
> +        "-device hda-duplex,bus=hda0.0,audiodev=audio0",
>          .objects = "intel-hda",
>      },{
>          .name = "ide-hd",
> diff --git a/tests/qtest/intel-hda-test.c b/tests/qtest/intel-hda-test.c
> index a58c98e4d11b..39ced2bc6ac6 100644
> --- a/tests/qtest/intel-hda-test.c
> +++ b/tests/qtest/intel-hda-test.c
> @@ -11,20 +11,24 @@
>  #include "libqtest-single.h"
>  
>  #define HDA_ID "hda0"
> -#define CODEC_DEVICES " -device hda-output,bus=" HDA_ID ".0" \
> -                      " -device hda-micro,bus=" HDA_ID ".0" \
> -                      " -device hda-duplex,bus=" HDA_ID ".0"
> +#define AUDIODEV " -audiodev driver=none,id=audio0 "
> +#define AUDIODEV_REF "audiodev=audio0"
> +#define CODEC_DEVICES " -device hda-output,bus=" HDA_ID ".0," AUDIODEV_REF \
> +                      " -device hda-micro,bus=" HDA_ID ".0," AUDIODEV_REF \
> +                      " -device hda-duplex,bus=" HDA_ID ".0," AUDIODEV_REF
>  
>  /* Tests only initialization so far. TODO: Replace with functional tests */
>  static void ich6_test(void)
>  {
> -    qtest_start("-device intel-hda,id=" HDA_ID CODEC_DEVICES);
> +    qtest_start(AUDIODEV "-device intel-hda,id=" HDA_ID CODEC_DEVICES);
>      qtest_end();
>  }
>  
>  static void ich9_test(void)
>  {
> -    qtest_start("-machine q35 -device ich9-intel-hda,bus=pcie.0,addr=1b.0,id="
> +    qtest_start("-machine q35"
> +                AUDIODEV
> +                "-device ich9-intel-hda,bus=pcie.0,addr=1b.0,id="
>                  HDA_ID CODEC_DEVICES);
>      qtest_end();
>  }
> @@ -39,6 +43,7 @@ static void test_issue542_ich6(void)
>      QTestState *s;
>  
>      s = qtest_init("-nographic -nodefaults -M pc-q35-6.2 "
> +                   AUDIODEV
>                     "-device intel-hda,id=" HDA_ID CODEC_DEVICES);
>  
>      qtest_outl(s, 0xcf8, 0x80000804);
> -- 
> 2.35.1
> 

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

Re: [PATCH 06/18] ui/vnc: Require audiodev=

[Daniel P. Berrangé]

On Mon, Apr 25, 2022 at 10:21:49AM +0200, Martin Kletzander wrote:
> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> ---
>  ui/vnc.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/ui/vnc.c b/ui/vnc.c
> index badf1d7664fe..2e7af139b030 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -4188,12 +4188,15 @@ void vnc_display_open(const char *id, Error **errp)
>      vd->ledstate = 0;
>  
>      audiodev = qemu_opt_get(opts, "audiodev");
> -    if (audiodev) {
> -        vd->audio_state = audio_state_by_name(audiodev);
> -        if (!vd->audio_state) {
> -            error_setg(errp, "Audiodev '%s' not found", audiodev);
> -            goto fail;
> -        }
> +    if (!audiodev) {
> +        error_setg(errp, "Audiodev parameter for vnc required");
> +        goto fail;
> +    }

I know we deprecated not setting 'audiodev', but I'm not convinced
this is the right approach.

The VNC audio extension is a custom QEMU invention that few VNC
clients have implemented, and even when implemented few turn it
on as it is pretty awful stuttering.

IMHO a better approach could be to leave audiodev optional, but
stop advertizing VNC_ENCODING_AUDIO when it isn't set.

IOW, current situation

   -vnc :1       -> enables audio capture from default backend

This patch

   -vnc :1       -> error
   -vnc :1,audiodev=audio0 ->  enable audio capture from 'audio0'

Better:

   -vnc :1       -> stop advertizing VNC_ENCODING_AUDIO
   -vnc :1,audiodev=audio0 ->  enable audio capture from 'audio0'


> +
> +    vd->audio_state = audio_state_by_name(audiodev);
> +    if (!vd->audio_state) {
> +        error_setg(errp, "Audiodev '%s' not found", audiodev);
> +        goto fail;
>      }
>  
>      device_id = qemu_opt_get(opts, "display");
> -- 
> 2.35.1
> 

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

Re: [PATCH 07/18] Introduce machine's default-audiodev property

[Daniel P. Berrangé]

On Mon, Apr 25, 2022 at 10:21:50AM +0200, Martin Kletzander wrote:
> Many machine types have default audio devices with no way to set the underlying
> audiodev.  Instead of adding an option for each and every one of them this new
> property can be used as a default during machine initialisation when creating
> such devices.
> 
> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> ---
>  hw/core/machine.c   | 23 +++++++++++++++++++++++
>  include/hw/boards.h |  1 +
>  2 files changed, 24 insertions(+)
> 
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index cb9bbc844d24..d055a126d398 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -596,6 +596,22 @@ static void machine_set_memdev(Object *obj, const char *value, Error **errp)
>      ms->ram_memdev_id = g_strdup(value);
>  }
>  
> +static char *machine_get_default_audiodev(Object *obj, Error **errp)
> +{
> +    MachineState *ms = MACHINE(obj);
> +
> +    return g_strdup(ms->default_audiodev);
> +}
> +
> +static void machine_set_default_audiodev(Object *obj, const char *value,
> +                                         Error **errp)
> +{
> +    MachineState *ms = MACHINE(obj);
> +
> +    g_free(ms->default_audiodev);
> +    ms->default_audiodev = g_strdup(value);
> +}
> +
>  HotpluggableCPUList *machine_query_hotpluggable_cpus(MachineState *machine)
>  {
>      int i;
> @@ -867,6 +883,12 @@ static void machine_class_init(ObjectClass *oc, void *data)
>      object_class_property_set_description(oc, "confidential-guest-support",
>                                            "Set confidential guest scheme to support");
>  
> +    object_class_property_add_str(oc, "default-audiodev",
> +                                  machine_get_default_audiodev,
> +                                  machine_set_default_audiodev);
> +    object_class_property_set_description(oc, "default-audiodev",
> +                                          "Audiodev to use for default machine devices");
> +

Hmm, if we add this, people might reasonably question why this default
audiodev can't be used for everything, instead of making other 'audiodev'
parameter mandatory.

For the x86 machines we have a property tied specifically to the PC
speaker.

  -machine ...,pcspk-audiodev=<name>


If we assume that's the desired pattern, then every machine which has
a built-in audio device should gain some  "{$device}-audiodev" proprerty
where '$device' is some reasonable name for the built-in audio device
of the machine.  This would work better if a machine ended up with
two built-in audio devices and needed separate audiodevs for them.


>      /* For compatibility */
>      object_class_property_add_str(oc, "memory-encryption",
>          machine_get_memory_encryption, machine_set_memory_encryption);
> @@ -961,6 +983,7 @@ static void machine_finalize(Object *obj)
>      g_free(ms->device_memory);
>      g_free(ms->nvdimms_state);
>      g_free(ms->numa_state);
> +    g_free(ms->default_audiodev);
>  }
>  
>  bool machine_usb(MachineState *machine)
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index d64b5481e834..5be1de50af03 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -346,6 +346,7 @@ struct MachineState {
>       */
>      MemoryRegion *ram;
>      DeviceMemoryState *device_memory;
> +    char *default_audiodev;
>  
>      ram_addr_t ram_size;
>      ram_addr_t maxram_size;
> -- 
> 2.35.1
> 

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

Re: [PATCH 03/18] hw/audio: Simplify hda audio init

[Daniel P. Berrangé]

On Mon, Apr 25, 2022 at 02:35:03PM +0100, Daniel P. Berrangé wrote:
> On Mon, Apr 25, 2022 at 10:21:46AM +0200, Martin Kletzander wrote:
> > No return values are used anywhere, so switch the functions to be void
> > and add support for error reporting using errp for use in next patches.
> 
> Making it 'void' makes sense, but I'm not sure we need the Error **errp
> addition, given my comment on patch #13.

Nevermind, I revised my comment on that patch, so

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

> 
> > 
> > Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> > ---
> >  hw/audio/hda-codec.c | 32 ++++++++++++++++++--------------
> >  hw/audio/intel-hda.c |  4 +---
> >  hw/audio/intel-hda.h |  2 +-
> >  3 files changed, 20 insertions(+), 18 deletions(-)
> 
> 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 :|

Re: [PATCH 04/18] hw/audio/lm4549: Add errp error reporting to init function

[Daniel P. Berrangé]

On Mon, Apr 25, 2022 at 10:21:47AM +0200, Martin Kletzander wrote:
> This will be used in future commit.
> 
> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> ---
>  hw/audio/lm4549.c | 3 ++-
>  hw/audio/lm4549.h | 3 ++-
>  hw/audio/pl041.c  | 2 +-
>  3 files changed, 5 insertions(+), 3 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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

Re: [PATCH 14/18] audio: Require AudioState in AUD_add_capture

[Daniel P. Berrangé]

On Mon, Apr 25, 2022 at 10:21:57AM +0200, Martin Kletzander wrote:
> Since all callers require a valid audiodev this function can now safely
> abort in case of missing AudioState.
> 
> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> ---
>  audio/audio.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/audio/audio.c b/audio/audio.c
> index b95aca444382..97eb645764c1 100644
> --- a/audio/audio.c
> +++ b/audio/audio.c
> @@ -1855,10 +1855,8 @@ CaptureVoiceOut *AUD_add_capture(
>      struct capture_callback *cb;
>  
>      if (!s) {
> -        if (!legacy_config) {
> -            dolog("Capturing without setting an audiodev is deprecated\n");
> -        }
> -        s = audio_init(NULL, NULL);
> +        error_setg(&error_abort,
> +                   "Capturing without setting an audiodev is not supported");

Preferrable to do:

  error_report("Capturing ...");
  abort();

Or pass in an 'Error **errp' parameter and let the caller decide
what todo

>      }
>  
>      if (!audio_get_pdo_out(s->dev)->mixing_engine) {
> -- 
> 2.35.1
> 

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

Re: [PATCH 15/18] audio: Be more strict during audio backend initialisation

[Daniel P. Berrangé]

On Mon, Apr 25, 2022 at 10:21:58AM +0200, Martin Kletzander wrote:
> Now that audiodev= is required and audio_init() will not be called
> without and AudioDev we can remove the fallback functionality and error
> out in case audio drivers fail initialisation or when the driver does
> not exist.
> 
> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> ---
>  audio/audio.c                   | 146 ++++++--------------------------
>  docs/about/deprecated.rst       |   8 --
>  docs/about/removed-features.rst |   8 ++
>  3 files changed, 34 insertions(+), 128 deletions(-)
> 
> diff --git a/audio/audio.c b/audio/audio.c
> index 97eb645764c1..c944cf817cf9 100644
> --- a/audio/audio.c
> +++ b/audio/audio.c
> @@ -87,6 +87,8 @@ audio_driver *audio_driver_lookup(const char *name)
>          }
>      }
>  
> +    error_setg(&error_fatal, "Unknown audio driver `%s'", name);
> +    /* Avoid compiler complaining that we do not return in non-void function */
>      return NULL;
>  }

I'd be inclined to plumb in 'Error **errp' here and in the next two
callers up, and pass &error_fatal from softmmu/vl.c

>  
> @@ -104,8 +106,6 @@ const struct mixeng_volume nominal_volume = {
>  #endif
>  };
>  
> -static bool legacy_config = true;
> -
>  int audio_bug (const char *funcname, int cond)
>  {
>      if (cond) {
> @@ -1532,31 +1532,27 @@ size_t audio_generic_read(HWVoiceIn *hw, void *buf, size_t size)
>      return total;
>  }
>  
> -static int audio_driver_init(AudioState *s, struct audio_driver *drv,
> -                             bool msg, Audiodev *dev)
> +static void audio_driver_init(AudioState *s, struct audio_driver *drv,
> +                              Audiodev *dev)
>  {
>      s->drv_opaque = drv->init(dev);
>  
> -    if (s->drv_opaque) {
> -        if (!drv->pcm_ops->get_buffer_in) {
> -            drv->pcm_ops->get_buffer_in = audio_generic_get_buffer_in;
> -            drv->pcm_ops->put_buffer_in = audio_generic_put_buffer_in;
> -        }
> -        if (!drv->pcm_ops->get_buffer_out) {
> -            drv->pcm_ops->get_buffer_out = audio_generic_get_buffer_out;
> -            drv->pcm_ops->put_buffer_out = audio_generic_put_buffer_out;
> -        }
> +    if (!s->drv_opaque) {
> +        error_setg(&error_fatal, "Could not init `%s' audio driver", drv->name);
> +    }
>  
> -        audio_init_nb_voices_out(s, drv);
> -        audio_init_nb_voices_in(s, drv);
> -        s->drv = drv;
> -        return 0;
> -    } else {
> -        if (msg) {
> -            dolog("Could not init `%s' audio driver\n", drv->name);
> -        }
> -        return -1;
> +    if (!drv->pcm_ops->get_buffer_in) {
> +        drv->pcm_ops->get_buffer_in = audio_generic_get_buffer_in;
> +        drv->pcm_ops->put_buffer_in = audio_generic_put_buffer_in;
>      }
> +    if (!drv->pcm_ops->get_buffer_out) {
> +        drv->pcm_ops->get_buffer_out = audio_generic_get_buffer_out;
> +        drv->pcm_ops->put_buffer_out = audio_generic_put_buffer_out;
> +    }
> +
> +    audio_init_nb_voices_out(s, drv);
> +    audio_init_nb_voices_in(s, drv);
> +    s->drv = drv;
>  }
>  
>  static void audio_vm_change_state_handler (void *opaque, bool running,
> @@ -1661,79 +1657,19 @@ static const VMStateDescription vmstate_audio = {
>  
>  static void audio_validate_opts(Audiodev *dev, Error **errp);
>  
> -static AudiodevListEntry *audiodev_find(
> -    AudiodevListHead *head, const char *drvname)
> -{
> -    AudiodevListEntry *e;
> -    QSIMPLEQ_FOREACH(e, head, next) {
> -        if (strcmp(AudiodevDriver_str(e->dev->driver), drvname) == 0) {
> -            return e;
> -        }
> -    }
> -
> -    return NULL;
> -}
> -
> -/*
> - * 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
> - */
> -static AudioState *audio_init(Audiodev *dev, const char *name)
> +static AudioState *audio_init(Audiodev *dev)
>  {
>      static bool atexit_registered;
> -    size_t i;
> -    int done = 0;
>      const char *drvname = NULL;
>      VMChangeStateEntry *e;
>      AudioState *s;
> -    struct audio_driver *driver;
> -    /* silence gcc warning about uninitialized variable */
> -    AudiodevListHead head = QSIMPLEQ_HEAD_INITIALIZER(head);
> -
> -    if (using_spice) {
> -        /*
> -         * When using spice allow the spice audio driver being picked
> -         * as default.
> -         *
> -         * Temporary hack.  Using audio devices without explicit
> -         * audiodev= property is already deprecated.  Same goes for
> -         * the -soundhw switch.  Once this support gets finally
> -         * removed we can also drop the concept of a default audio
> -         * backend and this can go away.
> -         */
> -        driver = audio_driver_lookup("spice");
> -        if (driver) {
> -            driver->can_be_default = 1;
> -        }
> -    }
>  
> -    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);
> +    if (!dev) {
> +        error_setg(&error_abort, "Mandatory audiodev parameter required");
>      }
>  
> +    drvname = AudiodevDriver_str(dev->driver);
> +
>      s = g_new0(AudioState, 1);
>      s->dev = dev;
>  
> @@ -1763,37 +1699,7 @@ static AudioState *audio_init(Audiodev *dev, const char *name)
>          s->nb_hw_voices_in = 0;
>      }
>  
> -    if (drvname) {
> -        driver = audio_driver_lookup(drvname);
> -        if (driver) {
> -            done = !audio_driver_init(s, driver, true, dev);
> -        } else {
> -            dolog ("Unknown audio driver `%s'\n", drvname);
> -        }
> -    } else {
> -        for (i = 0; audio_prio_list[i]; i++) {
> -            AudiodevListEntry *e = audiodev_find(&head, audio_prio_list[i]);
> -            driver = audio_driver_lookup(audio_prio_list[i]);
> -
> -            if (e && driver) {
> -                s->dev = dev = e->dev;
> -                audio_validate_opts(dev, &error_abort);
> -                done = !audio_driver_init(s, driver, false, dev);
> -                if (done) {
> -                    e->dev = NULL;
> -                    break;
> -                }
> -            }
> -        }
> -    }
> -    audio_free_audiodev_list(&head);
> -
> -    if (!done) {
> -        driver = audio_driver_lookup("none");
> -        done = !audio_driver_init(s, driver, false, dev);
> -        assert(done);
> -        dolog("warning: Using timer based audio emulation\n");
> -    }
> +    audio_driver_init(s, audio_driver_lookup(drvname), dev);
>  
>      if (dev->timer_period <= 0) {
>          s->period_ticks = 1;
> @@ -2120,7 +2026,7 @@ void audio_init_audiodevs(void)
>      AudiodevListEntry *e;
>  
>      QSIMPLEQ_FOREACH(e, &audiodevs, next) {
> -        audio_init(e->dev, NULL);
> +        audio_init(e->dev);
>      }
>  }
>  
> @@ -2133,7 +2039,7 @@ static void audio_init_dummy(const char *id)
>      dev->id = g_strdup(id);
>  
>      audio_validate_opts(dev, &error_abort);
> -    audio_init(dev, NULL);
> +    audio_init(dev);
>  
>      e->dev = dev;
>      QSIMPLEQ_INSERT_TAIL(&audiodevs, e, next);
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index 7ba71ebd3435..b2255319e347 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -31,14 +31,6 @@ 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)
> -''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
> -
> -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.
> -
>  ``-chardev`` backend aliases ``tty`` and ``parport`` (since 6.0)
>  ''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
>  
> diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
> index 086ba3edb042..f9eea4dda327 100644
> --- a/docs/about/removed-features.rst
> +++ b/docs/about/removed-features.rst
> @@ -346,6 +346,14 @@ needs two devices (``-device intel-hda -device hda-duplex``) and
>  pcspk-audiodev=<name>``.  And ``AC97`` and ``ES1370`` now have to be
>  specified in uppercase.
>  
> +Creating sound card devices and vnc without ``audiodev=`` property (removed in 7.1)
> +'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
> +
> +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.
> +
>  
>  QEMU Machine Protocol (QMP) commands
>  ------------------------------------
> -- 
> 2.35.1
> 

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

Re: [PATCH 16/18] audio: Remove legacy audio environment variables and options

[Daniel P. Berrangé]

On Mon, Apr 25, 2022 at 10:21:59AM +0200, Martin Kletzander wrote:
> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> ---
>  audio/audio.c                   |  13 -
>  audio/audio.h                   |   1 -
>  audio/audio_legacy.c            | 555 --------------------------------
>  audio/meson.build               |   1 -
>  docs/about/deprecated.rst       |   7 -
>  docs/about/removed-features.rst |   9 +
>  qemu-options.hx                 |  10 -
>  softmmu/vl.c                    |   4 -
>  8 files changed, 9 insertions(+), 591 deletions(-)
>  delete mode 100644 audio/audio_legacy.c

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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

Re: [PATCH 17/18] audio: Remove unused can_be_default

[Daniel P. Berrangé]

On Mon, Apr 25, 2022 at 10:22:00AM +0200, Martin Kletzander wrote:
> Since there is no fallback mechanism and default-guessing this is now
> not used and can be safely removed.
> 
> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> ---
>  audio/alsaaudio.c   | 1 -
>  audio/audio_int.h   | 1 -
>  audio/coreaudio.m   | 1 -
>  audio/dbusaudio.c   | 1 -
>  audio/dsoundaudio.c | 1 -
>  audio/jackaudio.c   | 1 -
>  audio/noaudio.c     | 1 -
>  audio/ossaudio.c    | 1 -
>  audio/paaudio.c     | 1 -
>  audio/sdlaudio.c    | 1 -
>  audio/wavaudio.c    | 1 -
>  11 files changed, 11 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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

Re: [PATCH 00/18] RFC: Remove deprecated audio features

[Mark Cave-Ayland]

On 25/04/2022 09:21, Martin Kletzander wrote:

> I wanted to deal with https://bugzilla.redhat.com/2043498 and I got a
> suggesstion that removing deprecated features could actually make it
> easier to propagate the error.  In the end (last patch) it turns out the
> error is still just reported with error_fatal, so it probably is not
> really needed, but I really wanted to dig into QEMU more and learn some
> of the internals for quite some time now.  So I used the opportunity.
> The one-liner ended up being an 18 patch series which was, for someone
> who has just one commit in QEMU codebase, a pretty challenging task.
> Although I tried my best to do things properly, I am not sure whether I
> handled everything correctly, hence the RFC.
> 
> Any comments are very much appreciated.  Thanks and have a nice day ;)
> 
> Martin Kletzander (18):
>    hw/audio: Remove -soundhw support
>    hw/input/tsc210x: Extract common init code into new function
>    hw/audio: Simplify hda audio init
>    hw/audio/lm4549: Add errp error reporting to init function
>    tests/qtest: Specify audiodev= and -audiodev
>    ui/vnc: Require audiodev=
>    Introduce machine's default-audiodev property
>    audio: Add easy dummy audio initialiser
>    hw/display/xlnx_dp.c: Add audiodev property
>    hw/input/tsc210x.c: Support machine-default audiodev with fallback
>    hw/arm: Support machine-default audiodev with fallback
>    hw/ppc: Support machine-default audiodev with fallback
>    audio: Make AUD_register_card fallible and require audiodev=
>    audio: Require AudioState in AUD_add_capture
>    audio: Be more strict during audio backend initialisation
>    audio: Remove legacy audio environment variables and options
>    audio: Remove unused can_be_default
>    audio/spiceaudio: Fail initialisation when not using spice
> 
>   audio/alsaaudio.c                             |   1 -
>   audio/audio.c                                 | 204 +++----
>   audio/audio.h                                 |   5 +-
>   audio/audio_int.h                             |   1 -
>   audio/audio_legacy.c                          | 555 ------------------
>   audio/coreaudio.m                             |   1 -
>   audio/dbusaudio.c                             |   1 -
>   audio/dsoundaudio.c                           |   1 -
>   audio/jackaudio.c                             |   1 -
>   audio/meson.build                             |   1 -
>   audio/noaudio.c                               |   1 -
>   audio/ossaudio.c                              |   1 -
>   audio/paaudio.c                               |   1 -
>   audio/sdlaudio.c                              |   1 -
>   audio/spiceaudio.c                            |   3 +-
>   audio/wavaudio.c                              |   1 -
>   docs/about/deprecated.rst                     |  24 -
>   docs/about/removed-features.rst               |  27 +
>   docs/qdev-device-use.txt                      |  21 +-
>   docs/replay.txt                               |   2 +-
>   hw/arm/integratorcp.c                         |   8 +-
>   hw/arm/musicpal.c                             |   8 +-
>   hw/arm/omap2.c                                |  11 +-
>   hw/arm/realview.c                             |   3 +
>   hw/arm/spitz.c                                |  10 +-
>   hw/arm/versatilepb.c                          |   3 +
>   hw/arm/vexpress.c                             |   3 +
>   hw/arm/xlnx-zcu102.c                          |   4 +
>   hw/arm/z2.c                                   |  12 +-
>   hw/audio/ac97.c                               |   9 +-
>   hw/audio/adlib.c                              |   9 +-
>   hw/audio/cs4231a.c                            |   8 +-
>   hw/audio/es1370.c                             |   8 +-
>   hw/audio/gus.c                                |   6 +-
>   hw/audio/hda-codec.c                          |  37 +-
>   hw/audio/intel-hda.c                          |  25 +-
>   hw/audio/intel-hda.h                          |   2 +-
>   hw/audio/lm4549.c                             |   7 +-
>   hw/audio/lm4549.h                             |   3 +-
>   hw/audio/meson.build                          |   1 -
>   hw/audio/pcspk.c                              |  15 +-
>   hw/audio/pl041.c                              |   2 +-
>   hw/audio/sb16.c                               |   9 +-
>   hw/audio/soundhw.c                            | 177 ------
>   hw/audio/wm8750.c                             |   5 +-
>   hw/core/machine.c                             |  23 +
>   hw/display/xlnx_dp.c                          |  12 +-
>   hw/input/tsc210x.c                            |  79 ++-
>   hw/ppc/prep.c                                 |   4 +
>   hw/usb/dev-audio.c                            |   5 +-
>   include/hw/audio/soundhw.h                    |  15 -
>   include/hw/boards.h                           |   1 +
>   qemu-options.hx                               |  37 --
>   .../codeconverter/test_regexps.py             |   1 -
>   softmmu/qdev-monitor.c                        |   2 -
>   softmmu/vl.c                                  |  10 -
>   tests/qtest/ac97-test.c                       |   3 +-
>   tests/qtest/es1370-test.c                     |   3 +-
>   tests/qtest/fuzz/generic_fuzz_configs.h       |   6 +-
>   tests/qtest/intel-hda-test.c                  |  15 +-
>   ui/vnc.c                                      |  15 +-
>   61 files changed, 329 insertions(+), 1140 deletions(-)
>   delete mode 100644 audio/audio_legacy.c
>   delete mode 100644 hw/audio/soundhw.c
>   delete mode 100644 include/hw/audio/soundhw.h

Thanks for the tidy-up! I'm not too familiar with the audio code, however one thing I 
noticed is that in patch 11 you are using qdev_prop_set_string() to set the audiodev 
for the machines.

For CharDevs there already exists a qdev_prop_set_chr() function which is used to 
assign the chardev backend to the device, so I'm wondering if it makes sense to add a 
similar qdev_prop_set_audiodev() function in the same way? Then if the method of 
referencing the audiodev from the device ever changes from being a string containing 
the name, it won't require updating all of the callers.

Finally a quick glance at the tsc210x and lm4549 devices indicates that they are 
using global device _init() functions which shouldn't really be used in modern 
implementations. If you are interested in QOMify-ing these devices as part of this 
work, then I can certainly help provide some pointers for getting started.


ATB,

Mark.

Re: [PATCH 01/18] hw/audio: Remove -soundhw support

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1579 bytes --]

On Mon, Apr 25, 2022 at 10:21:44AM +0200, Martin Kletzander wrote:
> One thing I am not sure about is whether to keep the aliases of ac97 and
> es1370 in the qdev_alias_table.
> 
> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> ---
>  docs/about/deprecated.rst                     |   9 -
>  docs/about/removed-features.rst               |  10 +
>  docs/qdev-device-use.txt                      |  21 +--
>  docs/replay.txt                               |   2 +-
>  hw/audio/ac97.c                               |   3 -
>  hw/audio/adlib.c                              |   2 -
>  hw/audio/cs4231a.c                            |   2 -
>  hw/audio/es1370.c                             |   3 -
>  hw/audio/gus.c                                |   2 -
>  hw/audio/intel-hda.c                          |  21 ---
>  hw/audio/meson.build                          |   1 -
>  hw/audio/pcspk.c                              |  11 --
>  hw/audio/sb16.c                               |   3 -
>  hw/audio/soundhw.c                            | 177 ------------------
>  include/hw/audio/soundhw.h                    |  15 --
>  qemu-options.hx                               |  27 ---
>  .../codeconverter/test_regexps.py             |   1 -
>  softmmu/qdev-monitor.c                        |   2 -
>  softmmu/vl.c                                  |   6 -
>  19 files changed, 19 insertions(+), 299 deletions(-)
>  delete mode 100644 hw/audio/soundhw.c
>  delete mode 100644 include/hw/audio/soundhw.h

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 00/18] RFC: Remove deprecated audio features

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 992 bytes --]

On Mon, Apr 25, 2022 at 10:21:43AM +0200, Martin Kletzander wrote:
> I wanted to deal with https://bugzilla.redhat.com/2043498 and I got a
> suggesstion that removing deprecated features could actually make it
> easier to propagate the error.  In the end (last patch) it turns out the
> error is still just reported with error_fatal, so it probably is not
> really needed, but I really wanted to dig into QEMU more and learn some
> of the internals for quite some time now.  So I used the opportunity.
> The one-liner ended up being an 18 patch series which was, for someone
> who has just one commit in QEMU codebase, a pretty challenging task.
> Although I tried my best to do things properly, I am not sure whether I
> handled everything correctly, hence the RFC.
> 
> Any comments are very much appreciated.  Thanks and have a nice day ;)

Hi Martin,
The -soundhw removal looks good to me. I'm not familiar with the audio
subsystem so I didn't review the rest.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 06/18] ui/vnc: Require audiodev=

[Paolo Bonzini]

On 4/25/22 10:21, Martin Kletzander wrote:
> @@ -4188,12 +4188,15 @@ void vnc_display_open(const char *id, Error **errp)
>       vd->ledstate = 0;
>   
>       audiodev = qemu_opt_get(opts, "audiodev");
> -    if (audiodev) {
> -        vd->audio_state = audio_state_by_name(audiodev);
> -        if (!vd->audio_state) {
> -            error_setg(errp, "Audiodev '%s' not found", audiodev);
> -            goto fail;
> -        }
> +    if (!audiodev) {
> +        error_setg(errp, "Audiodev parameter for vnc required");
> +        goto fail;
> +    }
> +

Wouldn't this break "-vnc :0"?  You can just ignore the audio commands 
if vd->audio_state is NULL.

Paolo

Re: [PATCH 06/18] ui/vnc: Require audiodev=

[Daniel P. Berrangé]

On Wed, Apr 27, 2022 at 11:32:41AM +0200, Paolo Bonzini wrote:
> On 4/25/22 10:21, Martin Kletzander wrote:
> > @@ -4188,12 +4188,15 @@ void vnc_display_open(const char *id, Error **errp)
> >       vd->ledstate = 0;
> >       audiodev = qemu_opt_get(opts, "audiodev");
> > -    if (audiodev) {
> > -        vd->audio_state = audio_state_by_name(audiodev);
> > -        if (!vd->audio_state) {
> > -            error_setg(errp, "Audiodev '%s' not found", audiodev);
> > -            goto fail;
> > -        }
> > +    if (!audiodev) {
> > +        error_setg(errp, "Audiodev parameter for vnc required");
> > +        goto fail;
> > +    }
> > +
> 
> Wouldn't this break "-vnc :0"?  You can just ignore the audio commands if
> vd->audio_state is NULL.

Yep, that's wha I suggested with skipping advertizing VNC_ENCODING_AUDIO
when audiodev is NULL

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

Re: [PATCH 01/18] hw/audio: Remove -soundhw support

[Paolo Bonzini]

On 4/25/22 10:21, Martin Kletzander wrote:
> One thing I am not sure about is whether to keep the aliases of ac97 and
> es1370 in the qdev_alias_table.
> 
> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>

I agree that -soundhw is a bad option, but I think we should preserve 
something similarly easy to use.  For example:

      -audio OPTIONS,model=XXX

would expand to

      -audiodev OPTIONS,id=audiodev0

and then call the same init_isa/init_pci functions that already exist; 
except that those would add an audiodev=audiodev0 property.

This way, one can use "-audio pa,model=hda".

Later on we can add a hook in the machines for the case when no model is 
specified, but you don't need to do that now because it's backwards 
compatible.

Let me post a sketch after lunch.

Paolo

> ---
>   docs/about/deprecated.rst                     |   9 -
>   docs/about/removed-features.rst               |  10 +
>   docs/qdev-device-use.txt                      |  21 +--
>   docs/replay.txt                               |   2 +-
>   hw/audio/ac97.c                               |   3 -
>   hw/audio/adlib.c                              |   2 -
>   hw/audio/cs4231a.c                            |   2 -
>   hw/audio/es1370.c                             |   3 -
>   hw/audio/gus.c                                |   2 -
>   hw/audio/intel-hda.c                          |  21 ---
>   hw/audio/meson.build                          |   1 -
>   hw/audio/pcspk.c                              |  11 --
>   hw/audio/sb16.c                               |   3 -
>   hw/audio/soundhw.c                            | 177 ------------------
>   include/hw/audio/soundhw.h                    |  15 --
>   qemu-options.hx                               |  27 ---
>   .../codeconverter/test_regexps.py             |   1 -
>   softmmu/qdev-monitor.c                        |   2 -
>   softmmu/vl.c                                  |   6 -
>   19 files changed, 19 insertions(+), 299 deletions(-)
>   delete mode 100644 hw/audio/soundhw.c
>   delete mode 100644 include/hw/audio/soundhw.h
> 
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index cf02ef6821e4..7ba71ebd3435 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -39,15 +39,6 @@ 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.
>   
> -Creating sound card devices using ``-soundhw`` (since 5.1)
> -''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
> -
> -Sound card devices should be created using ``-device`` instead.  The
> -names are the same for most devices.  The exceptions are ``hda`` which
> -needs two devices (``-device intel-hda -device hda-duplex``) and
> -``pcspk`` which can be activated using ``-machine
> -pcspk-audiodev=<name>``.
> -
>   ``-chardev`` backend aliases ``tty`` and ``parport`` (since 6.0)
>   ''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
>   
> diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
> index 4b831ea29176..086ba3edb042 100644
> --- a/docs/about/removed-features.rst
> +++ b/docs/about/removed-features.rst
> @@ -336,6 +336,16 @@ for the RISC-V ``virt`` machine and ``sifive_u`` machine.
>   The ``-no-quit`` was a synonym for ``-display ...,window-close=off`` which
>   should be used instead.
>   
> +Creating sound card devices using ``-soundhw`` (removed in 7.1)
> +'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
> +
> +Sound card devices should be created using ``-device`` instead.  The
> +names are the same for most devices.  The exceptions are ``hda`` which
> +needs two devices (``-device intel-hda -device hda-duplex``) and
> +``pcspk`` which can be activated using ``-machine
> +pcspk-audiodev=<name>``.  And ``AC97`` and ``ES1370`` now have to be
> +specified in uppercase.
> +
>   
>   QEMU Machine Protocol (QMP) commands
>   ------------------------------------
> diff --git a/docs/qdev-device-use.txt b/docs/qdev-device-use.txt
> index 240888933482..30e7eaa3e66d 100644
> --- a/docs/qdev-device-use.txt
> +++ b/docs/qdev-device-use.txt
> @@ -311,21 +311,16 @@ constraints.
>   
>   Host and guest part of audio devices have always been separate.
>   
> -The old way to define guest audio devices is -soundhw C1,...
> +Host side (backend) is defined using -audiodev with a specific driver:
>   
> -The new way is to define each guest audio device separately with
> --device.
> +    spice
> +    pa
> +    none
>   
> -Map from -soundhw sound card name to -device:
> -
> -    ac97        -device AC97
> -    cs4231a     -device cs4231a,iobase=IOADDR,irq=IRQ,dma=DMA
> -    es1370      -device ES1370
> -    gus         -device gus,iobase=IOADDR,irq=IRQ,dma=DMA,freq=F
> -    hda         -device intel-hda,msi=MSI -device hda-duplex
> -    sb16        -device sb16,iobase=IOADDR,irq=IRQ,dma=DMA,dma16=DMA16,version=V
> -    adlib       not yet available with -device
> -    pcspk       not yet available with -device
> +And each guest audio device is then defined with -device with
> +audiodev=AUDIODEV_ID that refers to the audio backend above.  Exceptions are
> +pcspk and adlib which are note yet available with -device and are part of a
> +machine type.
>   
>   For PCI devices, you can add bus=PCI-BUS,addr=DEVFN to control the PCI
>   device address, as usual.
> diff --git a/docs/replay.txt b/docs/replay.txt
> index 5b008ca4911f..c329767c148a 100644
> --- a/docs/replay.txt
> +++ b/docs/replay.txt
> @@ -294,7 +294,7 @@ Audio devices
>   
>   Audio data is recorded and replay automatically. The command line for recording
>   and replaying must contain identical specifications of audio hardware, e.g.:
> - -soundhw ac97
> + -audiodev driver=pa -device ac97,audiodev=audio0
>   
>   Serial ports
>   ------------
> diff --git a/hw/audio/ac97.c b/hw/audio/ac97.c
> index 3cb81310607f..fd0b3b97d5b5 100644
> --- a/hw/audio/ac97.c
> +++ b/hw/audio/ac97.c
> @@ -18,7 +18,6 @@
>    */
>   
>   #include "qemu/osdep.h"
> -#include "hw/audio/soundhw.h"
>   #include "audio/audio.h"
>   #include "hw/pci/pci.h"
>   #include "hw/qdev-properties.h"
> @@ -1430,8 +1429,6 @@ static const TypeInfo ac97_info = {
>   static void ac97_register_types (void)
>   {
>       type_register_static (&ac97_info);
> -    deprecated_register_soundhw("ac97", "Intel 82801AA AC97 Audio",
> -                                0, TYPE_AC97);
>   }
>   
>   type_init (ac97_register_types)
> diff --git a/hw/audio/adlib.c b/hw/audio/adlib.c
> index 5f979b1487d1..ba1be6c8378d 100644
> --- a/hw/audio/adlib.c
> +++ b/hw/audio/adlib.c
> @@ -25,7 +25,6 @@
>   #include "qemu/osdep.h"
>   #include "qapi/error.h"
>   #include "qemu/module.h"
> -#include "hw/audio/soundhw.h"
>   #include "audio/audio.h"
>   #include "hw/isa/isa.h"
>   #include "hw/qdev-properties.h"
> @@ -322,7 +321,6 @@ static const TypeInfo adlib_info = {
>   static void adlib_register_types (void)
>   {
>       type_register_static (&adlib_info);
> -    deprecated_register_soundhw("adlib", ADLIB_DESC, 1, TYPE_ADLIB);
>   }
>   
>   type_init (adlib_register_types)
> diff --git a/hw/audio/cs4231a.c b/hw/audio/cs4231a.c
> index 0723e3943044..f510b862efbe 100644
> --- a/hw/audio/cs4231a.c
> +++ b/hw/audio/cs4231a.c
> @@ -23,7 +23,6 @@
>    */
>   
>   #include "qemu/osdep.h"
> -#include "hw/audio/soundhw.h"
>   #include "audio/audio.h"
>   #include "hw/irq.h"
>   #include "hw/isa/isa.h"
> @@ -717,7 +716,6 @@ static const TypeInfo cs4231a_info = {
>   static void cs4231a_register_types (void)
>   {
>       type_register_static (&cs4231a_info);
> -    deprecated_register_soundhw("cs4231a", "CS4231A", 1, TYPE_CS4231A);
>   }
>   
>   type_init (cs4231a_register_types)
> diff --git a/hw/audio/es1370.c b/hw/audio/es1370.c
> index 690458981471..62359b84f279 100644
> --- a/hw/audio/es1370.c
> +++ b/hw/audio/es1370.c
> @@ -27,7 +27,6 @@
>   #define SILENT_ES1370
>   
>   #include "qemu/osdep.h"
> -#include "hw/audio/soundhw.h"
>   #include "audio/audio.h"
>   #include "hw/pci/pci.h"
>   #include "migration/vmstate.h"
> @@ -923,8 +922,6 @@ static const TypeInfo es1370_info = {
>   static void es1370_register_types (void)
>   {
>       type_register_static (&es1370_info);
> -    deprecated_register_soundhw("es1370", "ENSONIQ AudioPCI ES1370",
> -                                0, TYPE_ES1370);
>   }
>   
>   type_init (es1370_register_types)
> diff --git a/hw/audio/gus.c b/hw/audio/gus.c
> index 42f010b67193..c7710a684b88 100644
> --- a/hw/audio/gus.c
> +++ b/hw/audio/gus.c
> @@ -25,7 +25,6 @@
>   #include "qemu/osdep.h"
>   #include "qapi/error.h"
>   #include "qemu/module.h"
> -#include "hw/audio/soundhw.h"
>   #include "audio/audio.h"
>   #include "hw/irq.h"
>   #include "hw/isa/isa.h"
> @@ -317,7 +316,6 @@ static const TypeInfo gus_info = {
>   static void gus_register_types (void)
>   {
>       type_register_static (&gus_info);
> -    deprecated_register_soundhw("gus", "Gravis Ultrasound GF1", 1, TYPE_GUS);
>   }
>   
>   type_init (gus_register_types)
> diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
> index bc77e3d8c9dc..e77552363a4c 100644
> --- a/hw/audio/intel-hda.c
> +++ b/hw/audio/intel-hda.c
> @@ -26,7 +26,6 @@
>   #include "qemu/log.h"
>   #include "qemu/module.h"
>   #include "qemu/error-report.h"
> -#include "hw/audio/soundhw.h"
>   #include "intel-hda.h"
>   #include "migration/vmstate.h"
>   #include "intel-hda-defs.h"
> @@ -1307,25 +1306,6 @@ static const TypeInfo hda_codec_device_type_info = {
>       .class_init = hda_codec_device_class_init,
>   };
>   
> -/*
> - * create intel hda controller with codec attached to it,
> - * so '-soundhw hda' works.
> - */
> -static int intel_hda_and_codec_init(PCIBus *bus)
> -{
> -    DeviceState *controller;
> -    BusState *hdabus;
> -    DeviceState *codec;
> -
> -    warn_report("'-soundhw hda' is deprecated, "
> -                "please use '-device intel-hda -device hda-duplex' instead");
> -    controller = DEVICE(pci_create_simple(bus, -1, "intel-hda"));
> -    hdabus = QLIST_FIRST(&controller->child_bus);
> -    codec = qdev_new("hda-duplex");
> -    qdev_realize_and_unref(codec, hdabus, &error_fatal);
> -    return 0;
> -}
> -
>   static void intel_hda_register_types(void)
>   {
>       type_register_static(&hda_codec_bus_info);
> @@ -1333,7 +1313,6 @@ static void intel_hda_register_types(void)
>       type_register_static(&intel_hda_info_ich6);
>       type_register_static(&intel_hda_info_ich9);
>       type_register_static(&hda_codec_device_type_info);
> -    pci_register_soundhw("hda", "Intel HD Audio", intel_hda_and_codec_init);
>   }
>   
>   type_init(intel_hda_register_types)
> diff --git a/hw/audio/meson.build b/hw/audio/meson.build
> index e48a9fc73d57..746b5b98d198 100644
> --- a/hw/audio/meson.build
> +++ b/hw/audio/meson.build
> @@ -1,4 +1,3 @@
> -softmmu_ss.add(files('soundhw.c'))
>   softmmu_ss.add(when: 'CONFIG_AC97', if_true: files('ac97.c'))
>   softmmu_ss.add(when: 'CONFIG_ADLIB', if_true: files('fmopl.c', 'adlib.c'))
>   softmmu_ss.add(when: 'CONFIG_CS4231', if_true: files('cs4231.c'))
> diff --git a/hw/audio/pcspk.c b/hw/audio/pcspk.c
> index dfc7ebca4e17..72bea5fb202a 100644
> --- a/hw/audio/pcspk.c
> +++ b/hw/audio/pcspk.c
> @@ -24,7 +24,6 @@
>   
>   #include "qemu/osdep.h"
>   #include "hw/isa/isa.h"
> -#include "hw/audio/soundhw.h"
>   #include "audio/audio.h"
>   #include "qemu/module.h"
>   #include "qemu/timer.h"
> @@ -245,18 +244,8 @@ static const TypeInfo pcspk_info = {
>       .class_init     = pcspk_class_initfn,
>   };
>   
> -static int pcspk_audio_init_soundhw(ISABus *bus)
> -{
> -    PCSpkState *s = pcspk_state;
> -
> -    warn_report("'-soundhw pcspk' is deprecated, "
> -                "please set a backend using '-machine pcspk-audiodev=<name>' instead");
> -    return pcspk_audio_init(s);
> -}
> -
>   static void pcspk_register(void)
>   {
>       type_register_static(&pcspk_info);
> -    isa_register_soundhw("pcspk", "PC speaker", pcspk_audio_init_soundhw);
>   }
>   type_init(pcspk_register)
> diff --git a/hw/audio/sb16.c b/hw/audio/sb16.c
> index 2215386ddb2e..593da2478c14 100644
> --- a/hw/audio/sb16.c
> +++ b/hw/audio/sb16.c
> @@ -23,7 +23,6 @@
>    */
>   
>   #include "qemu/osdep.h"
> -#include "hw/audio/soundhw.h"
>   #include "audio/audio.h"
>   #include "hw/irq.h"
>   #include "hw/isa/isa.h"
> @@ -1469,8 +1468,6 @@ static const TypeInfo sb16_info = {
>   static void sb16_register_types (void)
>   {
>       type_register_static (&sb16_info);
> -    deprecated_register_soundhw("sb16", "Creative Sound Blaster 16",
> -                                1, TYPE_SB16);
>   }
>   
>   type_init (sb16_register_types)
> diff --git a/hw/audio/soundhw.c b/hw/audio/soundhw.c
> deleted file mode 100644
> index 173b674ff53a..000000000000
> --- a/hw/audio/soundhw.c
> +++ /dev/null
> @@ -1,177 +0,0 @@
> -/*
> - * QEMU System Emulator
> - *
> - * Copyright (c) 2003-2008 Fabrice Bellard
> - *
> - * 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 "qemu/option.h"
> -#include "qemu/help_option.h"
> -#include "qemu/error-report.h"
> -#include "qom/object.h"
> -#include "hw/isa/isa.h"
> -#include "hw/pci/pci.h"
> -#include "hw/audio/soundhw.h"
> -
> -struct soundhw {
> -    const char *name;
> -    const char *descr;
> -    const char *typename;
> -    int enabled;
> -    int isa;
> -    union {
> -        int (*init_isa) (ISABus *bus);
> -        int (*init_pci) (PCIBus *bus);
> -    } init;
> -};
> -
> -static struct soundhw soundhw[9];
> -static int soundhw_count;
> -
> -void isa_register_soundhw(const char *name, const char *descr,
> -                          int (*init_isa)(ISABus *bus))
> -{
> -    assert(soundhw_count < ARRAY_SIZE(soundhw) - 1);
> -    soundhw[soundhw_count].name = name;
> -    soundhw[soundhw_count].descr = descr;
> -    soundhw[soundhw_count].isa = 1;
> -    soundhw[soundhw_count].init.init_isa = init_isa;
> -    soundhw_count++;
> -}
> -
> -void pci_register_soundhw(const char *name, const char *descr,
> -                          int (*init_pci)(PCIBus *bus))
> -{
> -    assert(soundhw_count < ARRAY_SIZE(soundhw) - 1);
> -    soundhw[soundhw_count].name = name;
> -    soundhw[soundhw_count].descr = descr;
> -    soundhw[soundhw_count].isa = 0;
> -    soundhw[soundhw_count].init.init_pci = init_pci;
> -    soundhw_count++;
> -}
> -
> -void deprecated_register_soundhw(const char *name, const char *descr,
> -                                 int isa, const char *typename)
> -{
> -    assert(soundhw_count < ARRAY_SIZE(soundhw) - 1);
> -    soundhw[soundhw_count].name = name;
> -    soundhw[soundhw_count].descr = descr;
> -    soundhw[soundhw_count].isa = isa;
> -    soundhw[soundhw_count].typename = typename;
> -    soundhw_count++;
> -}
> -
> -void select_soundhw(const char *optarg)
> -{
> -    struct soundhw *c;
> -
> -    if (is_help_option(optarg)) {
> -    show_valid_cards:
> -
> -        if (soundhw_count) {
> -             printf("Valid sound card names (comma separated):\n");
> -             for (c = soundhw; c->name; ++c) {
> -                 printf ("%-11s %s\n", c->name, c->descr);
> -             }
> -             printf("\n-soundhw all will enable all of the above\n");
> -        } else {
> -             printf("Machine has no user-selectable audio hardware "
> -                    "(it may or may not have always-present audio hardware).\n");
> -        }
> -        exit(!is_help_option(optarg));
> -    }
> -    else {
> -        size_t l;
> -        const char *p;
> -        char *e;
> -        int bad_card = 0;
> -
> -        if (!strcmp(optarg, "all")) {
> -            for (c = soundhw; c->name; ++c) {
> -                c->enabled = 1;
> -            }
> -            return;
> -        }
> -
> -        p = optarg;
> -        while (*p) {
> -            e = strchr(p, ',');
> -            l = !e ? strlen(p) : (size_t) (e - p);
> -
> -            for (c = soundhw; c->name; ++c) {
> -                if (!strncmp(c->name, p, l) && !c->name[l]) {
> -                    c->enabled = 1;
> -                    break;
> -                }
> -            }
> -
> -            if (!c->name) {
> -                if (l > 80) {
> -                    error_report("Unknown sound card name (too big to show)");
> -                }
> -                else {
> -                    error_report("Unknown sound card name `%.*s'",
> -                                 (int) l, p);
> -                }
> -                bad_card = 1;
> -            }
> -            p += l + (e != NULL);
> -        }
> -
> -        if (bad_card) {
> -            goto show_valid_cards;
> -        }
> -    }
> -}
> -
> -void soundhw_init(void)
> -{
> -    struct soundhw *c;
> -    ISABus *isa_bus = (ISABus *) object_resolve_path_type("", TYPE_ISA_BUS, NULL);
> -    PCIBus *pci_bus = (PCIBus *) object_resolve_path_type("", TYPE_PCI_BUS, NULL);
> -
> -    for (c = soundhw; c->name; ++c) {
> -        if (c->enabled) {
> -            if (c->typename) {
> -                warn_report("'-soundhw %s' is deprecated, "
> -                            "please use '-device %s' instead",
> -                            c->name, c->typename);
> -                if (c->isa) {
> -                    isa_create_simple(isa_bus, c->typename);
> -                } else {
> -                    pci_create_simple(pci_bus, -1, c->typename);
> -                }
> -            } else if (c->isa) {
> -                if (!isa_bus) {
> -                    error_report("ISA bus not available for %s", c->name);
> -                    exit(1);
> -                }
> -                c->init.init_isa(isa_bus);
> -            } else {
> -                if (!pci_bus) {
> -                    error_report("PCI bus not available for %s", c->name);
> -                    exit(1);
> -                }
> -                c->init.init_pci(pci_bus);
> -            }
> -        }
> -    }
> -}
> -
> diff --git a/include/hw/audio/soundhw.h b/include/hw/audio/soundhw.h
> deleted file mode 100644
> index f09a297854af..000000000000
> --- a/include/hw/audio/soundhw.h
> +++ /dev/null
> @@ -1,15 +0,0 @@
> -#ifndef HW_SOUNDHW_H
> -#define HW_SOUNDHW_H
> -
> -void isa_register_soundhw(const char *name, const char *descr,
> -                          int (*init_isa)(ISABus *bus));
> -
> -void pci_register_soundhw(const char *name, const char *descr,
> -                          int (*init_pci)(PCIBus *bus));
> -void deprecated_register_soundhw(const char *name, const char *descr,
> -                                 int isa, const char *typename);
> -
> -void soundhw_init(void);
> -void select_soundhw(const char *optarg);
> -
> -#endif
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 34e9b32a5c00..47bbd86a4533 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -892,33 +892,6 @@ SRST
>           ``qemu.wav``.
>   ERST
>   
> -DEF("soundhw", HAS_ARG, QEMU_OPTION_soundhw,
> -    "-soundhw c1,... enable audio support\n"
> -    "                and only specified sound cards (comma separated list)\n"
> -    "                use '-soundhw help' to get the list of supported cards\n"
> -    "                use '-soundhw all' to enable all of them\n", QEMU_ARCH_ALL)
> -SRST
> -``-soundhw card1[,card2,...] or -soundhw all``
> -    Enable audio and selected sound hardware. Use 'help' to print all
> -    available sound hardware. For example:
> -
> -    .. parsed-literal::
> -
> -        |qemu_system_x86| -soundhw sb16,adlib disk.img
> -        |qemu_system_x86| -soundhw es1370 disk.img
> -        |qemu_system_x86| -soundhw ac97 disk.img
> -        |qemu_system_x86| -soundhw hda disk.img
> -        |qemu_system_x86| -soundhw all disk.img
> -        |qemu_system_x86| -soundhw help
> -
> -    Note that Linux's i810\_audio OSS kernel (for AC97) module might
> -    require manually specifying clocking.
> -
> -    ::
> -
> -        modprobe i810_audio clocking=48000
> -ERST
> -
>   DEF("device", HAS_ARG, QEMU_OPTION_device,
>       "-device driver[,prop[=value][,...]]\n"
>       "                add device (based on driver)\n"
> diff --git a/scripts/codeconverter/codeconverter/test_regexps.py b/scripts/codeconverter/codeconverter/test_regexps.py
> index a445634d88ac..0161cadffee6 100644
> --- a/scripts/codeconverter/codeconverter/test_regexps.py
> +++ b/scripts/codeconverter/codeconverter/test_regexps.py
> @@ -264,7 +264,6 @@ def test_initial_includes():
>   #define SILENT_ES1370
>   
>   #include "qemu/osdep.h"
> -#include "hw/audio/soundhw.h"
>   #include "audio/audio.h"
>   #include "hw/pci/pci.h"
>   #include "migration/vmstate.h"
> diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
> index 12fe60c4670d..5c80d135ebf0 100644
> --- a/softmmu/qdev-monitor.c
> +++ b/softmmu/qdev-monitor.c
> @@ -66,9 +66,7 @@ typedef struct QDevAlias
>   
>   /* Please keep this table sorted by typename. */
>   static const QDevAlias qdev_alias_table[] = {
> -    { "AC97", "ac97" }, /* -soundhw name */
>       { "e1000", "e1000-82540em" },
> -    { "ES1370", "es1370" }, /* -soundhw name */
>       { "ich9-ahci", "ahci" },
>       { "lsi53c895a", "lsi" },
>       { "virtio-9p-device", "virtio-9p", QEMU_ARCH_VIRTIO_MMIO },
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index 06a0e342fe9e..2f7158574e4f 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -80,7 +80,6 @@
>   #include "migration/snapshot.h"
>   #include "sysemu/tpm.h"
>   #include "sysemu/dma.h"
> -#include "hw/audio/soundhw.h"
>   #include "audio/audio.h"
>   #include "sysemu/cpus.h"
>   #include "sysemu/cpu-timers.h"
> @@ -2669,8 +2668,6 @@ static void qemu_create_cli_devices(void)
>   {
>       DeviceOption *opt;
>   
> -    soundhw_init();
> -
>       qemu_opts_foreach(qemu_find_opts("fw_cfg"),
>                         parse_fw_cfg, fw_cfg_find(), &error_fatal);
>   
> @@ -3018,9 +3015,6 @@ void qemu_init(int argc, char **argv, char **envp)
>               case QEMU_OPTION_audiodev:
>                   audio_parse_option(optarg);
>                   break;
> -            case QEMU_OPTION_soundhw:
> -                select_soundhw (optarg);
> -                break;
>               case QEMU_OPTION_h:
>                   help(0);
>                   break;

Re: [PATCH 05/18] tests/qtest: Specify audiodev= and -audiodev

[Martin Kletzander]

[-- Attachment #1: Type: text/plain, Size: 5348 bytes --]

On Mon, Apr 25, 2022 at 02:42:53PM +0100, Daniel P. Berrangé wrote:
>On Mon, Apr 25, 2022 at 10:21:48AM +0200, Martin Kletzander wrote:
>> This will enable removing deprecated default audiodev support.
>>
>> I did not figure out how to make the audiodev represented as an
>> interface node, so this is a workaround.  I am not sure what would be
>> the proper way.
>
>Not sure I understand what you mean by this 'interface node' reference ?
>

I meant a qos node, I though that would be the proper way, but since
audiodev is a backend and not a device I wasn't able to plug it in using
qos_node_consumes().  Maybe I was just trying too hard.

>The code looks fine though
>
>Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>
>>
>> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
>> ---
>>  tests/qtest/ac97-test.c                 |  3 ++-
>>  tests/qtest/es1370-test.c               |  3 ++-
>>  tests/qtest/fuzz/generic_fuzz_configs.h |  6 ++++--
>>  tests/qtest/intel-hda-test.c            | 15 ++++++++++-----
>>  4 files changed, 18 insertions(+), 9 deletions(-)
>>
>> diff --git a/tests/qtest/ac97-test.c b/tests/qtest/ac97-test.c
>> index e09f2495d24d..9711f1f6d966 100644
>> --- a/tests/qtest/ac97-test.c
>> +++ b/tests/qtest/ac97-test.c
>> @@ -45,7 +45,8 @@ static void *ac97_create(void *pci_bus, QGuestAllocator *alloc, void *addr)
>>  static void ac97_register_nodes(void)
>>  {
>>      QOSGraphEdgeOptions opts = {
>> -        .extra_device_opts = "addr=04.0",
>> +        .extra_device_opts = "addr=04.0,audiodev=audio0",
>> +        .before_cmd_line = "-audiodev driver=none,id=audio0",
>>      };
>>      add_qpci_address(&opts, &(QPCIAddress) { .devfn = QPCI_DEVFN(4, 0) });
>>
>> diff --git a/tests/qtest/es1370-test.c b/tests/qtest/es1370-test.c
>> index 2fd7fd2d3d30..5facda8d0d8d 100644
>> --- a/tests/qtest/es1370-test.c
>> +++ b/tests/qtest/es1370-test.c
>> @@ -46,7 +46,8 @@ static void *es1370_create(void *pci_bus, QGuestAllocator *alloc, void *addr)
>>  static void es1370_register_nodes(void)
>>  {
>>      QOSGraphEdgeOptions opts = {
>> -        .extra_device_opts = "addr=04.0",
>> +        .extra_device_opts = "addr=04.0,audiodev=audio0",
>> +        .before_cmd_line = "-audiodev driver=none,id=audio0",
>>      };
>>      add_qpci_address(&opts, &(QPCIAddress) { .devfn = QPCI_DEVFN(4, 0) });
>>
>> diff --git a/tests/qtest/fuzz/generic_fuzz_configs.h b/tests/qtest/fuzz/generic_fuzz_configs.h
>> index 004c701915e1..84a93b3c350b 100644
>> --- a/tests/qtest/fuzz/generic_fuzz_configs.h
>> +++ b/tests/qtest/fuzz/generic_fuzz_configs.h
>> @@ -101,8 +101,10 @@ const generic_fuzz_config predefined_configs[] = {
>>      },{
>>          .name = "intel-hda",
>>          .args = "-machine q35 -nodefaults -device intel-hda,id=hda0 "
>> -        "-device hda-output,bus=hda0.0 -device hda-micro,bus=hda0.0 "
>> -        "-device hda-duplex,bus=hda0.0",
>> +        "-audiodev driver=none,id=audio0",
>> +        "-device hda-output,bus=hda0.0,audiodev=audio0 "
>> +        "-device hda-micro,bus=hda0.0,audiodev=audio0 "
>> +        "-device hda-duplex,bus=hda0.0,audiodev=audio0",
>>          .objects = "intel-hda",
>>      },{
>>          .name = "ide-hd",
>> diff --git a/tests/qtest/intel-hda-test.c b/tests/qtest/intel-hda-test.c
>> index a58c98e4d11b..39ced2bc6ac6 100644
>> --- a/tests/qtest/intel-hda-test.c
>> +++ b/tests/qtest/intel-hda-test.c
>> @@ -11,20 +11,24 @@
>>  #include "libqtest-single.h"
>>
>>  #define HDA_ID "hda0"
>> -#define CODEC_DEVICES " -device hda-output,bus=" HDA_ID ".0" \
>> -                      " -device hda-micro,bus=" HDA_ID ".0" \
>> -                      " -device hda-duplex,bus=" HDA_ID ".0"
>> +#define AUDIODEV " -audiodev driver=none,id=audio0 "
>> +#define AUDIODEV_REF "audiodev=audio0"
>> +#define CODEC_DEVICES " -device hda-output,bus=" HDA_ID ".0," AUDIODEV_REF \
>> +                      " -device hda-micro,bus=" HDA_ID ".0," AUDIODEV_REF \
>> +                      " -device hda-duplex,bus=" HDA_ID ".0," AUDIODEV_REF
>>
>>  /* Tests only initialization so far. TODO: Replace with functional tests */
>>  static void ich6_test(void)
>>  {
>> -    qtest_start("-device intel-hda,id=" HDA_ID CODEC_DEVICES);
>> +    qtest_start(AUDIODEV "-device intel-hda,id=" HDA_ID CODEC_DEVICES);
>>      qtest_end();
>>  }
>>
>>  static void ich9_test(void)
>>  {
>> -    qtest_start("-machine q35 -device ich9-intel-hda,bus=pcie.0,addr=1b.0,id="
>> +    qtest_start("-machine q35"
>> +                AUDIODEV
>> +                "-device ich9-intel-hda,bus=pcie.0,addr=1b.0,id="
>>                  HDA_ID CODEC_DEVICES);
>>      qtest_end();
>>  }
>> @@ -39,6 +43,7 @@ static void test_issue542_ich6(void)
>>      QTestState *s;
>>
>>      s = qtest_init("-nographic -nodefaults -M pc-q35-6.2 "
>> +                   AUDIODEV
>>                     "-device intel-hda,id=" HDA_ID CODEC_DEVICES);
>>
>>      qtest_outl(s, 0xcf8, 0x80000804);
>> --
>> 2.35.1
>>
>
>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 :|
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 07/18] Introduce machine's default-audiodev property

[Martin Kletzander]

[-- Attachment #1: Type: text/plain, Size: 5102 bytes --]

On Mon, Apr 25, 2022 at 03:06:14PM +0100, Daniel P. Berrangé wrote:
>On Mon, Apr 25, 2022 at 10:21:50AM +0200, Martin Kletzander wrote:
>> Many machine types have default audio devices with no way to set the underlying
>> audiodev.  Instead of adding an option for each and every one of them this new
>> property can be used as a default during machine initialisation when creating
>> such devices.
>>
>> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
>> ---
>>  hw/core/machine.c   | 23 +++++++++++++++++++++++
>>  include/hw/boards.h |  1 +
>>  2 files changed, 24 insertions(+)
>>
>> diff --git a/hw/core/machine.c b/hw/core/machine.c
>> index cb9bbc844d24..d055a126d398 100644
>> --- a/hw/core/machine.c
>> +++ b/hw/core/machine.c
>> @@ -596,6 +596,22 @@ static void machine_set_memdev(Object *obj, const char *value, Error **errp)
>>      ms->ram_memdev_id = g_strdup(value);
>>  }
>>
>> +static char *machine_get_default_audiodev(Object *obj, Error **errp)
>> +{
>> +    MachineState *ms = MACHINE(obj);
>> +
>> +    return g_strdup(ms->default_audiodev);
>> +}
>> +
>> +static void machine_set_default_audiodev(Object *obj, const char *value,
>> +                                         Error **errp)
>> +{
>> +    MachineState *ms = MACHINE(obj);
>> +
>> +    g_free(ms->default_audiodev);
>> +    ms->default_audiodev = g_strdup(value);
>> +}
>> +
>>  HotpluggableCPUList *machine_query_hotpluggable_cpus(MachineState *machine)
>>  {
>>      int i;
>> @@ -867,6 +883,12 @@ static void machine_class_init(ObjectClass *oc, void *data)
>>      object_class_property_set_description(oc, "confidential-guest-support",
>>                                            "Set confidential guest scheme to support");
>>
>> +    object_class_property_add_str(oc, "default-audiodev",
>> +                                  machine_get_default_audiodev,
>> +                                  machine_set_default_audiodev);
>> +    object_class_property_set_description(oc, "default-audiodev",
>> +                                          "Audiodev to use for default machine devices");
>> +
>
>Hmm, if we add this, people might reasonably question why this default
>audiodev can't be used for everything, instead of making other 'audiodev'
>parameter mandatory.
>
>For the x86 machines we have a property tied specifically to the PC
>speaker.
>
>  -machine ...,pcspk-audiodev=<name>
>
>
>If we assume that's the desired pattern, then every machine which has
>a built-in audio device should gain some  "{$device}-audiodev" proprerty
>where '$device' is some reasonable name for the built-in audio device
>of the machine.  This would work better if a machine ended up with
>two built-in audio devices and needed separate audiodevs for them.
>

Which is what I wanted to avoid because it creates huge amount of names
which are not easy to find out.  I imagine that the default audio
devices which already exist are not much known and mostly used as-is
without the users needing to figure out what they are.  Especially those
that are enabled even with -nodefaults.  And I expect people who just
want to just emulate an old Palm do not really want to specify the names
of both devices which need an audiodev, especially when one of them is a
touch display or something weird like that.  Moreover you cannot specify
anything for these devices currently, so you cannot use two different
backends now.

I understand that the naming suggests it would be a default audiodev for
anything without one explicitly specified, but I could not think of any
other suitable name.  And I felt like having a default that would be
used for all devices defeats the purpose of removing default audiodev.

If adding a machine option for every such device is the way to go, then
I can try changing that.  Hopefully that would not turn into another
18-patch series =)  So should I prefer that route?

>
>>      /* For compatibility */
>>      object_class_property_add_str(oc, "memory-encryption",
>>          machine_get_memory_encryption, machine_set_memory_encryption);
>> @@ -961,6 +983,7 @@ static void machine_finalize(Object *obj)
>>      g_free(ms->device_memory);
>>      g_free(ms->nvdimms_state);
>>      g_free(ms->numa_state);
>> +    g_free(ms->default_audiodev);
>>  }
>>
>>  bool machine_usb(MachineState *machine)
>> diff --git a/include/hw/boards.h b/include/hw/boards.h
>> index d64b5481e834..5be1de50af03 100644
>> --- a/include/hw/boards.h
>> +++ b/include/hw/boards.h
>> @@ -346,6 +346,7 @@ struct MachineState {
>>       */
>>      MemoryRegion *ram;
>>      DeviceMemoryState *device_memory;
>> +    char *default_audiodev;
>>
>>      ram_addr_t ram_size;
>>      ram_addr_t maxram_size;
>> --
>> 2.35.1
>>
>
>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 :|
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 00/18] RFC: Remove deprecated audio features

[Martin Kletzander]

[-- Attachment #1: Type: text/plain, Size: 3333 bytes --]

On Mon, Apr 25, 2022 at 06:05:56PM +0100, Mark Cave-Ayland wrote:
>On 25/04/2022 09:21, Martin Kletzander wrote:
>
>> I wanted to deal with https://bugzilla.redhat.com/2043498 and I got a
>> suggesstion that removing deprecated features could actually make it
>> easier to propagate the error.  In the end (last patch) it turns out the
>> error is still just reported with error_fatal, so it probably is not
>> really needed, but I really wanted to dig into QEMU more and learn some
>> of the internals for quite some time now.  So I used the opportunity.
>> The one-liner ended up being an 18 patch series which was, for someone
>> who has just one commit in QEMU codebase, a pretty challenging task.
>> Although I tried my best to do things properly, I am not sure whether I
>> handled everything correctly, hence the RFC.
>>
>> Any comments are very much appreciated.  Thanks and have a nice day ;)
>>
>> Martin Kletzander (18):
>>    hw/audio: Remove -soundhw support
>>    hw/input/tsc210x: Extract common init code into new function
>>    hw/audio: Simplify hda audio init
>>    hw/audio/lm4549: Add errp error reporting to init function
>>    tests/qtest: Specify audiodev= and -audiodev
>>    ui/vnc: Require audiodev=
>>    Introduce machine's default-audiodev property
>>    audio: Add easy dummy audio initialiser
>>    hw/display/xlnx_dp.c: Add audiodev property
>>    hw/input/tsc210x.c: Support machine-default audiodev with fallback
>>    hw/arm: Support machine-default audiodev with fallback
>>    hw/ppc: Support machine-default audiodev with fallback
>>    audio: Make AUD_register_card fallible and require audiodev=
>>    audio: Require AudioState in AUD_add_capture
>>    audio: Be more strict during audio backend initialisation
>>    audio: Remove legacy audio environment variables and options
>>    audio: Remove unused can_be_default
>>    audio/spiceaudio: Fail initialisation when not using spice
>>

[...]

>
>Thanks for the tidy-up! I'm not too familiar with the audio code, however one thing I
>noticed is that in patch 11 you are using qdev_prop_set_string() to set the audiodev
>for the machines.
>
>For CharDevs there already exists a qdev_prop_set_chr() function which is used to
>assign the chardev backend to the device, so I'm wondering if it makes sense to add a
>similar qdev_prop_set_audiodev() function in the same way? Then if the method of
>referencing the audiodev from the device ever changes from being a string containing
>the name, it won't require updating all of the callers.
>

I guess that could be possible, but I'm not familiar with QEMU code much
and to be honest have no idea how I would go about doing that right now.

>Finally a quick glance at the tsc210x and lm4549 devices indicates that they are
>using global device _init() functions which shouldn't really be used in modern
>implementations. If you are interested in QOMify-ing these devices as part of this
>work, then I can certainly help provide some pointers for getting started.
>

I thought about it for some devices.  Not all the devices I touched
here, because I feel like that would lead to a rabbit hole.  And 18
patches just because I wanted to error out in one condition was already
too much =)  But I'll be glad for any pointers on where to start and how
should I get to it.  Thanks.

>
>ATB,
>
>Mark.
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 00/18] RFC: Remove deprecated audio features

[Paolo Bonzini]

On 4/25/22 10:21, Martin Kletzander wrote:
> I wanted to deal with https://bugzilla.redhat.com/2043498 and I got a
> suggesstion that removing deprecated features could actually make it
> easier to propagate the error.  In the end (last patch) it turns out the
> error is still just reported with error_fatal, so it probably is not
> really needed, but I really wanted to dig into QEMU more and learn some
> of the internals for quite some time now.  So I used the opportunity.
> The one-liner ended up being an 18 patch series which was, for someone
> who has just one commit in QEMU codebase, a pretty challenging task.
> Although I tried my best to do things properly, I am not sure whether I
> handled everything correctly, hence the RFC.

Rebased and queued what's left...  It took a while. :)

Paolo

> Martin Kletzander (18):
>    hw/audio: Remove -soundhw support
>    hw/input/tsc210x: Extract common init code into new function
>    hw/audio: Simplify hda audio init
>    hw/audio/lm4549: Add errp error reporting to init function
>    tests/qtest: Specify audiodev= and -audiodev
>    ui/vnc: Require audiodev=
>    Introduce machine's default-audiodev property
>    audio: Add easy dummy audio initialiser
>    hw/display/xlnx_dp.c: Add audiodev property
>    hw/input/tsc210x.c: Support machine-default audiodev with fallback
>    hw/arm: Support machine-default audiodev with fallback
>    hw/ppc: Support machine-default audiodev with fallback
>    audio: Make AUD_register_card fallible and require audiodev=
>    audio: Require AudioState in AUD_add_capture
>    audio: Be more strict during audio backend initialisation
>    audio: Remove legacy audio environment variables and options
>    audio: Remove unused can_be_default
>    audio/spiceaudio: Fail initialisation when not using spice
> 
>   audio/alsaaudio.c                             |   1 -
>   audio/audio.c                                 | 204 +++----
>   audio/audio.h                                 |   5 +-
>   audio/audio_int.h                             |   1 -
>   audio/audio_legacy.c                          | 555 ------------------
>   audio/coreaudio.m                             |   1 -
>   audio/dbusaudio.c                             |   1 -
>   audio/dsoundaudio.c                           |   1 -
>   audio/jackaudio.c                             |   1 -
>   audio/meson.build                             |   1 -
>   audio/noaudio.c                               |   1 -
>   audio/ossaudio.c                              |   1 -
>   audio/paaudio.c                               |   1 -
>   audio/sdlaudio.c                              |   1 -
>   audio/spiceaudio.c                            |   3 +-
>   audio/wavaudio.c                              |   1 -
>   docs/about/deprecated.rst                     |  24 -
>   docs/about/removed-features.rst               |  27 +
>   docs/qdev-device-use.txt                      |  21 +-
>   docs/replay.txt                               |   2 +-
>   hw/arm/integratorcp.c                         |   8 +-
>   hw/arm/musicpal.c                             |   8 +-
>   hw/arm/omap2.c                                |  11 +-
>   hw/arm/realview.c                             |   3 +
>   hw/arm/spitz.c                                |  10 +-
>   hw/arm/versatilepb.c                          |   3 +
>   hw/arm/vexpress.c                             |   3 +
>   hw/arm/xlnx-zcu102.c                          |   4 +
>   hw/arm/z2.c                                   |  12 +-
>   hw/audio/ac97.c                               |   9 +-
>   hw/audio/adlib.c                              |   9 +-
>   hw/audio/cs4231a.c                            |   8 +-
>   hw/audio/es1370.c                             |   8 +-
>   hw/audio/gus.c                                |   6 +-
>   hw/audio/hda-codec.c                          |  37 +-
>   hw/audio/intel-hda.c                          |  25 +-
>   hw/audio/intel-hda.h                          |   2 +-
>   hw/audio/lm4549.c                             |   7 +-
>   hw/audio/lm4549.h                             |   3 +-
>   hw/audio/meson.build                          |   1 -
>   hw/audio/pcspk.c                              |  15 +-
>   hw/audio/pl041.c                              |   2 +-
>   hw/audio/sb16.c                               |   9 +-
>   hw/audio/soundhw.c                            | 177 ------
>   hw/audio/wm8750.c                             |   5 +-
>   hw/core/machine.c                             |  23 +
>   hw/display/xlnx_dp.c                          |  12 +-
>   hw/input/tsc210x.c                            |  79 ++-
>   hw/ppc/prep.c                                 |   4 +
>   hw/usb/dev-audio.c                            |   5 +-
>   include/hw/audio/soundhw.h                    |  15 -
>   include/hw/boards.h                           |   1 +
>   qemu-options.hx                               |  37 --
>   .../codeconverter/test_regexps.py             |   1 -
>   softmmu/qdev-monitor.c                        |   2 -
>   softmmu/vl.c                                  |  10 -
>   tests/qtest/ac97-test.c                       |   3 +-
>   tests/qtest/es1370-test.c                     |   3 +-
>   tests/qtest/fuzz/generic_fuzz_configs.h       |   6 +-
>   tests/qtest/intel-hda-test.c                  |  15 +-
>   ui/vnc.c                                      |  15 +-
>   61 files changed, 329 insertions(+), 1140 deletions(-)
>   delete mode 100644 audio/audio_legacy.c
>   delete mode 100644 hw/audio/soundhw.c
>   delete mode 100644 include/hw/audio/soundhw.h
>