Re: [PATCH 00/18] RFC: Remove deprecated audio features
On 4/25/22 10:21, Martin Kletzander wrote: I wanted to deal with https://bugzilla.redhat.com/2043498 and I got a suggesstion that removing deprecated features could actually make it easier to propagate the error. In the end (last patch) it turns out the error is still just reported with error_fatal, so it probably is not really needed, but I really wanted to dig into QEMU more and learn some of the internals for quite some time now. So I used the opportunity. The one-liner ended up being an 18 patch series which was, for someone who has just one commit in QEMU codebase, a pretty challenging task. Although I tried my best to do things properly, I am not sure whether I handled everything correctly, hence the RFC. Rebased and queued what's left... It took a while. :) Paolo Martin Kletzander (18): hw/audio: Remove -soundhw support hw/input/tsc210x: Extract common init code into new function hw/audio: Simplify hda audio init hw/audio/lm4549: Add errp error reporting to init function tests/qtest: Specify audiodev= and -audiodev ui/vnc: Require audiodev= Introduce machine's default-audiodev property audio: Add easy dummy audio initialiser hw/display/xlnx_dp.c: Add audiodev property hw/input/tsc210x.c: Support machine-default audiodev with fallback hw/arm: Support machine-default audiodev with fallback hw/ppc: Support machine-default audiodev with fallback audio: Make AUD_register_card fallible and require audiodev= audio: Require AudioState in AUD_add_capture audio: Be more strict during audio backend initialisation audio: Remove legacy audio environment variables and options audio: Remove unused can_be_default audio/spiceaudio: Fail initialisation when not using spice audio/alsaaudio.c | 1 - audio/audio.c | 204 +++ audio/audio.h | 5 +- audio/audio_int.h | 1 - audio/audio_legacy.c | 555 -- audio/coreaudio.m | 1 - audio/dbusaudio.c | 1 - audio/dsoundaudio.c | 1 - audio/jackaudio.c | 1 - audio/meson.build | 1 - audio/noaudio.c | 1 - audio/ossaudio.c | 1 - audio/paaudio.c | 1 - audio/sdlaudio.c | 1 - audio/spiceaudio.c| 3 +- audio/wavaudio.c | 1 - docs/about/deprecated.rst | 24 - docs/about/removed-features.rst | 27 + docs/qdev-device-use.txt | 21 +- docs/replay.txt | 2 +- hw/arm/integratorcp.c | 8 +- hw/arm/musicpal.c | 8 +- hw/arm/omap2.c| 11 +- hw/arm/realview.c | 3 + hw/arm/spitz.c| 10 +- hw/arm/versatilepb.c | 3 + hw/arm/vexpress.c | 3 + hw/arm/xlnx-zcu102.c | 4 + hw/arm/z2.c | 12 +- hw/audio/ac97.c | 9 +- hw/audio/adlib.c | 9 +- hw/audio/cs4231a.c| 8 +- hw/audio/es1370.c | 8 +- hw/audio/gus.c| 6 +- hw/audio/hda-codec.c | 37 +- hw/audio/intel-hda.c | 25 +- hw/audio/intel-hda.h | 2 +- hw/audio/lm4549.c | 7 +- hw/audio/lm4549.h | 3 +- hw/audio/meson.build | 1 - hw/audio/pcspk.c | 15 +- hw/audio/pl041.c | 2 +- hw/audio/sb16.c | 9 +- hw/audio/soundhw.c| 177 -- hw/audio/wm8750.c | 5 +- hw/core/machine.c | 23 + hw/display/xlnx_dp.c | 12 +- hw/input/tsc210x.c| 79 ++- hw/ppc/prep.c | 4 + hw/usb/dev-audio.c| 5 +- include/hw/audio/soundhw.h| 15 - include/hw/boards.h | 1 + qemu-options.hx | 37 -- .../codeconverter/test_regexps.py | 1 - softmmu/qdev-monitor.c| 2 - softmmu/vl.c | 10 - tests/qtest/ac97-test.c |
Re: [PATCH 00/18] RFC: Remove deprecated audio features
On Mon, Apr 25, 2022 at 06:05:56PM +0100, Mark Cave-Ayland wrote: On 25/04/2022 09:21, Martin Kletzander wrote: I wanted to deal with https://bugzilla.redhat.com/2043498 and I got a suggesstion that removing deprecated features could actually make it easier to propagate the error. In the end (last patch) it turns out the error is still just reported with error_fatal, so it probably is not really needed, but I really wanted to dig into QEMU more and learn some of the internals for quite some time now. So I used the opportunity. The one-liner ended up being an 18 patch series which was, for someone who has just one commit in QEMU codebase, a pretty challenging task. Although I tried my best to do things properly, I am not sure whether I handled everything correctly, hence the RFC. Any comments are very much appreciated. Thanks and have a nice day ;) Martin Kletzander (18): hw/audio: Remove -soundhw support hw/input/tsc210x: Extract common init code into new function hw/audio: Simplify hda audio init hw/audio/lm4549: Add errp error reporting to init function tests/qtest: Specify audiodev= and -audiodev ui/vnc: Require audiodev= Introduce machine's default-audiodev property audio: Add easy dummy audio initialiser hw/display/xlnx_dp.c: Add audiodev property hw/input/tsc210x.c: Support machine-default audiodev with fallback hw/arm: Support machine-default audiodev with fallback hw/ppc: Support machine-default audiodev with fallback audio: Make AUD_register_card fallible and require audiodev= audio: Require AudioState in AUD_add_capture audio: Be more strict during audio backend initialisation audio: Remove legacy audio environment variables and options audio: Remove unused can_be_default audio/spiceaudio: Fail initialisation when not using spice [...] Thanks for the tidy-up! I'm not too familiar with the audio code, however one thing I noticed is that in patch 11 you are using qdev_prop_set_string() to set the audiodev for the machines. For CharDevs there already exists a qdev_prop_set_chr() function which is used to assign the chardev backend to the device, so I'm wondering if it makes sense to add a similar qdev_prop_set_audiodev() function in the same way? Then if the method of referencing the audiodev from the device ever changes from being a string containing the name, it won't require updating all of the callers. I guess that could be possible, but I'm not familiar with QEMU code much and to be honest have no idea how I would go about doing that right now. Finally a quick glance at the tsc210x and lm4549 devices indicates that they are using global device _init() functions which shouldn't really be used in modern implementations. If you are interested in QOMify-ing these devices as part of this work, then I can certainly help provide some pointers for getting started. I thought about it for some devices. Not all the devices I touched here, because I feel like that would lead to a rabbit hole. And 18 patches just because I wanted to error out in one condition was already too much =) But I'll be glad for any pointers on where to start and how should I get to it. Thanks. ATB, Mark. signature.asc Description: PGP signature
Re: [PATCH 00/18] RFC: Remove deprecated audio features
On Mon, Apr 25, 2022 at 10:21:43AM +0200, Martin Kletzander wrote: > I wanted to deal with https://bugzilla.redhat.com/2043498 and I got a > suggesstion that removing deprecated features could actually make it > easier to propagate the error. In the end (last patch) it turns out the > error is still just reported with error_fatal, so it probably is not > really needed, but I really wanted to dig into QEMU more and learn some > of the internals for quite some time now. So I used the opportunity. > The one-liner ended up being an 18 patch series which was, for someone > who has just one commit in QEMU codebase, a pretty challenging task. > Although I tried my best to do things properly, I am not sure whether I > handled everything correctly, hence the RFC. > > Any comments are very much appreciated. Thanks and have a nice day ;) Hi Martin, The -soundhw removal looks good to me. I'm not familiar with the audio subsystem so I didn't review the rest. Stefan signature.asc Description: PGP signature
Re: [PATCH 00/18] RFC: Remove deprecated audio features
On 25/04/2022 09:21, Martin Kletzander wrote: I wanted to deal with https://bugzilla.redhat.com/2043498 and I got a suggesstion that removing deprecated features could actually make it easier to propagate the error. In the end (last patch) it turns out the error is still just reported with error_fatal, so it probably is not really needed, but I really wanted to dig into QEMU more and learn some of the internals for quite some time now. So I used the opportunity. The one-liner ended up being an 18 patch series which was, for someone who has just one commit in QEMU codebase, a pretty challenging task. Although I tried my best to do things properly, I am not sure whether I handled everything correctly, hence the RFC. Any comments are very much appreciated. Thanks and have a nice day ;) Martin Kletzander (18): hw/audio: Remove -soundhw support hw/input/tsc210x: Extract common init code into new function hw/audio: Simplify hda audio init hw/audio/lm4549: Add errp error reporting to init function tests/qtest: Specify audiodev= and -audiodev ui/vnc: Require audiodev= Introduce machine's default-audiodev property audio: Add easy dummy audio initialiser hw/display/xlnx_dp.c: Add audiodev property hw/input/tsc210x.c: Support machine-default audiodev with fallback hw/arm: Support machine-default audiodev with fallback hw/ppc: Support machine-default audiodev with fallback audio: Make AUD_register_card fallible and require audiodev= audio: Require AudioState in AUD_add_capture audio: Be more strict during audio backend initialisation audio: Remove legacy audio environment variables and options audio: Remove unused can_be_default audio/spiceaudio: Fail initialisation when not using spice audio/alsaaudio.c | 1 - audio/audio.c | 204 +++ audio/audio.h | 5 +- audio/audio_int.h | 1 - audio/audio_legacy.c | 555 -- audio/coreaudio.m | 1 - audio/dbusaudio.c | 1 - audio/dsoundaudio.c | 1 - audio/jackaudio.c | 1 - audio/meson.build | 1 - audio/noaudio.c | 1 - audio/ossaudio.c | 1 - audio/paaudio.c | 1 - audio/sdlaudio.c | 1 - audio/spiceaudio.c| 3 +- audio/wavaudio.c | 1 - docs/about/deprecated.rst | 24 - docs/about/removed-features.rst | 27 + docs/qdev-device-use.txt | 21 +- docs/replay.txt | 2 +- hw/arm/integratorcp.c | 8 +- hw/arm/musicpal.c | 8 +- hw/arm/omap2.c| 11 +- hw/arm/realview.c | 3 + hw/arm/spitz.c| 10 +- hw/arm/versatilepb.c | 3 + hw/arm/vexpress.c | 3 + hw/arm/xlnx-zcu102.c | 4 + hw/arm/z2.c | 12 +- hw/audio/ac97.c | 9 +- hw/audio/adlib.c | 9 +- hw/audio/cs4231a.c| 8 +- hw/audio/es1370.c | 8 +- hw/audio/gus.c| 6 +- hw/audio/hda-codec.c | 37 +- hw/audio/intel-hda.c | 25 +- hw/audio/intel-hda.h | 2 +- hw/audio/lm4549.c | 7 +- hw/audio/lm4549.h | 3 +- hw/audio/meson.build | 1 - hw/audio/pcspk.c | 15 +- hw/audio/pl041.c | 2 +- hw/audio/sb16.c | 9 +- hw/audio/soundhw.c| 177 -- hw/audio/wm8750.c | 5 +- hw/core/machine.c | 23 + hw/display/xlnx_dp.c | 12 +- hw/input/tsc210x.c| 79 ++- hw/ppc/prep.c | 4 + hw/usb/dev-audio.c| 5 +- include/hw/audio/soundhw.h| 15 - include/hw/boards.h | 1 + qemu-options.hx | 37 -- .../codeconverter/test_regexps.py | 1 - softmmu/qdev-monitor.c| 2 - softmmu/vl.c | 10 - tests/qtest/ac97-test.c
