Re: [spice-gtk PATCH 0/9] Windows: identify USB devices by vid:pid instead of bus.address

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 03/25/2013 12:17 PM, Hans de Goede wrote:
Hi,

Hi Hans,

Thanks for reviewing.


On 03/25/2013 11:01 AM, Uri Lublin wrote:
rhbz#842816

It seems that sometimes a USB device's bus.address is changing after
installation of WinUSB driver for that device.
So instead use vid:pid which are consistent across driver installation.

The switch to using vid:pid is done in patch 8/9.

Some code reuses variables named "bus" and "address" to hold vid and pid
values. If people think this should be changed to new variables names (ifdefed) and/or new function instead of spice_usb_device_manager_get_udev_bus_n_address
let me know.

Using two devices with the same vid:pid on the same Windows client is already
not supported, since Windows installs USB drivers for specific devices
based on their vid:pid

Hmm, I'm not very happy with the approach you've chosen here. Just because usbclerck / our driver swapping magic currently does not support having devices with identical vid:pid is not a good reason to add the same problem to other parts of the code. The contrary is true really, this is something which we
should try to fix, not make worse.

I'm only trying to fix the Windows bus.addr inconsistency problem.
I don't think that makes things worse, but better.
I agree that this patch-set inserts the existing driver limitation
(of using two devices with the same vid:pid) into spice-gtk,
for Windows (and keeps Linux as is).
This limitation already exists for Windows clients, and
we can remove it from spice-gtk in the future if needed.


Have you tried using libusb_get_port_path? I know that is sensitive to
device swap races on the same port, so how about using a combination of
libusb_get_port_path + vid & pid to identify a device?

I did not try using libusb_get_port_path.
It may be a better solution:
 - It works the same for both Linux and Windows.
- It fixes the Windows problem (all should be consistent through driver install)
 - It is a more robust solution for Linux (is it really ?).
   + Wouldn't device_address change upon device_swap ?
It's also requires a bit more memory.

We'd need to use bus_number as part of device identification.
There may be two different devices on two different buses that have
the same port_path (at least that's the case on Linux).

This seems like a feature (bug?) of libusbx:
I changed libusbx's examples/listdevs.c to print port_number and port_path
and plugged the same device on my machine's ports (diff attached):

$ ./listdevs | grep 0457
0457:0151 (bus 1, device 26 port=1) (port_path[2]={1 1 }
$ ./listdevs | grep 0457
0457:0151 (bus 2, device 7 port=1) (port_path[2]={1 1 }

So the identifier must be vid:pid, bus_number, port_path.

Thanks,
    Uri.


diff --git a/examples/listdevs.c b/examples/listdevs.c
index e3d7ef0..f02c795 100644
--- a/examples/listdevs.c
+++ b/examples/listdevs.c
@@ -24,6 +24,9 @@
 static void print_devs(libusb_device **devs)
 {
 	libusb_device *dev;
+	uint8_t port_path[10];
+	uint8_t port_path_len = 10;
+	int npaths, j;
 	int i = 0;
 
 	while ((dev = devs[i++]) != NULL) {
@@ -34,9 +37,18 @@ static void print_devs(libusb_device **devs)
 			return;
 		}
 
-		printf("%04x:%04x (bus %d, device %d)\n",
+		npaths = libusb_get_port_path(NULL, dev, port_path, port_path_len);
+		printf("%04x:%04x (bus %d, device %d port=%d) (port_path[%d]={",
 			desc.idVendor, desc.idProduct,
-			libusb_get_bus_number(dev), libusb_get_device_address(dev));
+			libusb_get_bus_number(dev), libusb_get_device_address(dev),
+			libusb_get_port_number(dev),
+			npaths);
+
+		for (j=0; j<npaths; j++) {
+			printf("%u ", port_path[j]);
+		}
+
+		printf("}\n");
 	}
 }
 
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel

[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]