[PATCH v2 00/13] audio: deprecate -soundhw

[PATCH v2 00/13] audio: deprecate -soundhw

[Gerd Hoffmann]

v2:
 - use g_assert_not_reached() for stubs.
 - add deprecation notice.

Gerd Hoffmann (13):
  stubs: add isa_create_simple
  stubs: add pci_create_simple
  audio: add deprecated_register_soundhw
  audio: deprecate -soundhw ac97
  audio: deprecate -soundhw es1370
  audio: deprecate -soundhw adlib
  audio: deprecate -soundhw cs4231a
  audio: deprecate -soundhw gus
  audio: deprecate -soundhw sb16
  audio: deprecate -soundhw hda
  audio: deprecate -soundhw pcspk
  audio: add soundhw deprecation notice
  [RFC] audio: try use onboard audiodev for pcspk

 include/hw/audio/soundhw.h |  2 ++
 hw/audio/ac97.c            |  9 ++-------
 hw/audio/adlib.c           |  8 +-------
 hw/audio/cs4231a.c         |  8 +-------
 hw/audio/es1370.c          |  9 ++-------
 hw/audio/gus.c             |  8 +-------
 hw/audio/intel-hda.c       |  3 +++
 hw/audio/pcspk.c           | 27 ++++++++++++++++++++++++---
 hw/audio/sb16.c            |  9 ++-------
 hw/audio/soundhw.c         | 24 +++++++++++++++++++++++-
 qdev-monitor.c             |  2 ++
 stubs/isa-bus.c            |  7 +++++++
 stubs/pci-bus.c            |  7 +++++++
 docs/system/deprecated.rst |  9 +++++++++
 stubs/Makefile.objs        |  2 ++
 15 files changed, 88 insertions(+), 46 deletions(-)
 create mode 100644 stubs/isa-bus.c
 create mode 100644 stubs/pci-bus.c

-- 
2.18.4

[PATCH v2 01/13] stubs: add isa_create_simple

[Gerd Hoffmann]

Needed for -soundhw cleanup.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 stubs/isa-bus.c     | 7 +++++++
 stubs/Makefile.objs | 1 +
 2 files changed, 8 insertions(+)
 create mode 100644 stubs/isa-bus.c

diff --git a/stubs/isa-bus.c b/stubs/isa-bus.c
new file mode 100644
index 000000000000..522f448997d4
--- /dev/null
+++ b/stubs/isa-bus.c
@@ -0,0 +1,7 @@
+#include "qemu/osdep.h"
+#include "hw/isa/isa.h"
+
+ISADevice *isa_create_simple(ISABus *bus, const char *name)
+{
+    g_assert_not_reached();
+}
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 45be5dc0ed78..c61ff38cc801 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -13,6 +13,7 @@ stub-obj-y += get-vm-name.o
 stub-obj-y += iothread.o
 stub-obj-y += iothread-lock.o
 stub-obj-y += is-daemonized.o
+stub-obj-y += isa-bus.o
 stub-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
 stub-obj-$(CONFIG_LINUX_IO_URING) += io_uring.o
 stub-obj-y += machine-init-done.o
-- 
2.18.4

[PATCH v2 02/13] stubs: add pci_create_simple

[Gerd Hoffmann]

Needed for -soundhw cleanup.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 stubs/pci-bus.c     | 7 +++++++
 stubs/Makefile.objs | 1 +
 2 files changed, 8 insertions(+)
 create mode 100644 stubs/pci-bus.c

diff --git a/stubs/pci-bus.c b/stubs/pci-bus.c
new file mode 100644
index 000000000000..a8932fa93250
--- /dev/null
+++ b/stubs/pci-bus.c
@@ -0,0 +1,7 @@
+#include "qemu/osdep.h"
+#include "hw/pci/pci.h"
+
+PCIDevice *pci_create_simple(PCIBus *bus, int devfn, const char *name)
+{
+    g_assert_not_reached();
+}
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index c61ff38cc801..5e7f2b3f061e 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -40,6 +40,7 @@ stub-obj-y += target-get-monitor-def.o
 stub-obj-y += vmgenid.o
 stub-obj-y += xen-common.o
 stub-obj-y += xen-hvm.o
+stub-obj-y += pci-bus.o
 stub-obj-y += pci-host-piix.o
 stub-obj-y += ram-block.o
 stub-obj-y += ramfb.o
