qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] usb: assign unique serial numbers to hid devices
@ 2018-09-14  9:56 Gerd Hoffmann
  2018-09-14 10:20 ` Dr. David Alan Gilbert
  0 siblings, 1 reply; 4+ messages in thread
From: Gerd Hoffmann @ 2018-09-14  9:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

Windows guests have trouble dealing with usb devices having identical
serial numbers.  So, assign unique serial numbers to usb hid devices.
All other usb devices have this already.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/dev-hid.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/hw/usb/dev-hid.c b/hw/usb/dev-hid.c
index 62d18290dc..7ad6fb33a9 100644
--- a/hw/usb/dev-hid.c
+++ b/hw/usb/dev-hid.c
@@ -61,7 +61,9 @@ enum {
     STR_PRODUCT_MOUSE,
     STR_PRODUCT_TABLET,
     STR_PRODUCT_KEYBOARD,
-    STR_SERIALNUMBER,
+    STR_SERIAL_MOUSE,
+    STR_SERIAL_TABLET,
+    STR_SERIAL_KEYBOARD,
     STR_CONFIG_MOUSE,
     STR_CONFIG_TABLET,
     STR_CONFIG_KEYBOARD,
@@ -72,7 +74,9 @@ static const USBDescStrings desc_strings = {
     [STR_PRODUCT_MOUSE]    = "QEMU USB Mouse",
     [STR_PRODUCT_TABLET]   = "QEMU USB Tablet",
     [STR_PRODUCT_KEYBOARD] = "QEMU USB Keyboard",
-    [STR_SERIALNUMBER]     = "42", /* == remote wakeup works */
+    [STR_SERIAL_MOUSE]     = "89126",
+    [STR_SERIAL_TABLET]    = "28754",
+    [STR_SERIAL_KEYBOARD]  = "68284",
     [STR_CONFIG_MOUSE]     = "HID Mouse",
     [STR_CONFIG_TABLET]    = "HID Tablet",
     [STR_CONFIG_KEYBOARD]  = "HID Keyboard",
@@ -375,7 +379,7 @@ static const USBDesc desc_mouse = {
         .bcdDevice         = 0,
         .iManufacturer     = STR_MANUFACTURER,
         .iProduct          = STR_PRODUCT_MOUSE,
-        .iSerialNumber     = STR_SERIALNUMBER,
+        .iSerialNumber     = STR_SERIAL_MOUSE,
     },
     .full = &desc_device_mouse,
     .str  = desc_strings,
@@ -389,7 +393,7 @@ static const USBDesc desc_mouse2 = {
         .bcdDevice         = 0,
         .iManufacturer     = STR_MANUFACTURER,
         .iProduct          = STR_PRODUCT_MOUSE,
-        .iSerialNumber     = STR_SERIALNUMBER,
+        .iSerialNumber     = STR_SERIAL_MOUSE,
     },
     .full = &desc_device_mouse,
     .high = &desc_device_mouse2,
@@ -404,7 +408,7 @@ static const USBDesc desc_tablet = {
         .bcdDevice         = 0,
         .iManufacturer     = STR_MANUFACTURER,
         .iProduct          = STR_PRODUCT_TABLET,
-        .iSerialNumber     = STR_SERIALNUMBER,
+        .iSerialNumber     = STR_SERIAL_TABLET,
     },
     .full = &desc_device_tablet,
     .str  = desc_strings,
@@ -418,7 +422,7 @@ static const USBDesc desc_tablet2 = {
         .bcdDevice         = 0,
         .iManufacturer     = STR_MANUFACTURER,
         .iProduct          = STR_PRODUCT_TABLET,
-        .iSerialNumber     = STR_SERIALNUMBER,
+        .iSerialNumber     = STR_SERIAL_TABLET,
     },
     .full = &desc_device_tablet,
     .high = &desc_device_tablet2,
@@ -433,7 +437,7 @@ static const USBDesc desc_keyboard = {
         .bcdDevice         = 0,
         .iManufacturer     = STR_MANUFACTURER,
         .iProduct          = STR_PRODUCT_KEYBOARD,
-        .iSerialNumber     = STR_SERIALNUMBER,
+        .iSerialNumber     = STR_SERIAL_TABLET,
     },
     .full = &desc_device_keyboard,
     .str  = desc_strings,
@@ -447,7 +451,7 @@ static const USBDesc desc_keyboard2 = {
         .bcdDevice         = 0,
         .iManufacturer     = STR_MANUFACTURER,
         .iProduct          = STR_PRODUCT_KEYBOARD,
-        .iSerialNumber     = STR_SERIALNUMBER,
+        .iSerialNumber     = STR_SERIAL_TABLET,
     },
     .full = &desc_device_keyboard,
     .high = &desc_device_keyboard2,
@@ -718,9 +722,7 @@ static void usb_hid_initfn(USBDevice *dev, int kind,
         return;
     }
 
-    if (dev->serial) {
-        usb_desc_set_string(dev, STR_SERIALNUMBER, dev->serial);
-    }
+    usb_desc_create_serial(dev);
     usb_desc_init(dev);
     us->intr = usb_ep_get(dev, USB_TOKEN_IN, 1);
     hid_init(&us->hid, kind, usb_hid_changed);
-- 
2.9.3

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

* Re: [Qemu-devel] [PATCH] usb: assign unique serial numbers to hid devices
  2018-09-14  9:56 [Qemu-devel] [PATCH] usb: assign unique serial numbers to hid devices Gerd Hoffmann
@ 2018-09-14 10:20 ` Dr. David Alan Gilbert
  2018-09-14 10:48   ` Gerd Hoffmann
  0 siblings, 1 reply; 4+ messages in thread
From: Dr. David Alan Gilbert @ 2018-09-14 10:20 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: qemu-devel

* Gerd Hoffmann (kraxel@redhat.com) wrote:
> Windows guests have trouble dealing with usb devices having identical
> serial numbers.  So, assign unique serial numbers to usb hid devices.
> All other usb devices have this already.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

OK, so that's part of https://bugs.launchpad.net/qemu/+bug/1096714
> ---
>  hw/usb/dev-hid.c | 24 +++++++++++++-----------
>  1 file changed, 13 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/usb/dev-hid.c b/hw/usb/dev-hid.c
> index 62d18290dc..7ad6fb33a9 100644
> --- a/hw/usb/dev-hid.c
> +++ b/hw/usb/dev-hid.c
> @@ -61,7 +61,9 @@ enum {
>      STR_PRODUCT_MOUSE,
>      STR_PRODUCT_TABLET,
>      STR_PRODUCT_KEYBOARD,
> -    STR_SERIALNUMBER,
> +    STR_SERIAL_MOUSE,
> +    STR_SERIAL_TABLET,
> +    STR_SERIAL_KEYBOARD,
>      STR_CONFIG_MOUSE,
>      STR_CONFIG_TABLET,
>      STR_CONFIG_KEYBOARD,
> @@ -72,7 +74,9 @@ static const USBDescStrings desc_strings = {
>      [STR_PRODUCT_MOUSE]    = "QEMU USB Mouse",
>      [STR_PRODUCT_TABLET]   = "QEMU USB Tablet",
>      [STR_PRODUCT_KEYBOARD] = "QEMU USB Keyboard",
> -    [STR_SERIALNUMBER]     = "42", /* == remote wakeup works */
> +    [STR_SERIAL_MOUSE]     = "89126",
> +    [STR_SERIAL_TABLET]    = "28754",
> +    [STR_SERIAL_KEYBOARD]  = "68284",

Is there any significance to the magical numbers?

Will this change trigger a serial number change across migrate
or will the current serial be carried across?

Dave

>      [STR_CONFIG_MOUSE]     = "HID Mouse",
>      [STR_CONFIG_TABLET]    = "HID Tablet",
>      [STR_CONFIG_KEYBOARD]  = "HID Keyboard",
> @@ -375,7 +379,7 @@ static const USBDesc desc_mouse = {
>          .bcdDevice         = 0,
>          .iManufacturer     = STR_MANUFACTURER,
>          .iProduct          = STR_PRODUCT_MOUSE,
> -        .iSerialNumber     = STR_SERIALNUMBER,
> +        .iSerialNumber     = STR_SERIAL_MOUSE,
>      },
>      .full = &desc_device_mouse,
>      .str  = desc_strings,
> @@ -389,7 +393,7 @@ static const USBDesc desc_mouse2 = {
>          .bcdDevice         = 0,
>          .iManufacturer     = STR_MANUFACTURER,
>          .iProduct          = STR_PRODUCT_MOUSE,
> -        .iSerialNumber     = STR_SERIALNUMBER,
> +        .iSerialNumber     = STR_SERIAL_MOUSE,
>      },
>      .full = &desc_device_mouse,
>      .high = &desc_device_mouse2,
> @@ -404,7 +408,7 @@ static const USBDesc desc_tablet = {
>          .bcdDevice         = 0,
>          .iManufacturer     = STR_MANUFACTURER,
>          .iProduct          = STR_PRODUCT_TABLET,
> -        .iSerialNumber     = STR_SERIALNUMBER,
> +        .iSerialNumber     = STR_SERIAL_TABLET,
>      },
>      .full = &desc_device_tablet,
>      .str  = desc_strings,
> @@ -418,7 +422,7 @@ static const USBDesc desc_tablet2 = {
>          .bcdDevice         = 0,
>          .iManufacturer     = STR_MANUFACTURER,
>          .iProduct          = STR_PRODUCT_TABLET,
> -        .iSerialNumber     = STR_SERIALNUMBER,
> +        .iSerialNumber     = STR_SERIAL_TABLET,
>      },
>      .full = &desc_device_tablet,
>      .high = &desc_device_tablet2,
> @@ -433,7 +437,7 @@ static const USBDesc desc_keyboard = {
>          .bcdDevice         = 0,
>          .iManufacturer     = STR_MANUFACTURER,
>          .iProduct          = STR_PRODUCT_KEYBOARD,
> -        .iSerialNumber     = STR_SERIALNUMBER,
> +        .iSerialNumber     = STR_SERIAL_TABLET,
>      },
>      .full = &desc_device_keyboard,
>      .str  = desc_strings,
> @@ -447,7 +451,7 @@ static const USBDesc desc_keyboard2 = {
>          .bcdDevice         = 0,
>          .iManufacturer     = STR_MANUFACTURER,
>          .iProduct          = STR_PRODUCT_KEYBOARD,
> -        .iSerialNumber     = STR_SERIALNUMBER,
> +        .iSerialNumber     = STR_SERIAL_TABLET,
>      },
>      .full = &desc_device_keyboard,
>      .high = &desc_device_keyboard2,
> @@ -718,9 +722,7 @@ static void usb_hid_initfn(USBDevice *dev, int kind,
>          return;
>      }
>  
> -    if (dev->serial) {
> -        usb_desc_set_string(dev, STR_SERIALNUMBER, dev->serial);
> -    }
> +    usb_desc_create_serial(dev);
>      usb_desc_init(dev);
>      us->intr = usb_ep_get(dev, USB_TOKEN_IN, 1);
>      hid_init(&us->hid, kind, usb_hid_changed);
> -- 
> 2.9.3
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

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

* Re: [Qemu-devel] [PATCH] usb: assign unique serial numbers to hid devices
  2018-09-14 10:20 ` Dr. David Alan Gilbert
