[Spice-devel] [PATCH v5 06/13] UsbDeviceManager: Track device redirection operations in progress
Dmitry Fleytman
dmitry at daynix.com
Sun Feb 28 08:51:29 UTC 2016
> On 10 Feb 2016, at 23:48 PM, Jonathon Jongsma <jjongsma at redhat.com> wrote:
>
> On Thu, 2015-10-29 at 17:27 +0200, Dmitry Fleytman wrote:
>> From: Kirill Moizik <kmoizik at redhat.com>
>>
>> During device connection, unwanted hotplug events may happen.
>> We need to ignore those therefore we track redirection operations
>> in progress.
>>
>> See also comment to commit
>> "Do not process USB hotplug events while redirection is in progress"
>> that introduces corresponding filtering out logic.
>>
>> Signed-off-by: Kirill Moizik <kmoizik at redhat.com>
>> Signed-off-by: Dmitry Fleytman <dfleytma at redhat.com>
>> ---
>> src/usb-device-manager.c | 69 +++++++++++++++++++++++++++++++++++++++--------
>> -
>> 1 file changed, 56 insertions(+), 13 deletions(-)
>>
>> diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c
>> index 4d376b6..0626923 100644
>> --- a/src/usb-device-manager.c
>> +++ b/src/usb-device-manager.c
>> @@ -122,6 +122,7 @@ struct _SpiceUsbDeviceManagerPrivate {
>> GUdevClient *udev;
>> libusb_device **coldplug_list; /* Avoid needless reprobing during init */
>> #else
>> + gboolean redirecting;
>
> I'd prefer a comment here indicating that in the gudev case, the 'redirecting'
> status is handled by the GUdevClient.
Added.
>
>> libusb_hotplug_callback_handle hp_handle;
>> #endif
>> #ifdef G_OS_WIN32
>> @@ -208,10 +209,25 @@
>> _spice_usb_device_manager_connect_device_async(SpiceUsbDeviceManager *self,
>> GAsyncReadyCallback callback,
>> gpointer user_data);
>>
>> +static
>> +void _connect_device_async_cb(GObject *gobject,
>> + GAsyncResult *channel_res,
>> + gpointer user_data);
>> +
>> G_DEFINE_BOXED_TYPE(SpiceUsbDevice, spice_usb_device,
>> (GBoxedCopyFunc)spice_usb_device_ref,
>> (GBoxedFreeFunc)spice_usb_device_unref)
>>
>> +static void
>> +_set_redirecting(SpiceUsbDeviceManager *self, gboolean is_redirecting)
>> +{
>> +#ifdef USE_GUDEV
>> + g_object_set(self->priv->udev, "redirecting", is_redirecting, NULL);
>> +#else
>> + self->priv->redirecting = is_redirecting;
>> +#endif
>> +}
>> +
>> #else
>> G_DEFINE_BOXED_TYPE(SpiceUsbDevice, spice_usb_device, g_object_ref,
>> g_object_unref)
>> #endif
>> @@ -1135,7 +1151,7 @@ static void
>> spice_usb_device_manager_drv_install_cb(GObject *gobject,
>> SpiceUsbDevice *device;
>> UsbInstallCbInfo *cbinfo;
>> GCancellable *cancellable;
>> - GAsyncReadyCallback callback;
>> + gpointer data;
>>
>> g_return_if_fail(user_data != NULL);
>>
>> @@ -1144,8 +1160,7 @@ static void
>> spice_usb_device_manager_drv_install_cb(GObject *gobject,
>> device = cbinfo->device;
>> installer = cbinfo->installer;
>> cancellable = cbinfo->cancellable;
>> - callback = cbinfo->callback;
>> - user_data = cbinfo->user_data;
>> + data = cbinfo->user_data;
>>
>> g_free(cbinfo);
>>
>> @@ -1167,8 +1182,8 @@ static void
>> spice_usb_device_manager_drv_install_cb(GObject *gobject,
>> _spice_usb_device_manager_connect_device_async(self,
>> device,
>> cancellable,
>> - callback,
>> - user_data);
>> + _connect_device_async_cb,
>> + data);
>>
>> spice_usb_device_unref(device);
>> }
>> @@ -1496,6 +1511,8 @@
>> _spice_usb_device_manager_uninstall_driver_async(SpiceUsbDeviceManager *self,
>>
>> #endif
>>
>> +#ifdef USE_USBREDIR
>> +
>
> Moving the #ifdef outside of the function here seems like a fine change, but it
> doesn't appear to be necessary for this patch and makes the patch slightly
> harder to review. Can this change be split?
Moving these chunks to a separate patch breaks compilation without USE_USBREDIR.
>
>> static void
>> _spice_usb_device_manager_connect_device_async(SpiceUsbDeviceManager *self,
>> SpiceUsbDevice *device,
>> @@ -1513,7 +1530,6 @@
>> _spice_usb_device_manager_connect_device_async(SpiceUsbDeviceManager *self,
>> result = g_simple_async_result_new(G_OBJECT(self), callback, user_data,
>>
>> spice_usb_device_manager_connect_device_async);
>>
>> -#ifdef USE_USBREDIR
>> SpiceUsbDeviceManagerPrivate *priv = self->priv;
>> libusb_device *libdev;
>> guint i;
>> @@ -1559,18 +1575,17 @@
>> _spice_usb_device_manager_connect_device_async(SpiceUsbDeviceManager *self,
>> libusb_unref_device(libdev);
>> return;
>> }
>> -#endif
>>
>> g_simple_async_result_set_error(result,
>> SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
>> _("No free USB channel"));
>> -#ifdef USE_USBREDIR
>> done:
>> -#endif
>> g_simple_async_result_complete_in_idle(result);
>> g_object_unref(result);
>> }
>>
>> +#endif
>> +
>> /**
>> * spice_usb_device_manager_connect_device_async:
>> * @self: a #SpiceUsbDeviceManager.
>> @@ -1589,11 +1604,20 @@ void
>> spice_usb_device_manager_connect_device_async(SpiceUsbDeviceManager *self,
>> GAsyncReadyCallback callback,
>> gpointer user_data)
>> {
>> + g_return_if_fail(SPICE_IS_USB_DEVICE_MANAGER(self));
>>
>> -#if defined(USE_USBREDIR) && defined(G_OS_WIN32)
>> +#ifdef USE_USBREDIR
>> +
>> + GSimpleAsyncResult *result =
>> + g_simple_async_result_new(G_OBJECT(self), callback, user_data,
>> +
>> spice_usb_device_manager_connect_device_async);
>> +
>> + _set_redirecting(self, TRUE);
>> +
>> +#ifdef G_OS_WIN32
>> if (self->priv->use_usbclerk) {
>> _spice_usb_device_manager_install_driver_async(self, device,
>> cancellable,
>> - callback, user_data);
>> + callback, result);
>> return;
>> }
>> #endif
>> @@ -1601,10 +1625,13 @@ void
>> spice_usb_device_manager_connect_device_async(SpiceUsbDeviceManager *self,
>> _spice_usb_device_manager_connect_device_async(self,
>> device,
>> cancellable,
>> - callback,
>> - user_data);
>> + _connect_device_async_cb,
>> + result);
>> +
>> +#endif
>> }
>>
>> +
>> /**
>> * spice_usb_device_manager_connect_device_finish:
>> * @self: a #SpiceUsbDeviceManager.
>> @@ -1630,6 +1657,22 @@ gboolean
>> spice_usb_device_manager_connect_device_finish(
>> return TRUE;
>> }
>>
>> +#ifdef USE_USBREDIR
>> +static
>> +void _connect_device_async_cb(GObject *gobject,
>> + GAsyncResult *channel_res,
>> + gpointer user_data)
>> +{
>> + SpiceUsbDeviceManager *self = SPICE_USB_DEVICE_MANAGER(gobject);
>> + GSimpleAsyncResult *result = user_data;
>> +
>> + _set_redirecting(self, FALSE);
>> +
>> + g_simple_async_result_complete(result);
>> + g_object_unref(result);
>> +}
>> +#endif
>> +
>> /**
>> * spice_usb_device_manager_disconnect_device:
>> * @manager: the #SpiceUsbDeviceManager manager
>
> ACK with minor changes
>
> Acked-by: Jonathon Jongsma <jjongsma at redhat.com <mailto:jjongsma at redhat.com>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20160228/1015e37a/attachment-0001.html>
More information about the Spice-devel
mailing list