<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""> <div><blockquote type="cite" class=""><div class="">On 10 Feb 2016, at 23:48 PM, Jonathon Jongsma <<a href="mailto:jjongsma@redhat.com" class="">jjongsma@redhat.com</a>> wrote:</div> <div class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">On Thu, 2015-10-29 at 17:27 +0200, Dmitry Fleytman wrote:<br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">From: Kirill Moizik <<a href="mailto:kmoizik@redhat.com" class="">kmoizik@redhat.com</a>> 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 <<a href="mailto:kmoizik@redhat.com" class="">kmoizik@redhat.com</a>> Signed-off-by: Dmitry Fleytman <<a href="mailto:dfleytma@redhat.com" class="">dfleytma@redhat.com</a>> --- 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; </blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">I'd prefer a comment here indicating that in the gudev case, the 'redirecting'<br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">status is handled by the GUdevClient.<br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></blockquote><div> </div><div>Added.</div> <blockquote type="cite" class=""><div class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""> 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 + </blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Moving the #ifdef outside of the function here seems like a fine change, but it<br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">doesn't appear to be necessary for this patch and makes the patch slightly<br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">harder to review. Can this change be split?<br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></blockquote><div> </div><div> </div><div>Moving these chunks to a separate patch breaks compilation without USE_USBREDIR.</div> <blockquote type="cite" class=""><div class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">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 </blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">ACK with minor changes<br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Acked-by: Jonathon Jongsma <<a href="mailto:jjongsma@redhat.com" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">jjongsma@redhat.com</a><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">></div></blockquote></div> </body></html>