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
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
[-- 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 --]
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 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 >
[-- 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 --]
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