-- 
2.18.4

[PATCH v2 03/13] audio: add deprecated_register_soundhw

[Gerd Hoffmann]

Add helper function for -soundhw deprecation.  It can replace the
simple init functions which just call {isa,pci}_create_simple()
with a hardcoded type.  It also prints a deprecation message.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/hw/audio/soundhw.h |  2 ++
 hw/audio/soundhw.c         | 24 +++++++++++++++++++++++-
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/include/hw/audio/soundhw.h b/include/hw/audio/soundhw.h
index c8eef8241846..f09a297854af 100644
--- a/include/hw/audio/soundhw.h
+++ b/include/hw/audio/soundhw.h
@@ -6,6 +6,8 @@ void isa_register_soundhw(const char *name, const char *descr,
 
 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);
diff --git a/hw/audio/soundhw.c b/hw/audio/soundhw.c
index c750473c8f0c..173b674ff53a 100644
--- a/hw/audio/soundhw.c
+++ b/hw/audio/soundhw.c
@@ -22,6 +22,7 @@
  * THE SOFTWARE.
  */
 #include "qemu/osdep.h"
+#include "qemu/option.h"
 #include "qemu/help_option.h"
 #include "qemu/error-report.h"
 #include "qom/object.h"
@@ -32,6 +33,7 @@
 struct soundhw {
     const char *name;
     const char *descr;
+    const char *typename;
     int enabled;
     int isa;
     union {
@@ -65,6 +67,17 @@ void pci_register_soundhw(const char *name, const char *descr,
     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;
@@ -136,7 +149,16 @@ void soundhw_init(void)
 
     for (c = soundhw; c->name; ++c) {
         if (c->enabled) {
-            if (c->isa) {
+            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);
-- 
2.18.4

[PATCH v2 04/13] audio: deprecate -soundhw ac97

[Gerd Hoffmann]

Switch to deprecated_register_soundhw().  Remove the now obsolete init
function.  Add an alias so both ac97 and AC97 are working with -device.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/audio/ac97.c | 9 ++-------
 qdev-monitor.c  | 1 +
 2 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/hw/audio/ac97.c b/hw/audio/ac97.c
index 8a9b9924c495..38522cf0ba44 100644
--- a/hw/audio/ac97.c
+++ b/hw/audio/ac97.c
@@ -1393,12 +1393,6 @@ static void ac97_exit(PCIDevice *dev)
     AUD_remove_card(&s->card);
 }
 
-static int ac97_init (PCIBus *bus)
-{
-    pci_create_simple(bus, -1, TYPE_AC97);
-    return 0;
-}
-
 static Property ac97_properties[] = {
     DEFINE_AUDIO_PROPERTIES(AC97LinkState, card),
     DEFINE_PROP_END_OF_LIST (),
@@ -1436,7 +1430,8 @@ static const TypeInfo ac97_info = {
 static void ac97_register_types (void)
 {
     type_register_static (&ac97_info);
-    pci_register_soundhw("ac97", "Intel 82801AA AC97 Audio", ac97_init);
+    deprecated_register_soundhw("ac97", "Intel 82801AA AC97 Audio",
+                                0, TYPE_AC97);
 }
 
 type_init (ac97_register_types)
diff --git a/qdev-monitor.c b/qdev-monitor.c
index a4735d3bb190..9c3cb89473c6 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -53,6 +53,7 @@ typedef struct QDevAlias
 
 /* Please keep this table sorted by typename. */
 static const QDevAlias qdev_alias_table[] = {
+    { "AC97", "ac97" }, /* -soundhw name */
     { "e1000", "e1000-82540em" },
     { "ich9-ahci", "ahci" },
     { "lsi53c895a", "lsi" },
-- 
2.18.4

[PATCH v2 05/13] audio: deprecate -soundhw es1370

[Gerd Hoffmann]

Switch to deprecated_register_soundhw().  Remove the now obsolete init
function.  Add an alias so both es1370 and ES1370 are working with
-device.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/audio/es1370.c | 9 ++-------
 qdev-monitor.c    | 1 +
 2 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/hw/audio/es1370.c b/hw/audio/es1370.c
index 89c4dabcd44f..f36ed95ac93f 100644
--- a/hw/audio/es1370.c
+++ b/hw/audio/es1370.c
@@ -881,12 +881,6 @@ static void es1370_exit(PCIDevice *dev)
     AUD_remove_card(&s->card);
 }
 
-static int es1370_init (PCIBus *bus)
-{
-    pci_create_simple (bus, -1, TYPE_ES1370);
-    return 0;
-}
-
 static Property es1370_properties[] = {
     DEFINE_AUDIO_PROPERTIES(ES1370State, card),
     DEFINE_PROP_END_OF_LIST(),
@@ -925,7 +919,8 @@ static const TypeInfo es1370_info = {
 static void es1370_register_types (void)
 {
     type_register_static (&es1370_info);
-    pci_register_soundhw("es1370", "ENSONIQ AudioPCI ES1370", es1370_init);
+    deprecated_register_soundhw("es1370", "ENSONIQ AudioPCI ES1370",
+                                0, TYPE_ES1370);
 }
 
 type_init (es1370_register_types)
diff --git a/qdev-monitor.c b/qdev-monitor.c
index 9c3cb89473c6..748733215004 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -55,6 +55,7 @@ typedef struct QDevAlias
 static const QDevAlias qdev_alias_table[] = {
     { "AC97", "ac97" }, /* -soundhw name */
     { "e1000", "e1000-82540em" },
+    { "ES1370", "es1370" }, /* -soundhw name */
     { "ich9-ahci", "ahci" },
     { "lsi53c895a", "lsi" },
     { "virtio-9p-ccw", "virtio-9p", QEMU_ARCH_S390X },
-- 
2.18.4

[PATCH v2 06/13] audio: deprecate -soundhw adlib

[Gerd Hoffmann]

Switch to deprecated_register_soundhw().
Remove the now obsolete init function.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/audio/adlib.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/hw/audio/adlib.c b/hw/audio/adlib.c
index 7c3b67dcfb8c..65dff5b6fca4 100644
--- a/hw/audio/adlib.c
+++ b/hw/audio/adlib.c
@@ -319,16 +319,10 @@ static const TypeInfo adlib_info = {
     .class_init    = adlib_class_initfn,
 };
 
-static int Adlib_init (ISABus *bus)
-{
-    isa_create_simple (bus, TYPE_ADLIB);
-    return 0;
-}
-
 static void adlib_register_types (void)
 {
     type_register_static (&adlib_info);
-    isa_register_soundhw("adlib", ADLIB_DESC, Adlib_init);
+    deprecated_register_soundhw("adlib", ADLIB_DESC, 1, TYPE_ADLIB);
 }
 
 type_init (adlib_register_types)
-- 
2.18.4

[PATCH v2 07/13] audio: deprecate -soundhw cs4231a

[Gerd Hoffmann]

Switch to deprecated_register_soundhw().
Remove the now obsolete init function.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/audio/cs4231a.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/hw/audio/cs4231a.c b/hw/audio/cs4231a.c
index ffdbb58d6a11..59705a8d4701 100644
--- a/hw/audio/cs4231a.c
+++ b/hw/audio/cs4231a.c
@@ -683,12 +683,6 @@ static void cs4231a_realizefn (DeviceState *dev, Error **errp)
     AUD_register_card ("cs4231a", &s->card);
 }
 
-static int cs4231a_init (ISABus *bus)
-{
-    isa_create_simple (bus, TYPE_CS4231A);
-    return 0;
-}
-
 static Property cs4231a_properties[] = {
     DEFINE_AUDIO_PROPERTIES(CSState, card),
     DEFINE_PROP_UINT32 ("iobase",  CSState, port, 0x534),
@@ -720,7 +714,7 @@ static const TypeInfo cs4231a_info = {
 static void cs4231a_register_types (void)
 {
     type_register_static (&cs4231a_info);
-    isa_register_soundhw("cs4231a", "CS4231A", cs4231a_init);
+    deprecated_register_soundhw("cs4231a", "CS4231A", 1, TYPE_CS4231A);
 }
 
 type_init (cs4231a_register_types)
-- 
2.18.4

[PATCH v2 08/13] audio: deprecate -soundhw gus

[Gerd Hoffmann]

Switch to deprecated_register_soundhw().
Remove the now obsolete init function.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/audio/gus.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/hw/audio/gus.c b/hw/audio/gus.c
index eb4a803fb53b..61d16fad9ffb 100644
--- a/hw/audio/gus.c
+++ b/hw/audio/gus.c
@@ -292,12 +292,6 @@ static void gus_realizefn (DeviceState *dev, Error **errp)
     AUD_set_active_out (s->voice, 1);
 }
 
-static int GUS_init (ISABus *bus)
-{
-    isa_create_simple (bus, TYPE_GUS);
-    return 0;
-}
-
 static Property gus_properties[] = {
     DEFINE_AUDIO_PROPERTIES(GUSState, card),
     DEFINE_PROP_UINT32 ("freq",    GUSState, freq,        44100),
@@ -328,7 +322,7 @@ static const TypeInfo gus_info = {
 static void gus_register_types (void)
 {
     type_register_static (&gus_info);
-    isa_register_soundhw("gus", "Gravis Ultrasound GF1", GUS_init);
+    deprecated_register_soundhw("gus", "Gravis Ultrasound GF1", 1, TYPE_GUS);
 }
 
 type_init (gus_register_types)
-- 
2.18.4

[PATCH v2 09/13] audio: deprecate -soundhw sb16

[Gerd Hoffmann]

Switch to deprecated_register_soundhw().
Remove the now obsolete init function.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/audio/sb16.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/hw/audio/sb16.c b/hw/audio/sb16.c
index df6f755a37f8..2d9e50f99b5d 100644
--- a/hw/audio/sb16.c
+++ b/hw/audio/sb16.c
@@ -1415,12 +1415,6 @@ static void sb16_realizefn (DeviceState *dev, Error **errp)
     AUD_register_card ("sb16", &s->card);
 }
 
-static int SB16_init (ISABus *bus)
-{
-    isa_create_simple (bus, TYPE_SB16);
-    return 0;
-}
-
 static Property sb16_properties[] = {
     DEFINE_AUDIO_PROPERTIES(SB16State, card),
     DEFINE_PROP_UINT32 ("version", SB16State, ver,  0x0405), /* 4.5 */
@@ -1453,7 +1447,8 @@ static const TypeInfo sb16_info = {
 static void sb16_register_types (void)
 {
     type_register_static (&sb16_info);
-    isa_register_soundhw("sb16", "Creative Sound Blaster 16", SB16_init);
+    deprecated_register_soundhw("sb16", "Creative Sound Blaster 16",
+                                1, TYPE_SB16);
 }
 
 type_init (sb16_register_types)
-- 
2.18.4

[PATCH v2 10/13] audio: deprecate -soundhw hda

[Gerd Hoffmann]

Add deprecation message to the audio init function.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/audio/intel-hda.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
index 4696ae0d9a61..d491e407b317 100644
--- a/hw/audio/intel-hda.c
+++ b/hw/audio/intel-hda.c
@@ -25,6 +25,7 @@
 #include "qemu/bitops.h"
 #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"
@@ -1307,6 +1308,8 @@ static int intel_hda_and_codec_init(PCIBus *bus)
     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_create(hdabus, "hda-duplex");
-- 
2.18.4

[PATCH v2 11/13] audio: deprecate -soundhw pcspk

[Gerd Hoffmann]

Add deprecation message to the audio init function.

Factor out audio initialization and call that from
both audio init and realize, so setting audiodev via
-global is enough to properly initialize pcspk.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/audio/pcspk.c | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/hw/audio/pcspk.c b/hw/audio/pcspk.c
index c37a3878612e..ab290e686783 100644
--- a/hw/audio/pcspk.c
+++ b/hw/audio/pcspk.c
@@ -28,6 +28,7 @@
 #include "audio/audio.h"
 #include "qemu/module.h"
 #include "qemu/timer.h"
+#include "qemu/error-report.h"
 #include "hw/timer/i8254.h"
 #include "migration/vmstate.h"
 #include "hw/audio/pcspk.h"
@@ -112,11 +113,15 @@ static void pcspk_callback(void *opaque, int free)
     }
 }
 
-static int pcspk_audio_init(ISABus *bus)
+static int pcspk_audio_init(PCSpkState *s)
 {
-    PCSpkState *s = pcspk_state;
     struct audsettings as = {PCSPK_SAMPLE_RATE, 1, AUDIO_FORMAT_U8, 0};
 
+    if (s->voice) {
+        /* already initialized */
+        return 0;
+    }
+
     AUD_register_card(s_spk, &s->card);
 
     s->voice = AUD_open_out(&s->card, s->voice, s_spk, s, pcspk_callback, &as);
@@ -185,6 +190,10 @@ static void pcspk_realizefn(DeviceState *dev, Error **errp)
 
     isa_register_ioport(isadev, &s->ioport, s->iobase);
 
+    if (s->card.state) {
+        pcspk_audio_init(s);
+    }
+
     pcspk_state = s;
 }
 
@@ -236,9 +245,18 @@ 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 '-global isa-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);
+    isa_register_soundhw("pcspk", "PC speaker", pcspk_audio_init_soundhw);
 }
 type_init(pcspk_register)
-- 
2.18.4

Re: [PATCH v2 11/13] audio: deprecate -soundhw pcspk

[Ján Tomko]

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

On a Friday in 2020, Gerd Hoffmann wrote:
>Add deprecation message to the audio init function.
>
>Factor out audio initialization and call that from
>both audio init and realize, so setting audiodev via
>-global is enough to properly initialize pcspk.
>
>Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>---
> hw/audio/pcspk.c | 24 +++++++++++++++++++++---
> 1 file changed, 21 insertions(+), 3 deletions(-)
>
>@@ -236,9 +245,18 @@ 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 '-global isa-pcspk.audiodev=<name>' instead");
>+    return pcspk_audio_init(s);

-soundhw pcspk is the only soundhw device present in libvirt git.

Is there a way to probe for this change via QMP?

Jano

>+}
>+
> static void pcspk_register(void)
> {
>     type_register_static(&pcspk_info);
>-    isa_register_soundhw("pcspk", "PC speaker", pcspk_audio_init);
>+    isa_register_soundhw("pcspk", "PC speaker", pcspk_audio_init_soundhw);
> }
> type_init(pcspk_register)
>-- 
>2.18.4
>

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

Re: [PATCH v2 11/13] audio: deprecate -soundhw pcspk

[Gerd Hoffmann]

On Fri, May 15, 2020 at 05:08:23PM +0200, Ján Tomko wrote:
> On a Friday in 2020, Gerd Hoffmann wrote:
> > Add deprecation message to the audio init function.
> > 
> > Factor out audio initialization and call that from
> > both audio init and realize, so setting audiodev via
> > -global is enough to properly initialize pcspk.
> > 
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > ---
> > hw/audio/pcspk.c | 24 +++++++++++++++++++++---
> > 1 file changed, 21 insertions(+), 3 deletions(-)
> > 
> > @@ -236,9 +245,18 @@ 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 '-global isa-pcspk.audiodev=<name>' instead");
> > +    return pcspk_audio_init(s);
> 
> -soundhw pcspk is the only soundhw device present in libvirt git.
> 
> Is there a way to probe for this change via QMP?

Oops.  I'm surprised libvirt actually supports pcspk.

There is no way to see that in qmp, and I can't think of an easy way
to add that.  Does libvirt check for command line switches still?
So it could see -soundhw going away if that happens?

take care,
  Gerd

Re: [PATCH v2 11/13] audio: deprecate -soundhw pcspk

[Daniel P. Berrangé]

On Mon, May 18, 2020 at 12:16:28PM +0200, Gerd Hoffmann wrote:
> On Fri, May 15, 2020 at 05:08:23PM +0200, Ján Tomko wrote:
> > On a Friday in 2020, Gerd Hoffmann wrote:
> > > Add deprecation message to the audio init function.
> > > 
> > > Factor out audio initialization and call that from
> > > both audio init and realize, so setting audiodev via
> > > -global is enough to properly initialize pcspk.
> > > 
> > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > > ---
> > > hw/audio/pcspk.c | 24 +++++++++++++++++++++---
> > > 1 file changed, 21 insertions(+), 3 deletions(-)
> > > 
> > > @@ -236,9 +245,18 @@ 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 '-global isa-pcspk.audiodev=<name>' instead");
> > > +    return pcspk_audio_init(s);
> > 
> > -soundhw pcspk is the only soundhw device present in libvirt git.
> > 
> > Is there a way to probe for this change via QMP?
> 
> Oops.  I'm surprised libvirt actually supports pcspk.
> 
> There is no way to see that in qmp, and I can't think of an easy way
> to add that.  Does libvirt check for command line switches still?
> So it could see -soundhw going away if that happens?

IIUC, instead of probing for whether -soundhw is deprecated, it should
be suffiicent for us to probe if "isa-pcspk.audiodev" exists. Assuming
we always use isa-pcspk.audiodev if it exists, then we'll trivially
avoid using the -soundhw arg.

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 v2 11/13] audio: deprecate -soundhw pcspk

[Ján Tomko]

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

On a Monday in 2020, Daniel P. Berrangé wrote:
>On Mon, May 18, 2020 at 12:16:28PM +0200, Gerd Hoffmann wrote:
>> On Fri, May 15, 2020 at 05:08:23PM +0200, Ján Tomko wrote:
>> > On a Friday in 2020, Gerd Hoffmann wrote:
>> > > Add deprecation message to the audio init function.
>> > >
>> > > Factor out audio initialization and call that from
>> > > both audio init and realize, so setting audiodev via
>> > > -global is enough to properly initialize pcspk.
>> > >
>> > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>> > > ---
>> > > hw/audio/pcspk.c | 24 +++++++++++++++++++++---
>> > > 1 file changed, 21 insertions(+), 3 deletions(-)
>> > >
>> > > @@ -236,9 +245,18 @@ 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 '-global isa-pcspk.audiodev=<name>' instead");
>> > > +    return pcspk_audio_init(s);
>> >
>> > -soundhw pcspk is the only soundhw device present in libvirt git.
>> >
>> > Is there a way to probe for this change via QMP?
>>
>> Oops.  I'm surprised libvirt actually supports pcspk.
>>
>> There is no way to see that in qmp, and I can't think of an easy way
>> to add that.  Does libvirt check for command line switches still?
>> So it could see -soundhw going away if that happens?
>
>IIUC, instead of probing for whether -soundhw is deprecated, it should
>be suffiicent for us to probe if "isa-pcspk.audiodev" exists. Assuming
>we always use isa-pcspk.audiodev if it exists, then we'll trivially
>avoid using the -soundhw arg.
>

Yes, we can probe for that, but the phrasing in the commit message makes
it look like setting the property via -global will only be effective
after this commit.

Jano

>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: 488 bytes --]

Re: [PATCH v2 11/13] audio: deprecate -soundhw pcspk

[Gerd Hoffmann]

On Mon, May 18, 2020 at 11:26:50AM +0100, Daniel P. Berrangé wrote:
> On Mon, May 18, 2020 at 12:16:28PM +0200, Gerd Hoffmann wrote:
> > On Fri, May 15, 2020 at 05:08:23PM +0200, Ján Tomko wrote:
> > > On a Friday in 2020, Gerd Hoffmann wrote:
> > > > Add deprecation message to the audio init function.
> > > > 
> > > > Factor out audio initialization and call that from
> > > > both audio init and realize, so setting audiodev via
> > > > -global is enough to properly initialize pcspk.
> > > > 
> > > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > > > ---
> > > > hw/audio/pcspk.c | 24 +++++++++++++++++++++---
> > > > 1 file changed, 21 insertions(+), 3 deletions(-)
> > > > 
> > > > @@ -236,9 +245,18 @@ 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 '-global isa-pcspk.audiodev=<name>' instead");
> > > > +    return pcspk_audio_init(s);
> > > 
> > > -soundhw pcspk is the only soundhw device present in libvirt git.
> > > 
> > > Is there a way to probe for this change via QMP?
> > 
> > Oops.  I'm surprised libvirt actually supports pcspk.
> > 
> > There is no way to see that in qmp, and I can't think of an easy way
> > to add that.  Does libvirt check for command line switches still?
> > So it could see -soundhw going away if that happens?
> 
> IIUC, instead of probing for whether -soundhw is deprecated, it should
> be suffiicent for us to probe if "isa-pcspk.audiodev" exists. Assuming
> we always use isa-pcspk.audiodev if it exists, then we'll trivially
> avoid using the -soundhw arg.

It's not that easy unfortunately.  We have .audiodev for a few releases
already.  But just setting that isn't enough to initialize pcspk in
current qemu, "-soundhw pcspk" is still needed ...

I'm looking at how to initialize onboard audio devices currently, maybe
the best way to handle that is to do it flash-style with machine
properties (i.e have a pc.pcslk alias for pcspk.audiodev, simliar to
pc.flash0 being an alias for pflash.drive).  That'll be better that
-global and it'll also be visible in QOM.

Initialization order looks tricky though.  I'd have to create pcspk
early, simliar to flash, in pc_machine_initfn().  Problem is I don't
have a isa bus yet at that point (flash is sysbus and doesn't have this
problem).  I'm open to suggestions hiow do deal with that best.

take care,
  Gerd

Re: [PATCH v2 11/13] audio: deprecate -soundhw pcspk

[Gerd Hoffmann]

  Hi,

> Initialization order looks tricky though.  I'd have to create pcspk
> early, simliar to flash, in pc_machine_initfn().  Problem is I don't
> have a isa bus yet at that point (flash is sysbus and doesn't have this
> problem).  I'm open to suggestions hiow do deal with that best.

Seems I've found a way to deal with that: "ISADevice *pcspk =
object_new(TYPE_PC_SPEAKER);" can be done before the isa bus exists
& we can fixup things later using qdev_set_parent_bus().

cheers,
  Gerd

Re: [PATCH v2 11/13] audio: deprecate -soundhw pcspk

[Markus Armbruster]

Gerd Hoffmann <kraxel@redhat.com> writes:

>   Hi,
>
>> Initialization order looks tricky though.  I'd have to create pcspk
>> early, simliar to flash, in pc_machine_initfn().  Problem is I don't
>> have a isa bus yet at that point (flash is sysbus and doesn't have this
>> problem).  I'm open to suggestions hiow do deal with that best.
>
> Seems I've found a way to deal with that: "ISADevice *pcspk =
> object_new(TYPE_PC_SPEAKER);" can be done before the isa bus exists
> & we can fixup things later using qdev_set_parent_bus().

You'll want to watch out for the series I hope to post shortly: it'll be
dev = qdev_new(TYPE_PC_SPEAKER); qdev_realize(dev, bus, errp) then.  No
need for qdev_set_parent_bus().

Re: [PATCH v2 11/13] audio: deprecate -soundhw pcspk

[Gerd Hoffmann]

  Hi,

> >> Initialization order looks tricky though.  I'd have to create pcspk
> >> early, simliar to flash, in pc_machine_initfn().  Problem is I don't
> >> have a isa bus yet at that point (flash is sysbus and doesn't have this
> >> problem).  I'm open to suggestions hiow do deal with that best.
> >
> > Seems I've found a way to deal with that: "ISADevice *pcspk =
> > object_new(TYPE_PC_SPEAKER);" can be done before the isa bus exists
> > & we can fixup things later using qdev_set_parent_bus().
> 
> You'll want to watch out for the series I hope to post shortly: it'll be
> dev = qdev_new(TYPE_PC_SPEAKER); qdev_realize(dev, bus, errp) then.  No
> need for qdev_set_parent_bus().

Ah, cool, that shows that I'm on the right path with my idea ;)
Sneak preview:
	https://git.kraxel.org/cgit/qemu/log/?h=sirius/soundhw

Suggestions for a good name?  I've used "pc.pcspk" for now,
but maybe "pc.audiodev0" or "pc.sound0" is better?

take care,
  Gerd

Re: [PATCH v2 11/13] audio: deprecate -soundhw pcspk

[Paolo Bonzini]

On 18/05/20 15:27, Gerd Hoffmann wrote:
> Ah, cool, that shows that I'm on the right path with my idea ;)
> Sneak preview:
> 	https://git.kraxel.org/cgit/qemu/log/?h=sirius/soundhw
> 
> Suggestions for a good name?  I've used "pc.pcspk" for now,
> but maybe "pc.audiodev0" or "pc.sound0" is better?

Something like "-M pc,pcspk-audiodev=..."?

Paolo

[PATCH v2 12/13] audio: add soundhw deprecation notice

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 docs/system/deprecated.rst | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 3142fac38658..7de1045b7e27 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -88,6 +88,15 @@ should specify an ``audiodev=`` property.  Additionally, when using
 vnc, you should specify an ``audiodev=`` propery 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 ``-global
+pcspk.audiodev=<name>``.
+
 ``-mon ...,control=readline,pretty=on|off`` (since 4.1)
 '''''''''''''''''''''''''''''''''''''''''''''''''''''''
 
-- 
2.18.4

[PATCH v2 13/13] [RFC] audio: try use onboard audiodev for pcspk

[Gerd Hoffmann]

New naming convention:  Use "onboard" audiodev for onboard audio
devices, using "-audiodev pa,id=onboard" for example.

This patchs implements it for pcspk, it will try to use "onboard" by
default.  Setting another name using "-global pcspk.audiodev=<name>"
continues to work.

If we want go this route we should do the same for other onboard
audio devices too (arm boards, ...).

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/audio/pcspk.c           | 3 +++
 docs/system/deprecated.rst | 4 ++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/hw/audio/pcspk.c b/hw/audio/pcspk.c
index ab290e686783..9a08e9d8e05b 100644
--- a/hw/audio/pcspk.c
+++ b/hw/audio/pcspk.c
@@ -190,6 +190,9 @@ static void pcspk_realizefn(DeviceState *dev, Error **errp)
 
     isa_register_ioport(isadev, &s->ioport, s->iobase);
 
+    if (!s->card.state) {
+        s->card.state = audio_state_by_name("onboard");
+    }
     if (s->card.state) {
         pcspk_audio_init(s);
     }
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 7de1045b7e27..34312fc0a963 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -94,8 +94,8 @@ 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 ``-global
-pcspk.audiodev=<name>``.
+``pcspk`` which can be activated by creating an audiodev named
+``onboard``.
 
 ``-mon ...,control=readline,pretty=on|off`` (since 4.1)
 '''''''''''''''''''''''''''''''''''''''''''''''''''''''
-- 
2.18.4

Re: [PATCH v2 00/13] audio: deprecate -soundhw

[Paolo Bonzini]

On 15/05/20 16:35, Gerd Hoffmann wrote:
> v2:
>  - use g_assert_not_reached() for stubs.
>  - add deprecation notice.

If I understand it, the deprecation message suggests "-device ac97"
instead of "-soundhw ac97", but that in turn relies on the deprecated
default audiodev feature.

So I'm not sure deprecating -soundhw is a good idea.  Instead, is it
possible to make "-soundhw foo" desugar to "-audiodev
something,id=audio0 -global foo.audiodev=audio0 -device foo", where the
"-device foo" would be omitted for isa-pcspk?  It's all ad hoc, but
that's the point of combined frontend/backend options like -nic.

This doesn't change that libvirt can just stop using -soundhw just by
looking for the isa-pcspk.audiodev property.

Thanks,

Paolo

> Gerd Hoffmann (13):
>   stubs: add isa_create_simple
>   stubs: add pci_create_simple
>   audio: add deprecated_register_soundhw
>   audio: deprecate -soundhw ac97
>   audio: deprecate -soundhw es1370
>   audio: deprecate -soundhw adlib
>   audio: deprecate -soundhw cs4231a
>   audio: deprecate -soundhw gus
>   audio: deprecate -soundhw sb16
>   audio: deprecate -soundhw hda
>   audio: deprecate -soundhw pcspk
>   audio: add soundhw deprecation notice
>   [RFC] audio: try use onboard audiodev for pcspk
> 
>  include/hw/audio/soundhw.h |  2 ++
>  hw/audio/ac97.c            |  9 ++-------
>  hw/audio/adlib.c           |  8 +-------
>  hw/audio/cs4231a.c         |  8 +-------
>  hw/audio/es1370.c          |  9 ++-------
>  hw/audio/gus.c             |  8 +-------
>  hw/audio/intel-hda.c       |  3 +++
>  hw/audio/pcspk.c           | 27 ++++++++++++++++++++++++---
>  hw/audio/sb16.c            |  9 ++-------
>  hw/audio/soundhw.c         | 24 +++++++++++++++++++++++-
>  qdev-monitor.c             |  2 ++
>  stubs/isa-bus.c            |  7 +++++++
>  stubs/pci-bus.c            |  7 +++++++
>  docs/system/deprecated.rst |  9 +++++++++
>  stubs/Makefile.objs        |  2 ++
>  15 files changed, 88 insertions(+), 46 deletions(-)
>  create mode 100644 stubs/isa-bus.c
>  create mode 100644 stubs/pci-bus.c
>