@ 2018-09-14 10:48   ` Gerd Hoffmann
  2018-09-14 10:57     ` Dr. David Alan Gilbert
  0 siblings, 1 reply; 4+ messages in thread
From: Gerd Hoffmann @ 2018-09-14 10:48 UTC (permalink / raw)
  To: Dr. David Alan Gilbert; +Cc: qemu-devel

> > +    [STR_SERIAL_MOUSE]     = "89126",
> > +    [STR_SERIAL_TABLET]    = "28754",
> > +    [STR_SERIAL_KEYBOARD]  = "68284",
> 
> Is there any significance to the magical numbers?

No, other than that the three device types should have different ones.

> Will this change trigger a serial number change across migrate
> or will the current serial be carried across?

No, it is not migrated over.
So I guess we should better add a compat property ...

cheers,
  Gerd

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

* Re: [Qemu-devel] [PATCH] usb: assign unique serial numbers to hid devices
  2018-09-14 10:48   ` Gerd Hoffmann
@ 2018-09-14 10:57     ` Dr. David Alan Gilbert
  0 siblings, 0 replies; 4+ messages in thread
From: Dr. David Alan Gilbert @ 2018-09-14 10:57 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: qemu-devel

* Gerd Hoffmann (kraxel@redhat.com) wrote:
> > > +    [STR_SERIAL_MOUSE]     = "89126",
> > > +    [STR_SERIAL_TABLET]    = "28754",
> > > +    [STR_SERIAL_KEYBOARD]  = "68284",
> > 
> > Is there any significance to the magical numbers?
> 
> No, other than that the three device types should have different ones.

OK, I had vague memories a while ago of us asking about encoding things
in serial numbers and the previous 42 not actually being magical.

> > Will this change trigger a serial number change across migrate
> > or will the current serial be carried across?
> 
> No, it is not migrated over.
> So I guess we should better add a compat property ...

Thanks,

Dave

> cheers,
>   Gerd
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

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

end of thread, other threads:[~2018-09-14 10:57 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-09-14  9:56 [Qemu-devel] [PATCH] usb: assign unique serial numbers to hid devices Gerd Hoffmann
2018-09-14 10:20 ` Dr. David Alan Gilbert
2018-09-14 10:48   ` Gerd Hoffmann
2018-09-14 10:57     ` Dr. David Alan Gilbert

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).