From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52111) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1g0mMd-0005zf-M9 for qemu-devel@nongnu.org; Fri, 14 Sep 2018 07:34:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1g0mMY-0003aH-OZ for qemu-devel@nongnu.org; Fri, 14 Sep 2018 07:34:19 -0400 Received: from mx1.redhat.com ([209.132.183.28]:58572) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1g0mMY-0003a0-GM for qemu-devel@nongnu.org; Fri, 14 Sep 2018 07:34:14 -0400 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id B8CB787637 for ; Fri, 14 Sep 2018 11:34:13 +0000 (UTC) Date: Fri, 14 Sep 2018 12:34:08 +0100 From: "Dr. David Alan Gilbert" Message-ID: <20180914113407.GB3081@work-vm> References: <20180914104835.26019-1-kraxel@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180914104835.26019-1-kraxel@redhat.com> Subject: Re: [Qemu-devel] [PATCH v2] usb: assign unique serial numbers to hid devices List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gerd Hoffmann Cc: qemu-devel@nongnu.org * 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 > --- > include/hw/compat.h | 14 +++++++++++++- > hw/usb/dev-hid.c | 24 +++++++++++++----------- > 2 files changed, 26 insertions(+), 12 deletions(-) > > diff --git a/include/hw/compat.h b/include/hw/compat.h > index 6f4d5fc647..c059459394 100644 > --- a/include/hw/compat.h > +++ b/include/hw/compat.h > @@ -2,7 +2,19 @@ > #define HW_COMPAT_H > > #define HW_COMPAT_3_0 \ > - /* empty */ > + {\ > + .driver = "usb-kbd",\ > + .property = "serial",\ > + .value = "42",\ > + },{\ > + .driver = "usb-mouse",\ > + .property = "serial",\ > + .value = "42",\ > + },{\ > + .driver = "usb-tablet",\ > + .property = "serial",\ > + .value = "42",\ > + }, > #define HW_COMPAT_2_12 \ > {\ > 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", OK, so now I've found the bit about the magic 42; commit 7b074a22 of yours; recommended checking for 42 for knowing we had autosuspend; what's actually in the current fedora hid rules is serial!=1 - I wonder what others have. It's a shame these are random numbers rather than 2,3,4; but still. > [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); In the cases where I've not passed serial= on the command line, who wins, does this mean create_serial uses the path or does it use the magic constants you set above? Dave > 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