On Mon, Aug 03, 2015 at 04:10:45PM +0300, Kirill Moizik wrote:From: Kirill Moizik <kmoizik@xxxxxxxxxx>
---
src/channel-usbredir.c | 46 +++++++++++++++++++++++---------
src/usb-device-manager.c | 69 +++++++++++++++++++++++++++++++++++++++---------
2 files changed, 90 insertions(+), 25 deletions(-)
diff --git a/src/channel-usbredir.c b/src/channel-usbredir.c
index 69cb565..7394b81 100644
--- a/src/channel-usbredir.c
+++ b/src/channel-usbredir.c
@@ -315,6 +315,27 @@ static void spice_usbredir_channel_open_acl_cb(
}
#endif
+#ifndef USE_POLKIT
+static void
+spice_usbredir_channel_open_device_async(GSimpleAsyncResult *simple,
+ GObject *object,
+ GCancellable *cancellable)
This should be named _open_device_async_cb()
Fixed. +{
+ GError *err = NULL;
+ SpiceUsbredirChannel *channel= (SpiceUsbredirChannel *)object;
SPICE_USBREDIR_CHANNEL(object) would be better than the direct cast.
Sure. Fixed. + SpiceUsbredirChannelPrivate *priv = channel->priv;
+ g_mutex_lock(priv->flows_mutex);
+ if (!spice_usbredir_channel_open_device(channel, &err)) {
+ g_simple_async_result_take_error(simple, err);
+ libusb_unref_device(priv->device);
+ priv->device = NULL;
+ g_boxed_free(spice_usb_device_get_type(), priv->spice_device);
+ priv->spice_device = NULL;
+ }
+ g_mutex_unlock(priv->flows_mutex);
+}
+#endif
+
G_GNUC_INTERNAL
void spice_usbredir_channel_connect_device_async(
SpiceUsbredirChannel *channel,
@@ -324,15 +345,16 @@ void spice_usbredir_channel_connect_device_async(
GAsyncReadyCallback callback,
gpointer user_data)
{
- SpiceUsbredirChannelPrivate *priv = channel->priv;
- GSimpleAsyncResult *result;
-#if ! USE_POLKIT
- GError *err = NULL;
-#endif
g_return_if_fail(SPICE_IS_USBREDIR_CHANNEL(channel));
g_return_if_fail(device != NULL);
+ SpiceUsbredirChannelPrivate *priv = channel->priv;
+ GSimpleAsyncResult *result;
+#ifndef USE_POLKIT
+ g_object_ref(channel);
+#endif
I don't think you need this _ref() here,g_simple_async_result_new(channel, ...) will take a reference for youfor as long as the GSimpleAsyncResult object is alive.
You are right, removed here and in a similar code on disconnection.
+
CHANNEL_DEBUG(channel, "connecting usb channel %p", channel);
result = g_simple_async_result_new(G_OBJECT(channel), callback, user_data,
@@ -369,18 +391,18 @@ void spice_usbredir_channel_connect_device_async(
channel);
return;
#else
- if (!spice_usbredir_channel_open_device(channel, &err)) {
- g_simple_async_result_take_error(result, err);
- libusb_unref_device(priv->device);
- priv->device = NULL;
- g_boxed_free(spice_usb_device_get_type(), priv->spice_device);
- priv->spice_device = NULL;
- }
+ g_simple_async_result_run_in_thread(result,
+ spice_usbredir_channel_open_device_async,
+ G_PRIORITY_DEFAULT,
+ cancellable);
+ g_object_unref(result);
+ return;
#endif
done:
g_simple_async_result_complete_in_idle(result);
g_object_unref(result);
+ g_object_unref(channel);
}
G_GNUC_INTERNAL
All the changes below in usb-device-manager.c seems to be more or lessunrelated to the changes above, and aimed at setting 'redirecting' toTRUE/FALSE when redirection starts/ends, I suspect it would be clearerto do this in a separate commit.
Agree, moving to a separate commit. diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c
index e58450d..65c6568 100644
--- a/src/usb-device-manager.c
+++ b/src/usb-device-manager.c
@@ -207,6 +207,11 @@ _spice_usb_device_manager_connect_device_async(SpiceUsbDeviceManager *self,
GAsyncReadyCallback callback,
gpointer user_data);
+static
+void spice_usb_device_manager_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)
@@ -1128,8 +1133,16 @@ static void spice_usb_device_manager_channel_connect_cb(
}
g_simple_async_result_complete(result);
g_object_unref(result);
+ g_object_unref(channel);
}
+typedef struct _connect_cb_data
+{
+ SpiceUsbDeviceManager *self;
+ GAsyncReadyCallback callback;
+ gpointer user_data;
+} connect_cb_data;
+
#ifdef G_OS_WIN32
typedef struct _UsbInstallCbInfo {
[snip]@@ -1603,9 +1625,10 @@ void spice_usb_device_manager_connect_device_async(SpiceUsbDeviceManager *self,
_spice_usb_device_manager_connect_device_async(self,
device,
cancellable,
- callback,
- user_data);
+ spice_usb_device_manager_connect_device_async_cb,
+ data);
}
+#endif
}
gboolean spice_usb_device_manager_connect_device_finish(
@@ -1623,6 +1646,26 @@ gboolean spice_usb_device_manager_connect_device_finish(
return TRUE;
}
+#ifdef USE_USBREDIR
+static
+void spice_usb_device_manager_connect_device_async_cb(GObject *gobject,
+ GAsyncResult *channel_res,
+ gpointer user_data)
+{
+ connect_cb_data *data = user_data;
+ GSimpleAsyncResult *result = g_simple_async_result_new(G_OBJECT(data->self), data->callback,
+ data->user_data,
+ spice_usb_device_manager_disconnect_device_async);
+ g_object_set(data->self,"redirecting", FALSE, NULL);
+#ifndef USE_LIBUSB_HOTPLUG
+ spice_g_udev_set_redirecting(FALSE);
+#endif
+ g_simple_async_result_complete(result);
+ g_object_unref(result);
+ g_free(data);
+}
+#endif
+
Can't this be done in spice_usb_device_manager_channel_connect_cb()rather than introducing an extra async_cb step?
That would be better, but unfortunately spice_usb_device_manager_channel_connect_cb doesn’t have access to device manager/udev objects and we’d like to preserve that type of encapsulation.
Christophe
|