All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] usb: Un-deprecate -usbdevice (except for -usbdevice audio which gets removed)
@ 2021-03-09 16:50 Thomas Huth
  2021-03-09 17:16 ` Daniel P. Berrangé
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Thomas Huth @ 2021-03-09 16:50 UTC (permalink / raw)
  To: qemu-devel, Gerd Hoffmann
  Cc: Paolo Bonzini, Daniel P . Berrangé, Samuel Thibault

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 <thuth@redhat.com>
---
 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,
     "-usb            enable 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 the
         PS/2 mouse emulation when activated.
 
-    ``braille``
-        Braille device. This will use BrlAPI to display the braille
-        output on a real or fake device.
+    ``u2f-key``
+        U2F (Universal Second Factor) key.
+
+    ``wacom-tablet``
+        Wacom PenPartner USB tablet.
+
+
 ERST
 
 DEFHEADING()
diff --git a/softmmu/vl.c b/softmmu/vl.c
index ff488ea3e7..76ebe7bb7a 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -3180,8 +3180,6 @@ void qemu_init(int argc, char **argv, char **envp)
                 qemu_opts_parse_noisily(olist, "usb=on", false);
                 break;
             case QEMU_OPTION_usbdevice:
-                error_report("'-usbdevice' is deprecated, please use "
-                             "'-device usb-...' instead");
                 olist = qemu_find_opts("machine");
                 qemu_opts_parse_noisily(olist, "usb=on", false);
                 add_device_config(DEV_USB, optarg);
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v2] usb: Un-deprecate -usbdevice (except for -usbdevice audio which gets removed)
  2021-03-09 16:50 [PATCH v2] usb: Un-deprecate -usbdevice (except for -usbdevice audio which gets removed) Thomas Huth
@ 2021-03-09 17:16 ` Daniel P. Berrangé
  2021-03-09 17:21 ` Paolo Bonzini
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 13+ messages in thread
From: Daniel P. Berrangé @ 2021-03-09 17:16 UTC (permalink / raw)
  To: Thomas Huth; +Cc: Paolo Bonzini, qemu-devel, Samuel Thibault, Gerd Hoffmann

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 <thuth@redhat.com>
> ---
>  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é <berrange@redhat.com>

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 :|



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2] usb: Un-deprecate -usbdevice (except for -usbdevice audio which gets removed)
  2021-03-09 16:50 [PATCH v2] usb: Un-deprecate -usbdevice (except for -usbdevice audio which gets removed) Thomas Huth
  2021-03-09 17:16 ` Daniel P. Berrangé
@ 2021-03-09 17:21 ` Paolo Bonzini
  2021-03-10 12:14   ` Gerd Hoffmann
  2021-03-10 10:02 ` Gerd Hoffmann
  2021-03-10 13:17 ` Markus Armbruster
  3 siblings, 1 reply; 13+ messages in thread
From: Paolo Bonzini @ 2021-03-09 17:21 UTC (permalink / raw)
  To: Thomas Huth, qemu-devel, Gerd Hoffmann
  Cc: Samuel Thibault, Daniel P . Berrangé

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



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2] usb: Un-deprecate -usbdevice (except for -usbdevice audio which gets removed)
  2021-03-09 16:50 [PATCH v2] usb: Un-deprecate -usbdevice (except for -usbdevice audio which gets removed) Thomas Huth
  2021-03-09 17:16 ` Daniel P. Berrangé
  2021-03-09 17:21 ` Paolo Bonzini
@ 2021-03-10 10:02 ` Gerd Hoffmann
  2021-03-10 10:06   ` Thomas Huth
  2021-03-10 13:17 ` Markus Armbruster
  3 siblings, 1 reply; 13+ messages in thread
From: Gerd Hoffmann @ 2021-03-10 10:02 UTC (permalink / raw)
  To: Thomas Huth
  Cc: Paolo Bonzini, Daniel P . Berrangé, qemu-devel, Samuel Thibault

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 <thuth@redhat.com>

Want me pick this up or send a pull yourself?
In case of the latter:

Acked-by: Gerd Hoffmann <kraxel@redhat.com>

take care,
  Gerd



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2] usb: Un-deprecate -usbdevice (except for -usbdevice audio which gets removed)
  2021-03-10 10:02 ` Gerd Hoffmann
@ 2021-03-10 10:06   ` Thomas Huth
  0 siblings, 0 replies; 13+ messages in thread
From: Thomas Huth @ 2021-03-10 10:06 UTC (permalink / raw)
  To: Gerd Hoffmann
  Cc: Paolo Bonzini, Daniel P. Berrangé, qemu-devel, Samuel Thibault

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 <thuth@redhat.com>
> 
> 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



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2] usb: Un-deprecate -usbdevice (except for -usbdevice audio which gets removed)
  2021-03-09 17:21 ` Paolo Bonzini
@ 2021-03-10 12:14   ` Gerd Hoffmann
  0 siblings, 0 replies; 13+ messages in thread
From: Gerd Hoffmann @ 2021-03-10 12:14 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Samuel Thibault, Thomas Huth, Daniel P . Berrangé, qemu-devel

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



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2] usb: Un-deprecate -usbdevice (except for -usbdevice audio which gets removed)
  2021-03-09 16:50 [PATCH v2] usb: Un-deprecate -usbdevice (except for -usbdevice audio which gets removed) Thomas Huth
                   ` (2 preceding siblings ...)
  2021-03-10 10:02 ` Gerd Hoffmann
@ 2021-03-10 13:17 ` Markus Armbruster
  2021-03-10 15:02   ` Samuel Thibault
                     ` (2 more replies)
  3 siblings, 3 replies; 13+ messages in thread
