* Gerd Hoffmann (kra...@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.
>
> In the past the fixed serial number has been used to indicate working
> remote setup to linux guests. Here is a bit of history:
>
> * First there was nothing.
> * Then I added a rule to udev checking for serial == 42.
> (this is in rhel-6).
> * Then systemd + udev merged.
> * Then I changed the rule to check for serial != 1 instead, so we can
> use any serial but "1" which is the one the old broken devices had
> (this is in rhel-7). March 2014 in upstream systemd.
> * Then all usb power management rules where dropped from systemd (June
> 2015). Which I figured today (Sept 2018), after wondering that the
> rules are gone in fedora 28.
>
> So, three years ago the serial number check was dropped upstream, yet I
> hav't seen a single report about autosuspend issues (or cpu usage for
> usb emulation going up, which is the typical symtom).
>
> So I figured I can stop worring that changing the serial number will
> break things and just do it.
>
> And even if it turns out autosuspend is still an issue: I think
> meanwhile we can really stop worrying about guests running in old qemu
> versions with broken usb suspend (fixed in 0.13 !). If needed we can
> enable autosuspend unconditionally in guests.
>
> Signed-off-by: Gerd Hoffmann <kra...@redhat.com>
> Message-id: 20190110125108.22834-1-kra...@redhat.com
> ---
> hw/core/machine.c | 3 +++
> hw/usb/dev-hid.c | 26 +++++++++++++++-----------
> 2 files changed, 18 insertions(+), 11 deletions(-)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 2629515363..077fbd182a 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -30,6 +30,9 @@ GlobalProperty hw_compat_3_1[] = {
> { "memory-backend-memfd", "x-use-canonical-path-for-ramblock-id", "true"
> },
> { "tpm-crb", "ppi", "false" },
> { "tpm-tis", "ppi", "false" },
> + { "usb-kbd", "serial", "42" },
> + { "usb-mouse", "serial", "42" },
> + { "usb-kbd", "serial", "42" },
Hi Gerd,
There's a copy-pasteism there that happened when you squashed
it down to the new format; you've got the usb-kbd twice
as opposed to having the usb-tablet in there.
Hmm, now how do we fix that? That means if we fix that
now then the usb 3-1 machine type in 4.1 would be the same as
3.1 but different from 4.0; which I suspect is the right fix
at this time.
Dave
> };
> const size_t hw_compat_3_1_len = G_N_ELEMENTS(hw_compat_3_1);
>
> diff --git a/hw/usb/dev-hid.c b/hw/usb/dev-hid.c
> index 90cd745f06..f9ea3033a1 100644
> --- a/hw/usb/dev-hid.c
> +++ b/hw/usb/dev-hid.c
> @@ -61,10 +61,13 @@ enum {
> STR_PRODUCT_MOUSE,
> STR_PRODUCT_TABLET,
> STR_PRODUCT_KEYBOARD,
> - STR_SERIALNUMBER,
> + STR_SERIAL_COMPAT,
> STR_CONFIG_MOUSE,
> STR_CONFIG_TABLET,
> STR_CONFIG_KEYBOARD,
> + STR_SERIAL_MOUSE,
> + STR_SERIAL_TABLET,
> + STR_SERIAL_KEYBOARD,
> };
>
> static const USBDescStrings desc_strings = {
> @@ -72,10 +75,13 @@ 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_COMPAT] = "42",
> [STR_CONFIG_MOUSE] = "HID Mouse",
> [STR_CONFIG_TABLET] = "HID Tablet",
> [STR_CONFIG_KEYBOARD] = "HID Keyboard",
> + [STR_SERIAL_MOUSE] = "89126",
> + [STR_SERIAL_TABLET] = "28754",
> + [STR_SERIAL_KEYBOARD] = "68284",
> };
>
> static const USBDescIface desc_iface_mouse = {
> @@ -375,7 +381,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 +395,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 +410,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 +424,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 +439,7 @@ static const USBDesc desc_keyboard = {
> .bcdDevice = 0,
> .iManufacturer = STR_MANUFACTURER,
> .iProduct = STR_PRODUCT_KEYBOARD,
> - .iSerialNumber = STR_SERIALNUMBER,
> + .iSerialNumber = STR_SERIAL_KEYBOARD,
> },
> .full = &desc_device_keyboard,
> .str = desc_strings,
> @@ -447,7 +453,7 @@ static const USBDesc desc_keyboard2 = {
> .bcdDevice = 0,
> .iManufacturer = STR_MANUFACTURER,
> .iProduct = STR_PRODUCT_KEYBOARD,
> - .iSerialNumber = STR_SERIALNUMBER,
> + .iSerialNumber = STR_SERIAL_KEYBOARD,
> },
> .full = &desc_device_keyboard,
> .high = &desc_device_keyboard2,
> @@ -718,9 +724,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 / dgilb...@redhat.com / Manchester, UK
