[Spice-devel,spice-gtk,Win32,v5,22/22] usb-device-manager: mingw: ignore "remove" udev event when un/installing a driver

Submitted by Uri Lublin on July 9, 2012, 12:15 p.m.

Details

Message ID c855af058e67a156ef1d4f5b736ee08f3debb050.1341834461.git.uril@redhat.com
State New
Headers show

Not browsing as part of any series.

Commit Message

Uri Lublin July 9, 2012, 12:15 p.m.
---
 gtk/usb-device-manager.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

Patch hide | download patch | download mbox

diff --git a/gtk/usb-device-manager.c b/gtk/usb-device-manager.c
index 65119ad..2a92ff8 100644
--- a/gtk/usb-device-manager.c
+++ b/gtk/usb-device-manager.c
@@ -719,6 +719,16 @@  static void spice_usb_device_manager_remove_dev(SpiceUsbDeviceManager  *self,
         return;
     }
 
+#ifdef G_OS_WIN32
+    const guint8 state = spice_usb_device_get_state(device);
+    if ((state == SPICE_USB_DEVICE_STATE_INSTALLING) ||
+        (state == SPICE_USB_DEVICE_STATE_UNINSTALLING)) {
+        g_warning("skipping device at %d.%d. It is installing it's driver",
+                  bus, address);
+        return;
+    }
+#endif
+
     spice_usb_device_manager_disconnect_device(self, device);
 
     SPICE_DEBUG("device removed %p", device);

Comments

On Mon, Jul 9, 2012 at 2:15 PM, Uri Lublin <uril@redhat.com> wrote:
> ---
>  gtk/usb-device-manager.c |   10 ++++++++++
>  1 files changed, 10 insertions(+), 0 deletions(-)
>
> diff --git a/gtk/usb-device-manager.c b/gtk/usb-device-manager.c
> index 65119ad..2a92ff8 100644
> --- a/gtk/usb-device-manager.c
> +++ b/gtk/usb-device-manager.c
> @@ -719,6 +719,16 @@ static void spice_usb_device_manager_remove_dev(SpiceUsbDeviceManager  *self,
>          return;
>      }
>
> +#ifdef G_OS_WIN32
> +    const guint8 state = spice_usb_device_get_state(device);
> +    if ((state == SPICE_USB_DEVICE_STATE_INSTALLING) ||
> +        (state == SPICE_USB_DEVICE_STATE_UNINSTALLING)) {
> +        g_warning("skipping device at %d.%d. It is installing it's driver",
> +                  bus, address);
> +        return;
> +    }
> +#endif

What happens if the device is removed before it is installed or
uninstalled? Or a simpler case, do we handle correctly removing a
device currently being redirected? (thinking of unplugged abruptely by
user physically)

We could go with that for now, but it looks suspicious to me.
On 07/09/2012 08:24 PM, Marc-André Lureau wrote:
> On Mon, Jul 9, 2012 at 2:15 PM, Uri Lublin<uril@redhat.com>  wrote:
>> ---
>>   gtk/usb-device-manager.c |   10 ++++++++++
>>   1 files changed, 10 insertions(+), 0 deletions(-)
>>
>> diff --git a/gtk/usb-device-manager.c b/gtk/usb-device-manager.c
>> index 65119ad..2a92ff8 100644
>> --- a/gtk/usb-device-manager.c
>> +++ b/gtk/usb-device-manager.c
>> @@ -719,6 +719,16 @@ static void spice_usb_device_manager_remove_dev(SpiceUsbDeviceManager  *self,
>>           return;
>>       }
>>
>> +#ifdef G_OS_WIN32
>> +    const guint8 state = spice_usb_device_get_state(device);
>> +    if ((state == SPICE_USB_DEVICE_STATE_INSTALLING) ||
>> +        (state == SPICE_USB_DEVICE_STATE_UNINSTALLING)) {
>> +        g_warning("skipping device at %d.%d. It is installing it's driver",
>> +                  bus, address);
>> +        return;
>> +    }
>> +#endif
> What happens if the device is removed before it is installed or
> uninstalled?

If a device is removed during driver installation (or uninstallation), 
we ignore the
udev event. After driver installation, the matching libusb device will 
not be found (or
libusb_open it would fail) and usbredir operation would fail.
In such a scenario there is a problem, where usb-device-manager would 
keep that
device in it's list, and show it in the usb menu. Re-trying to usbredir 
the device would
fail as above.


>   Or a simpler case, do we handle correctly removing a
> device currently being redirected? (thinking of unplugged abruptely by
> user physically)
Yes, this is handled correctly.


We probably need a better state machine, and possibly gather more
information before making decisions (e.g. check what driver is installed
when a "device added" event or "device removed" event are received).

Thanks,
     Uri.