1. Patchew
  2. QEMU
  3. View series

[Qemu-devel] [PATCH] usb: assign unique serial numbers to hid devices

Gerd Hoffmann posted 1 patch 7 years, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20180914095645.24612-1-kraxel@redhat.com
Test docker-clang@ubuntu failed
Test checkpatch passed
There is a newer version of this series
hw/usb/dev-hid.c | 24 +++++++++++++-----------
1 file changed, 13 insertions(+), 11 deletions(-)
[Qemu-devel] [PATCH] usb: assign unique serial numbers to hid devices
Posted by Gerd Hoffmann 7 years, 1 month ago 
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
Re: [Qemu-devel] [PATCH] usb: assign unique serial numbers to hid devices
Posted by Dr. David Alan Gilbert 7 years, 1 month ago 
* 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
Re: [Qemu-devel] [PATCH] usb: assign unique serial numbers to hid devices
Posted by Gerd Hoffmann 7 years, 1 month ago 
> > +    [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
Re: [Qemu-devel] [PATCH] usb: assign unique serial numbers to hid devices
Posted by Dr. David Alan Gilbert 7 years, 1 month ago 
* 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

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.