[Spice-devel] [spice-gtk Win32 v4 06/17] Windows mingw: usb: Dynamically install a libusb driver for USB devices

[Uri Lublin]

On 07/06/2012 09:46 AM, Hans de Goede wrote:
> Hi,
>
> On 07/05/2012 10:43 PM, Uri Lublin wrote:
>> - Added win-usb-driver-install.[ch]
>> - Added win-usb-clerk.h
>>
>> Operation (on Windows, spice-gtk point of view):
>> - After some sanity checks, just before redir'ing a USB device
>>    a libusb driver needs to be installed (before libusb can open the 
>> device)
>> - A connection (NamedPipe) is established with usb-clerk, a libusb
>>    driver installation service, and a request for driver installation
>>    is sent.
>> - Installation status is asynchronously read from the pipe, and
>>    spice_usb_drv_install_finished() is called.
>> - Upon a successful intallation, usbredir continues.
>>
>> Linux operation is not changed.
>> ---
>>   gtk/Makefile.am              |    3 +
>>   gtk/usb-device-manager.c     |  150 +++++++++++++++-
>>   gtk/win-usb-clerk.h          |   35 ++++
>>   gtk/win-usb-driver-install.c |  387 
>> ++++++++++++++++++++++++++++++++++++++++++
>>   gtk/win-usb-driver-install.h |   98 +++++++++++
>>   5 files changed, 663 insertions(+), 10 deletions(-)
>>   create mode 100644 gtk/win-usb-clerk.h
>>   create mode 100644 gtk/win-usb-driver-install.c
>>   create mode 100644 gtk/win-usb-driver-install.h
>>
>
> <snip>
>
>> @@ -635,7 +649,7 @@ static void 
>> spice_usb_device_manager_add_dev(SpiceUsbDeviceManager  *self,
>>               spice_usb_device_manager_connect_device_async(self,
>>                                      device, NULL,
>>                                      
>> spice_usb_device_manager_auto_connect_cb,
>> -                                   g_object_ref(device));
>> +                                   device);
>>           }
>>       }
>>
>
>
> This seems wrong as spice_usb_device_manager_auto_connect_cb will 
> still do a spice_usb_device_unref, so
> we must ref it here to match, also this seems like something which 
> belongs in a different patch...

OK.

>
>>
>> + */
>> +static void spice_usb_device_manager_drv_install_cb(GObject *gobject,
>> +                                                    GAsyncResult *res,
>> +                                                    gpointer user_data)
>> +{
>> +    SpiceUsbDeviceManager *self;
>> +    SpiceWinUsbDriver *installer;
>> +    gint status;
>> +    GError *err = NULL;
>> +    SpiceUsbDevice *device;
>> +    UsbInstallCbInfo *cbinfo;
>> +    GCancellable *cancellable;
>> +    GAsyncReadyCallback callback;
>> +
>> +    SPICE_DEBUG("Win USB driver Installation finished");
>> +
>> +    g_return_if_fail(user_data != NULL);
>> +
>> +    cbinfo = user_data;
>> +    self        = cbinfo->manager;
>> +    device      = cbinfo->device;
>> +    installer   = cbinfo->installer;
>> +    cancellable = cbinfo->cancellable;
>> +    callback    = cbinfo->callback;
>> +    user_data   = cbinfo->user_data;
>> +
>> +    g_free(cbinfo);
>> +
>> +    g_return_if_fail(SPICE_IS_USB_DEVICE_MANAGER(self));
>> +    g_return_if_fail(SPICE_IS_WIN_USB_DRIVER(installer));
>> +    g_return_if_fail(device!= NULL);
>> +
>> +    status = spice_win_usb_driver_install_finish(installer, res, &err);
>> +
>> +    g_object_unref(installer);
>> +
>> +    if (err) {
>> +        g_warning("win usb driver installation failed -- %s",
>> +                  err->message);
>> +        g_error_free(err);
>> +        spice_usb_device_unref(device);
>> +        return;
>> +    }
>> +
>> +    if (!status) {
>> +        g_warning("failed to install win usb driver (status=0)");
>> +        spice_usb_device_unref(device);
>
> Can you please raise the appropriate error signal here, so that the 
> user gets
> an error dialog ?

In V5, the code does not return here on error.
Instead _spice_usb_device_manager_connect_device_async is always called 
(for install
operation). If the driver failed to install, libusb_open would fail, and 
the error would
be propagated correctly.

>
>> +        return;
>> +    }
>> +
>> +    /* device is already ref'ed */
>> +    _spice_usb_device_manager_connect_device_async(self,
>> +                                                   device,
>> +                                                   cancellable,
>> +                                                   callback,
>> +                                                   user_data);
>> +
>> +}
>> +#endif
>> +
>>   /* 
>> ------------------------------------------------------------------ */
>>   /* private 
>> api                                                        */
>>


Thanks,
     Uri.