<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Aug 11, 2015, at 15:14 PM, Christophe Fergeau <<a href="mailto:cfergeau@redhat.com" class="">cfergeau@redhat.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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 Mon, Aug 03, 2015 at 04:10:45PM +0300, Kirill Moizik wrote:</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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; line-height: 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>><br class=""><br class="">---<br class="">src/channel-usbredir.c | 46 +++++++++++++++++++++++---------<br class="">src/usb-device-manager.c | 69 +++++++++++++++++++++++++++++++++++++++---------<br class="">2 files changed, 90 insertions(+), 25 deletions(-)<br class=""><br class="">diff --git a/src/channel-usbredir.c b/src/channel-usbredir.c<br class="">index 69cb565..7394b81 100644<br class="">--- a/src/channel-usbredir.c<br class="">+++ b/src/channel-usbredir.c<br class="">@@ -315,6 +315,27 @@ static void spice_usbredir_channel_open_acl_cb(<br class="">}<br class="">#endif<br class=""><br class="">+#ifndef USE_POLKIT<br class="">+static void<br class="">+spice_usbredir_channel_open_device_async(GSimpleAsyncResult *simple,<br class="">+ GObject *object,<br class="">+ GCancellable *cancellable)<br class=""></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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; line-height: 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="">This should be named _open_device_async_cb()</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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><br class=""></div><div><br class=""></div><div>Fixed.</div><br class=""><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; line-height: 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; line-height: 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 class="">+ GError *err = NULL;<br class="">+ SpiceUsbredirChannel *channel= (SpiceUsbredirChannel *)object;<br class=""></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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; line-height: 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="">SPICE_USBREDIR_CHANNEL(object) would be better than the direct cast.</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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><br class=""></div><div><br class=""></div><div>Sure. Fixed.</div><br class=""><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; line-height: 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; line-height: 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="">+ SpiceUsbredirChannelPrivate *priv = channel->priv;<br class="">+ g_mutex_lock(priv->flows_mutex);<br class="">+ if (!spice_usbredir_channel_open_device(channel, &err)) {<br class="">+ g_simple_async_result_take_error(simple, err);<br class="">+ libusb_unref_device(priv->device);<br class="">+ priv->device = NULL;<br class="">+ g_boxed_free(spice_usb_device_get_type(), priv->spice_device);<br class="">+ priv->spice_device = NULL;<br class="">+ }<br class="">+ g_mutex_unlock(priv->flows_mutex);<br class="">+}<br class="">+#endif<br class="">+<br class="">G_GNUC_INTERNAL<br class="">void spice_usbredir_channel_connect_device_async(<br class=""> SpiceUsbredirChannel *channel,<br class="">@@ -324,15 +345,16 @@ void spice_usbredir_channel_connect_device_async(<br class=""> GAsyncReadyCallback callback,<br class=""> gpointer user_data)<br class="">{<br class="">- SpiceUsbredirChannelPrivate *priv = channel->priv;<br class="">- GSimpleAsyncResult *result;<br class="">-#if ! USE_POLKIT<br class="">- GError *err = NULL;<br class="">-#endif<br class=""><br class=""> g_return_if_fail(SPICE_IS_USBREDIR_CHANNEL(channel));<br class=""> g_return_if_fail(device != NULL);<br class=""><br class="">+ SpiceUsbredirChannelPrivate *priv = channel->priv;<br class="">+ GSimpleAsyncResult *result;<br class="">+#ifndef USE_POLKIT<br class="">+ g_object_ref(channel);<br class="">+#endif<br class=""></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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; line-height: 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 don't think you need this _ref() here,</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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; line-height: 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="">g_simple_async_result_new(channel, ...) will take a reference for you</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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; line-height: 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="">for as long as the GSimpleAsyncResult object is alive.</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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><br class=""></div><div><br class=""></div><div>You are right, removed here and in a similar code on disconnection.</div><div><br class=""></div><br class=""><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; line-height: 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; line-height: 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 class=""> CHANNEL_DEBUG(channel, "connecting usb channel %p", channel);<br class=""><br class=""> result = g_simple_async_result_new(G_OBJECT(channel), callback, user_data,<br class="">@@ -369,18 +391,18 @@ void spice_usbredir_channel_connect_device_async(<br class=""> channel);<br class=""> return;<br class="">#else<br class="">- if (!spice_usbredir_channel_open_device(channel, &err)) {<br class="">- g_simple_async_result_take_error(result, err);<br class="">- libusb_unref_device(priv->device);<br class="">- priv->device = NULL;<br class="">- g_boxed_free(spice_usb_device_get_type(), priv->spice_device);<br class="">- priv->spice_device = NULL;<br class="">- }<br class="">+ g_simple_async_result_run_in_thread(result,<br class="">+ spice_usbredir_channel_open_device_async,<br class="">+ G_PRIORITY_DEFAULT,<br class="">+ cancellable);<br class="">+ g_object_unref(result);<br class="">+ return;<br class="">#endif<br class=""><br class="">done:<br class=""> g_simple_async_result_complete_in_idle(result);<br class=""> g_object_unref(result);<br class="">+ g_object_unref(channel);<br class="">}<br class=""><br class="">G_GNUC_INTERNAL<br class=""></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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; line-height: 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="">All the changes below in usb-device-manager.c seems to be more or less</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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; line-height: 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="">unrelated to the changes above, and aimed at setting 'redirecting' to</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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; line-height: 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="">TRUE/FALSE when redirection starts/ends, I suspect it would be clearer</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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; line-height: 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="">to do this in a separate commit.</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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><br class=""></div><div><br class=""></div><div>Agree, moving to a separate commit.</div><br class=""><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; line-height: 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; line-height: 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="">diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c<br class="">index e58450d..65c6568 100644<br class="">--- a/src/usb-device-manager.c<br class="">+++ b/src/usb-device-manager.c<br class="">@@ -207,6 +207,11 @@ _spice_usb_device_manager_connect_device_async(SpiceUsbDeviceManager *self,<br class=""> GAsyncReadyCallback callback,<br class=""> gpointer user_data);<br class=""><br class="">+static<br class="">+void spice_usb_device_manager_connect_device_async_cb(GObject *gobject,<br class="">+ GAsyncResult *channel_res,<br class="">+ gpointer user_data);<br class="">+<br class="">G_DEFINE_BOXED_TYPE(SpiceUsbDevice, spice_usb_device,<br class=""> (GBoxedCopyFunc)spice_usb_device_ref,<br class=""> (GBoxedFreeFunc)spice_usb_device_unref)<br class="">@@ -1128,8 +1133,16 @@ static void spice_usb_device_manager_channel_connect_cb(<br class=""> }<br class=""> g_simple_async_result_complete(result);<br class=""> g_object_unref(result);<br class="">+ g_object_unref(channel);<br class="">}<br class=""><br class="">+typedef struct _connect_cb_data<br class="">+{<br class="">+ SpiceUsbDeviceManager *self;<br class="">+ GAsyncReadyCallback callback;<br class="">+ gpointer user_data;<br class="">+} connect_cb_data;<br class="">+<br class="">#ifdef G_OS_WIN32<br class=""><br class="">typedef struct _UsbInstallCbInfo {<br class=""></blockquote><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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="">[snip]</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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; line-height: 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="">@@ -1603,9 +1625,10 @@ void spice_usb_device_manager_connect_device_async(SpiceUsbDeviceManager *self,<br class=""> _spice_usb_device_manager_connect_device_async(self,<br class=""> device,<br class=""> cancellable,<br class="">- callback,<br class="">- user_data);<br class="">+ spice_usb_device_manager_connect_device_async_cb,<br class="">+ data);<br class=""> }<br class="">+#endif<br class="">}<br class=""><br class="">gboolean spice_usb_device_manager_connect_device_finish(<br class="">@@ -1623,6 +1646,26 @@ gboolean spice_usb_device_manager_connect_device_finish(<br class=""> return TRUE;<br class="">}<br class=""><br class="">+#ifdef USE_USBREDIR<br class="">+static<br class="">+void spice_usb_device_manager_connect_device_async_cb(GObject *gobject,<br class="">+ GAsyncResult *channel_res,<br class="">+ gpointer user_data)<br class="">+{<br class="">+ connect_cb_data *data = user_data;<br class="">+ GSimpleAsyncResult *result = g_simple_async_result_new(G_OBJECT(data->self), data->callback,<br class="">+ data->user_data,<br class="">+ spice_usb_device_manager_disconnect_device_async);<br class="">+ g_object_set(data->self,"redirecting", FALSE, NULL);<br class="">+#ifndef USE_LIBUSB_HOTPLUG<br class="">+ spice_g_udev_set_redirecting(FALSE);<br class="">+#endif<br class="">+ g_simple_async_result_complete(result);<br class="">+ g_object_unref(result);<br class="">+ g_free(data);<br class="">+}<br class="">+#endif<br class="">+<br class=""></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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; line-height: 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="">Can't this be done in spice_usb_device_manager_channel_connect_cb()</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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; line-height: 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="">rather than introducing an extra async_cb step?</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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><br class=""></div><div>That would be better, but unfortunately spice_usb_device_manager_channel_connect_cb</div><div>doesn’t have access to device manager/udev objects and we’d like to preserve that type of encapsulation.</div><br class=""><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; line-height: 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; line-height: 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="">Christophe</span></div></blockquote></div><br class=""></body></html>