From: Markus Armbruster @ 2021-03-10 13:17 UTC (permalink / raw)
  To: Thomas Huth
  Cc: Paolo Bonzini, Daniel P . Berrangé,
	qemu-devel, Samuel Thibault, Gerd Hoffmann

Thomas Huth <thuth@redhat.com> 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 <thuth@redhat.com>

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.



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2] usb: Un-deprecate -usbdevice (except for -usbdevice audio which gets removed)
  2021-03-10 13:17 ` Markus Armbruster
@ 2021-03-10 15:02   ` Samuel Thibault
  2021-03-10 15:26     ` Paolo Bonzini
  2021-03-10 15:44   ` Paolo Bonzini
  2021-03-10 16:06   ` Thomas Huth
  2 siblings, 1 reply; 13+ messages in thread
From: Samuel Thibault @ 2021-03-10 15:02 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: Paolo Bonzini, Thomas Huth, Daniel P . Berrangé,
	qemu-devel, Gerd Hoffmann

Markus Armbruster, le mer. 10 mars 2021 14:17:48 +0100, a ecrit:
> Thomas Huth <thuth@redhat.com> 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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2] usb: Un-deprecate -usbdevice (except for -usbdevice audio which gets removed)
  2021-03-10 15:02   ` Samuel Thibault
@ 2021-03-10 15:26     ` Paolo Bonzini
  2021-03-10 15:31       ` Daniel P. Berrangé
  0 siblings, 1 reply; 13+ messages in thread
From: Paolo Bonzini @ 2021-03-10 15:26 UTC (permalink / raw)
  To: Samuel Thibault, Markus Armbruster
  Cc: Thomas Huth, Daniel P. Berrangé, qemu-devel, Gerd Hoffmann

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



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2] usb: Un-deprecate -usbdevice (except for -usbdevice audio which gets removed)
  2021-03-10 15:26     ` Paolo Bonzini
@ 2021-03-10 15:31       ` Daniel P. Berrangé
  2021-03-10 15:43         ` Samuel Thibault
  0 siblings, 1 reply; 13+ messages in thread
From: Daniel P. Berrangé @ 2021-03-10 15:31 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: qemu-devel, Thomas Huth, Gerd Hoffmann, Samuel Thibault,
	Markus Armbruster

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 :|



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2] usb: Un-deprecate -usbdevice (except for -usbdevice audio which gets removed)
  2021-03-10 15:31       ` Daniel P. Berrangé
@ 2021-03-10 15:43         ` Samuel Thibault
  0 siblings, 0 replies; 13+ messages in thread
From: Samuel Thibault @ 2021-03-10 15:43 UTC (permalink / raw)
  To: Daniel P. Berrangé
  Cc: Paolo Bonzini, Thomas Huth, Gerd Hoffmann, Markus Armbruster, qemu-devel

Daniel P. Berrangé, le mer. 10 mars 2021 15:31:48 +0000, 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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2] usb: Un-deprecate -usbdevice (except for -usbdevice audio which gets removed)
  2021-03-10 13:17 ` Markus Armbruster
  2021-03-10 15:02   ` Samuel Thibault
@ 2021-03-10 15:44   ` Paolo Bonzini
  2021-03-10 16:06   ` Thomas Huth
  2 siblings, 0 replies; 13+ messages in thread
From: Paolo Bonzini @ 2021-03-10 15:44 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: Samuel Thibault, Thomas Huth, Daniel P . Berrangé,
	qemu-devel, Gerd Hoffmann

On Wed, Mar 10, 2021 at 2:17 PM Markus Armbruster <armbru@redhat.com> 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



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2] usb: Un-deprecate -usbdevice (except for -usbdevice audio which gets removed)
  2021-03-10 13:17 ` Markus Armbruster
  2021-03-10 15:02   ` Samuel Thibault
  2021-03-10 15:44   ` Paolo Bonzini
@ 2021-03-10 16:06   ` Thomas Huth
  2 siblings, 0 replies; 13+ messages in thread
From: Thomas Huth @ 2021-03-10 16:06 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: Paolo Bonzini, Daniel P.Berrangé,
	qemu-devel, Samuel Thibault, Gerd Hoffmann

On 10/03/2021 14.17, Markus Armbruster wrote:
> Thomas Huth <thuth@redhat.com> 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 <thuth@redhat.com>
> 
> 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



^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2021-03-10 16:53 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-09 16:50 [PATCH v2] usb: Un-deprecate -usbdevice (except for -usbdevice audio which gets removed) Thomas Huth
2021-03-09 17:16 ` Daniel P. Berrangé
2021-03-09 17:21 ` Paolo Bonzini
2021-03-10 12:14   ` Gerd Hoffmann
2021-03-10 10:02 ` Gerd Hoffmann
2021-03-10 10:06   ` Thomas Huth
2021-03-10 13:17 ` Markus Armbruster
2021-03-10 15:02   ` Samuel Thibault
2021-03-10 15:26     ` Paolo Bonzini
2021-03-10 15:31       ` Daniel P. Berrangé
2021-03-10 15:43         ` Samuel Thibault
2021-03-10 15:44   ` Paolo Bonzini
2021-03-10 16:06   ` Thomas Huth

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.