[PATCH v2 00/13] audio: deprecate -soundhw
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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 :|
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 :|
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
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
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().
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
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
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
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
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
participants (5)
-
Daniel P. Berrangé -
Gerd Hoffmann -
Ján Tomko -
Markus Armbruster -
Paolo Bonzini