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

[Uri Lublin]

On 03/27/2013 06:13 PM, Hans de Goede wrote:
> Hi,
>
> On 03/27/2013 04:33 PM, Uri Lublin wrote:
>> 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.
>
> Ok, so lets split the discussion in 2:
>
> 1) What do we need today
>
> Today we need a way to work around the bus.addr pair not being stable
> for a single device under Windows, since today we already do NOT
> support using multiple devices with the same vid:pid, using vid:pid
> (iow this patchset) would be an acceptable solution, but only today!

great.

>
> 2) Where do we want to be tomorrow
>
> "Tomorrow" we will hopefully be using filter drivers, and hopefully
> then we don't need the whole driver install dance (even for new devices
> which were never plugged in before?), and then we can remove a whole
> bunch of windows specific "murk" from the spice-gtk code for usb
> handling.

Yes, we want to use a filter driver which needs to be installed only once,
and works for all/more USB devices. We wanted filter driver from
the start, but it was not in good enough a condition.

Once we can use a filter driver, we can remove all the code
that asks usbclerk to install the driver for specific devices
(and of course we will not need usbclerk at all anymore)

>
> So if we can agree on this being a temporary thing, then I can agree
> with the vid:pid approach *for now*, but this really really needs to
> be replaced / fixed in the near future.

I agree. Once the filter driver is good enough we'll switch to using it.
When using a filter driver we can identify devices with bus.addr
on Windows too.

>
> So with this clear, and assuming you will agree, let me actually really
> review this patch-set :)

Thanks.

>
>
> Some remarks on alternative approaches below, but lets just go with
> vid:pid for now.
>
<snipped>
> bus.addr is 100% reliable on Linux, replacing it with bus + port-path
> introduces the issues we're having on Windows without any gains (other 
> then
> unification of the code.

Hopefully with the filter driver, bus.addr is going to be reliable for 
Windows too.

Thanks,
     Uri.