[Spice-devel] [USBCLERK PATCH] get_dev_info: ignore devices with no "install_state" information

[Hans de Goede]

ACK.

On 06/09/2013 01:30 PM, Uri Lublin wrote:
> It may happen (not sure how) that a single device appears
> twice in the list of USB devices. Only one entry has information
> for the currently installed device/driver.
>
> If that happens, currently we just pick the first one, which
> may be an invalid entry that says a nont-WinUSB (e.g. USBSTOR)
> is the driver installed, and WinUSB driver removal fails
> (saying it's already not installed).
>
> Experiments show that for such an invalid entry, getting "install_state"
> property fails.
>
> This patch makes get_dev_info ignore devices with no "install_state"
> and pick up the right entry (e.g. with WinUSB driver when driver
> removal is requested).
> ---
>
> I'm not sure why I have not seen this before.
> I encountered that problem when running spice-client on a WinXP VM,
> and trying to usbredir a USB storage device. Upon stopping usbredir
> of the device the WinUSB driver removal by usbclerk failed.
> Maybe changes in qemu-kvm USB emulation caused this, maybe
> something else.
>
> ---
>   usbclerk.cpp |    7 ++++++-
>   1 files changed, 6 insertions(+), 1 deletions(-)
>
> diff --git a/usbclerk.cpp b/usbclerk.cpp
> index 7df11f5..170ff99 100644
> --- a/usbclerk.cpp
> +++ b/usbclerk.cpp
> @@ -616,12 +616,17 @@ bool USBClerk::get_dev_info(HDEVINFO devs, int vid, int pid, SP_DEVINFO_DATA *de
>       TCHAR dev_id[MAX_DEVICE_ID_LEN];
>       TCHAR service_name[MAX_DEVICE_PROP_LEN];
>       bool dev_found = false;
> +    DWORD state_install;
>
>       _sntprintf(dev_prefix, MAX_DEVICE_ID_LEN, TEXT("USB\\VID_%04X&PID_%04X\\"), vid, pid);
>       dev_info->cbSize = sizeof(*dev_info);
>       for (DWORD dev_index = 0; SetupDiEnumDeviceInfo(devs, dev_index, dev_info); dev_index++) {
>           if (SetupDiGetDeviceInstanceId(devs, dev_info, dev_id, MAX_DEVICE_ID_LEN, NULL) &&
> -                (dev_found = !!wcsstr(dev_id, dev_prefix))) {
> +            wcsstr(dev_id, dev_prefix) &&
> +            SetupDiGetDeviceRegistryProperty(devs, dev_info, SPDRP_INSTALL_STATE, NULL,
> +                                             (PBYTE)&state_install, sizeof(state_install),
> +                                             NULL)) {
> +            dev_found = true;
>               break;
>           }
>       }
>