Re: [PATCH v2] usb: Un-deprecate -usbdevice (except for -usbdevice audio which gets removed)
On 10/03/2021 14.17, Markus Armbruster wrote: Thomas Huth writes: When trying to remove the -usbdevice option, there were complaints that "-usbdevice braille" is still a very useful shortcut for some people. Pointer? I missed it. Thus we never remove this option. Since it's not such a big burden to keep it around, and it's also convenient in the sense that you don't have to worry to enable a host controller explicitly with this option, we should remove it from he deprecation list again, and rather properly document the possible device for this option instead. However, there is one exception: "-usbdevice audio" should go away, since audio devices without "audiodev=..." parameter are also on the deprecation list and you cannot use "-usbdevice audio" with "audiodev". Signed-off-by: Thomas Huth To be frank, I don't like this. At all. -usbdevice comes with its own ad hoc mini-language, parsed by usbdevice_create(). Syntax is DRIVER[:PARAMS], where PARAMS defaults to "" and is parsed by an optional DRIVER-specific LegacyUSBFactory. Oh, there is still parameter parsing code left? I thought we'd remove it after removing the other legacy devices that actually took parameters... so yes, at least the parameter-related code in usbdevice_create() should go away, too. We already dropped multiple drivers: "host", "serial", "disk", "net" (commit 99761176e, v2.12), and "bt" (commit 43d68d0a9, v5.0). Right, these were the really ugly ones that used their own parameter parsing code. We've kept "audio" (dropped in this patch), "tablet", "mouse", "keyboard", "braille", "ccid", and "wacom-tablet". Only "mouse", "tablet", "braille" are documented (fixed in this patch). One more has crept in: "u2f-key" (commit bb014a810, v5.2). It's buggy: $ qemu-system-x86_64 -S -usbdevice u2f-key qemu-system-x86_64: -usbdevice u2f-key: '-usbdevice' is deprecated, please use '-device usb-...' instead ** ERROR:../qom/object.c:508:object_initialize_with_type: assertion failed: (type->abstract == false) Bail out! ERROR:../qom/object.c:508:object_initialize_with_type: assertion failed: (type->abstract == false) Aborted (core dumped) Broken right in the commit that added the stuff. The sugar never worked, and should be taken out again. Ouch, that should get removed again immediately, of course. Without a factory, "-usbdevice BAR" is sugar for -device BAZ -machine usb=on "braille" is the only driver with a factory. "-usbdevice braille" is sugar for -device usb-braille,chardev=braille -chardev braille,id=braille -machine usb=on It's buggy: $ qemu-system-x86_64 -S -usbdevice braille qemu-system-x86_64: -usbdevice braille: '-usbdevice' is deprecated, please use '-device usb-...' instead [three seconds tick by...] Segmentation fault (core dumped) That's a separate issue. It neglects to actually parse PARAMS: $ qemu-system-x86_64 -S -usbdevice braille:"I'm a Little Teapot" qemu-system-x86_64: -usbdevice braille:I'm a Little Teapot: '-usbdevice' is deprecated, please use '-device usb-...' instead [three seconds tick by...] Segmentation fault (core dumped) The whole machinery in support of optional PARAMS has long become useless. Right, we should get rid of the remainders of parameter parsing here. I fail to see why we could drop the sugar for serial, disk, net and host devices, but not for the others. > The only one that has something approaching a leg to stand on is braille. Still, I fail to see why having to specify a backend is fine for any number of other devices, but not for braille. As explained in the mails from Samuel and Paolo, it's still used out there, so we should not break this without an easy replacement. Gerd, can you please un-queue my patch again. I'll rework it wrt. u2f-key and the legacy parameter parsing code. Thomas
Re: [PATCH v2] usb: Un-deprecate -usbdevice (except for -usbdevice audio which gets removed)
On Wed, Mar 10, 2021 at 2:17 PM Markus Armbruster wrote: > One more has crept in: "u2f-key" (commit bb014a810, v5.2). It's buggy: > > $ qemu-system-x86_64 -S -usbdevice u2f-key > qemu-system-x86_64: -usbdevice u2f-key: '-usbdevice' is deprecated, > please use '-device usb-...' instead > ** > ERROR:../qom/object.c:508:object_initialize_with_type: assertion failed: > (type->abstract == false) > Bail out! ERROR:../qom/object.c:508:object_initialize_with_type: > assertion failed: (type->abstract == false) > Aborted (core dumped) > > Broken right in the commit that added the stuff. The sugar never > worked, and should be taken out again. Agreed. > "braille" is the only driver with a factory. "-usbdevice braille" is > sugar for > > -device usb-braille,chardev=braille -chardev braille,id=braille > -machine usb=on > > It's buggy: > > $ qemu-system-x86_64 -S -usbdevice braille > qemu-system-x86_64: -usbdevice braille: '-usbdevice' is deprecated, > please use '-device usb-...' instead > [three seconds tick by...] > Segmentation fault (core dumped) Also breaks in the same way with "./qemu-system-x86_64 -S -chardev braille,id=b", so it's irrelevant. > It neglects to actually parse PARAMS: > > $ qemu-system-x86_64 -S -usbdevice braille:"I'm a Little Teapot" > qemu-system-x86_64: -usbdevice braille:I'm a Little Teapot: '-usbdevice' > is deprecated, please use '-device usb-...' instead > [three seconds tick by...] > Segmentation fault (core dumped) > > The whole machinery in support of optional PARAMS has long become > useless. Agreed. But if parameters and u2f-key are removed, in a separate patch even, then -usbdevice can be kept as it is in wide use in the wild and there are no specific issues to be worried about. Paolo
Re: [PATCH v2] usb: Un-deprecate -usbdevice (except for -usbdevice audio which gets removed)
Daniel P. Berrangé, le mer. 10 mars 2021 15:31:48 +, a ecrit: > On Wed, Mar 10, 2021 at 04:26:46PM +0100, Paolo Bonzini wrote: > > On 10/03/21 16:02, Samuel Thibault wrote: > > > > > When trying to remove the -usbdevice option, there were complaints > > > > > that > > > > > "-usbdevice braille" is still a very useful shortcut for some people. > > > > Pointer? I missed it. > > > > > > For instance > > > https://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg00693.html > > > > In one sentence: "Braille is worth a special case because a subset of our > > user base (blind people) will use it 100% of the time, plus it is not > > supported by libvirt and hence virt-manager". > > If simplicity of enabling braille support is critical, we could get > something even simpler than "-usbdevice braille", and just provide > a bare "-braille" with no args required as a "do the right thing" > option ? That was discussed a bit earlier in the thread: https://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg00681.html https://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg00686.html https://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg00687.html Just like keyboard/mouse, one would still want to specify whether the braille device is to be connected through usb or serial, so at least "-braille usb" and "-braille serial". Note https://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg00689.html Paolo wrote: > Adding magic to "-device usb-braille" that creates both a > front-end and a back-end is completely the opposite of sane... The thing is: creating one without the other does not make sense. Samuel
Re: [PATCH v2] usb: Un-deprecate -usbdevice (except for -usbdevice audio which gets removed)
On Wed, Mar 10, 2021 at 04:26:46PM +0100, Paolo Bonzini wrote: > On 10/03/21 16:02, Samuel Thibault wrote: > > > > When trying to remove the -usbdevice option, there were complaints that > > > > "-usbdevice braille" is still a very useful shortcut for some people. > > > Pointer? I missed it. > > > > For instance > > https://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg00693.html > > In one sentence: "Braille is worth a special case because a subset of our > user base (blind people) will use it 100% of the time, plus it is not > supported by libvirt and hence virt-manager". If simplicity of enabling braille support is critical, we could get something even simpler than "-usbdevice braille", and just provide a bare "-braille" with no args required as a "do the right thing" option ? 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] usb: Un-deprecate -usbdevice (except for -usbdevice audio which gets removed)
On 10/03/21 16:02, Samuel Thibault wrote: When trying to remove the -usbdevice option, there were complaints that "-usbdevice braille" is still a very useful shortcut for some people. Pointer? I missed it. For instance https://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg00693.html In one sentence: "Braille is worth a special case because a subset of our user base (blind people) will use it 100% of the time, plus it is not supported by libvirt and hence virt-manager". Paolo
Re: [PATCH v2] usb: Un-deprecate -usbdevice (except for -usbdevice audio which gets removed)
Markus Armbruster, le mer. 10 mars 2021 14:17:48 +0100, a ecrit: > Thomas Huth writes: > > When trying to remove the -usbdevice option, there were complaints that > > "-usbdevice braille" is still a very useful shortcut for some people. > > Pointer? I missed it. For instance https://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg00693.html > "braille" is the only driver with a factory. "-usbdevice braille" is > sugar for > > -device usb-braille,chardev=braille -chardev braille,id=braille > -machine usb=on > > It's buggy: > > $ qemu-system-x86_64 -S -usbdevice braille > qemu-system-x86_64: -usbdevice braille: '-usbdevice' is deprecated, > please use '-device usb-...' instead > [three seconds tick by...] > Segmentation fault (core dumped) I don't have time to keep testing the usbdevice braille option of qemu from git indeed. But I do use the option at least weekly/monthly with releases. And some braille users do depend on it for their daily work. > It neglects to actually parse PARAMS: > > $ qemu-system-x86_64 -S -usbdevice braille:"I'm a Little Teapot" > qemu-system-x86_64: -usbdevice braille:I'm a Little Teapot: '-usbdevice' > is deprecated, please use '-device usb-...' instead I wasn't aware of the issue, thus not fixed of course. > The only one that has something approaching a leg to stand on is > braille. Still, I fail to see why having to specify a backend is fine > for any number of other devices, but not for braille. Because people who use this option have no idea how they would be supposed to make it work otherwise. > -device usb-braille,chardev=braille -chardev braille,id=braille > -machine usb=on This is *way* beyond what people would work out, even if documented. For them a braille device is just one device, there is no need for notion of frontend/backend, this is all in just one "device" for them. Put another way, it does not even make sense to build anything different from what you wrote: using another chardev backend with usb-braille frontend doesn't make sense, and exposing the braille chardev backend on another usb frontend doesn't make sense either. > If users need more time, we can extend the grace period. They don't need time, they need things that are simple to use. That three-option black magic really isn't. Samuel
Re: [PATCH v2] usb: Un-deprecate -usbdevice (except for -usbdevice audio which gets removed)
Thomas Huth writes: > When trying to remove the -usbdevice option, there were complaints that > "-usbdevice braille" is still a very useful shortcut for some people. Pointer? I missed it. > Thus we never remove this option. Since it's not such a big burden to > keep it around, and it's also convenient in the sense that you don't > have to worry to enable a host controller explicitly with this option, > we should remove it from he deprecation list again, and rather properly > document the possible device for this option instead. > > However, there is one exception: "-usbdevice audio" should go away, since > audio devices without "audiodev=..." parameter are also on the deprecation > list and you cannot use "-usbdevice audio" with "audiodev". > > Signed-off-by: Thomas Huth To be frank, I don't like this. At all. -usbdevice comes with its own ad hoc mini-language, parsed by usbdevice_create(). Syntax is DRIVER[:PARAMS], where PARAMS defaults to "" and is parsed by an optional DRIVER-specific LegacyUSBFactory. We already dropped multiple drivers: "host", "serial", "disk", "net" (commit 99761176e, v2.12), and "bt" (commit 43d68d0a9, v5.0). We've kept "audio" (dropped in this patch), "tablet", "mouse", "keyboard", "braille", "ccid", and "wacom-tablet". Only "mouse", "tablet", "braille" are documented (fixed in this patch). One more has crept in: "u2f-key" (commit bb014a810, v5.2). It's buggy: $ qemu-system-x86_64 -S -usbdevice u2f-key qemu-system-x86_64: -usbdevice u2f-key: '-usbdevice' is deprecated, please use '-device usb-...' instead ** ERROR:../qom/object.c:508:object_initialize_with_type: assertion failed: (type->abstract == false) Bail out! ERROR:../qom/object.c:508:object_initialize_with_type: assertion failed: (type->abstract == false) Aborted (core dumped) Broken right in the commit that added the stuff. The sugar never worked, and should be taken out again. Without a factory, "-usbdevice BAR" is sugar for -device BAZ -machine usb=on "braille" is the only driver with a factory. "-usbdevice braille" is sugar for -device usb-braille,chardev=braille -chardev braille,id=braille -machine usb=on It's buggy: $ qemu-system-x86_64 -S -usbdevice braille qemu-system-x86_64: -usbdevice braille: '-usbdevice' is deprecated, please use '-device usb-...' instead [three seconds tick by...] Segmentation fault (core dumped) It neglects to actually parse PARAMS: $ qemu-system-x86_64 -S -usbdevice braille:"I'm a Little Teapot" qemu-system-x86_64: -usbdevice braille:I'm a Little Teapot: '-usbdevice' is deprecated, please use '-device usb-...' instead [three seconds tick by...] Segmentation fault (core dumped) The whole machinery in support of optional PARAMS has long become useless. I fail to see why we could drop the sugar for serial, disk, net and host devices, but not for the others. The only one that has something approaching a leg to stand on is braille. Still, I fail to see why having to specify a backend is fine for any number of other devices, but not for braille. Does QEMU really need more ways to do the same things? Underdocumented and undertested ways. Let's drop -usbdevice as planned. If users need more time, we can extend the grace period.
Re: [PATCH v2] usb: Un-deprecate -usbdevice (except for -usbdevice audio which gets removed)
On Tue, Mar 09, 2021 at 06:21:03PM +0100, Paolo Bonzini wrote: > On 09/03/21 17:50, Thomas Huth wrote: > > +``-usbdevice audio`` (removed in 6.0) > > +' > > + > > +This option lacked the possibility to specify an audio backend device. > > +Use ``-device usb-audio`` now instead (and specify a corresponding USB > > +host controller if necessary). > > Perhaps "a corresponding USB host controller or ``-usb``. No need to send > v3 if you send it through your own pull request. Fixed & added to usb queue. thanks, Gerd
Re: [PATCH v2] usb: Un-deprecate -usbdevice (except for -usbdevice audio which gets removed)
On 10/03/2021 11.02, Gerd Hoffmann wrote: On Tue, Mar 09, 2021 at 05:50:35PM +0100, Thomas Huth wrote: When trying to remove the -usbdevice option, there were complaints that "-usbdevice braille" is still a very useful shortcut for some people. Thus we never remove this option. Since it's not such a big burden to keep it around, and it's also convenient in the sense that you don't have to worry to enable a host controller explicitly with this option, we should remove it from he deprecation list again, and rather properly document the possible device for this option instead. However, there is one exception: "-usbdevice audio" should go away, since audio devices without "audiodev=..." parameter are also on the deprecation list and you cannot use "-usbdevice audio" with "audiodev". Signed-off-by: Thomas Huth Want me pick this up or send a pull yourself? Yes, please take it through your USB branch (with or without the modification that Paolo suggested, I don't mind). Thanks, Thomas
Re: [PATCH v2] usb: Un-deprecate -usbdevice (except for -usbdevice audio which gets removed)
On Tue, Mar 09, 2021 at 05:50:35PM +0100, Thomas Huth wrote: > When trying to remove the -usbdevice option, there were complaints that > "-usbdevice braille" is still a very useful shortcut for some people. > Thus we never remove this option. Since it's not such a big burden to > keep it around, and it's also convenient in the sense that you don't > have to worry to enable a host controller explicitly with this option, > we should remove it from he deprecation list again, and rather properly > document the possible device for this option instead. > > However, there is one exception: "-usbdevice audio" should go away, since > audio devices without "audiodev=..." parameter are also on the deprecation > list and you cannot use "-usbdevice audio" with "audiodev". > > Signed-off-by: Thomas Huth Want me pick this up or send a pull yourself? In case of the latter: Acked-by: Gerd Hoffmann take care, Gerd
Re: [PATCH v2] usb: Un-deprecate -usbdevice (except for -usbdevice audio which gets removed)
On Tue, Mar 09, 2021 at 05:50:35PM +0100, Thomas Huth wrote: > When trying to remove the -usbdevice option, there were complaints that > "-usbdevice braille" is still a very useful shortcut for some people. > Thus we never remove this option. Since it's not such a big burden to > keep it around, and it's also convenient in the sense that you don't > have to worry to enable a host controller explicitly with this option, > we should remove it from he deprecation list again, and rather properly > document the possible device for this option instead. > > However, there is one exception: "-usbdevice audio" should go away, since > audio devices without "audiodev=..." parameter are also on the deprecation > list and you cannot use "-usbdevice audio" with "audiodev". > > Signed-off-by: Thomas Huth > --- > v2: Add an entry to removed-features.rst > > docs/system/deprecated.rst | 9 > docs/system/removed-features.rst | 8 +++ > hw/usb/dev-audio.c | 1 - > qemu-options.hx | 38 +++- > softmmu/vl.c | 2 -- > 5 files changed, 40 insertions(+), 18 deletions(-) Reviewed-by: Daniel P. Berrangé 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] usb: Un-deprecate -usbdevice (except for -usbdevice audio which gets removed)
On 09/03/21 17:50, Thomas Huth wrote: +``-usbdevice audio`` (removed in 6.0) +' + +This option lacked the possibility to specify an audio backend device. +Use ``-device usb-audio`` now instead (and specify a corresponding USB +host controller if necessary). Perhaps "a corresponding USB host controller or ``-usb``. No need to send v3 if you send it through your own pull request. Paolo
[PATCH v2] usb: Un-deprecate -usbdevice (except for -usbdevice audio which gets removed)
When trying to remove the -usbdevice option, there were complaints that "-usbdevice braille" is still a very useful shortcut for some people. Thus we never remove this option. Since it's not such a big burden to keep it around, and it's also convenient in the sense that you don't have to worry to enable a host controller explicitly with this option, we should remove it from he deprecation list again, and rather properly document the possible device for this option instead. However, there is one exception: "-usbdevice audio" should go away, since audio devices without "audiodev=..." parameter are also on the deprecation list and you cannot use "-usbdevice audio" with "audiodev". Signed-off-by: Thomas Huth --- v2: Add an entry to removed-features.rst docs/system/deprecated.rst | 9 docs/system/removed-features.rst | 8 +++ hw/usb/dev-audio.c | 1 - qemu-options.hx | 38 +++- softmmu/vl.c | 2 -- 5 files changed, 40 insertions(+), 18 deletions(-) diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst index cfabe69846..816eb4084f 100644 --- a/docs/system/deprecated.rst +++ b/docs/system/deprecated.rst @@ -21,15 +21,6 @@ deprecated. System emulator command line arguments -- -``-usbdevice`` (since 2.10.0) -' - -The ``-usbdevice DEV`` argument is now a synonym for setting -the ``-device usb-DEV`` argument instead. The deprecated syntax -would automatically enable USB support on the machine type. -If using the new syntax, USB support must be explicitly -enabled via the ``-machine usb=on`` argument. - ``-drive file=json:{...{'driver':'file'}}`` (since 3.0) ''' diff --git a/docs/system/removed-features.rst b/docs/system/removed-features.rst index c8481cafbd..f8db76d0b5 100644 --- a/docs/system/removed-features.rst +++ b/docs/system/removed-features.rst @@ -38,6 +38,14 @@ or ``-display default,show-cursor=on`` instead. QEMU 5.0 introduced an alternative syntax to specify the size of the translation block cache, ``-accel tcg,tb-size=``. +``-usbdevice audio`` (removed in 6.0) +' + +This option lacked the possibility to specify an audio backend device. +Use ``-device usb-audio`` now instead (and specify a corresponding USB +host controller if necessary). + + QEMU Machine Protocol (QMP) commands diff --git a/hw/usb/dev-audio.c b/hw/usb/dev-audio.c index e1486f81e0..f5cb246792 100644 --- a/hw/usb/dev-audio.c +++ b/hw/usb/dev-audio.c @@ -1024,7 +1024,6 @@ static const TypeInfo usb_audio_info = { static void usb_audio_register_types(void) { type_register_static(&usb_audio_info); -usb_legacy_register(TYPE_USB_AUDIO, "audio", NULL); } type_init(usb_audio_register_types) diff --git a/qemu-options.hx b/qemu-options.hx index 90801286c6..cef8c2da57 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -1705,7 +1705,7 @@ ERST DEFHEADING() -DEFHEADING(USB options:) +DEFHEADING(USB convenience options:) DEF("usb", 0, QEMU_OPTION_usb, "-usbenable on-board USB host controller (if not enabled by default)\n", @@ -1723,9 +1723,31 @@ DEF("usbdevice", HAS_ARG, QEMU_OPTION_usbdevice, QEMU_ARCH_ALL) SRST ``-usbdevice devname`` -Add the USB device devname. Note that this option is deprecated, -please use ``-device usb-...`` instead. See the chapter about +Add the USB device devname, and enable an on-board USB controller +if possible and necessary (just like it can be done via +``-machine usb=on``). Note that this option is mainly intended for +the user's convenience only. More fine-grained control can be +achieved by selecting a USB host controller (if necessary) and the +desired USB device via the ``-device`` option instead. For example, +instead of using ``-usbdevice mouse`` it is possible to use +``-device qemu-xhci -device usb-mouse`` to connect the USB mouse +to a USB 3.0 controller instead (at least on machines that support +PCI and do not have an USB controller enabled by default yet). +For more details, see the chapter about :ref:`Connecting USB devices` in the System Emulation Users Guide. +Possible devices for devname are: + +``braille`` +Braille device. This will use BrlAPI to display the braille +output on a real or fake device (i.e. it also creates a +corresponding ``braille`` chardev automatically beside the +``usb-braille`` USB device). + +``ccid`` +Smartcard reader device + +``keyboard`` +Standard USB keyboard. Will override the PS/2 keyboard (if present). ``mouse`` Virtual Mouse. This will override the PS/2 mouse emulation when @@ -1737,9 +1759,13 @@ SRST position without having to grab the mouse. Also overrides