[Spice-devel] [PATCH spice-gtk 4/7] usbredir: make channel lifetime equal to session lifetime
Hans de Goede
hdegoede at redhat.com
Sat Feb 18 15:50:21 PST 2012
---
configure.ac | 2 +-
gtk/channel-usbredir-priv.h | 31 ++++++----
gtk/channel-usbredir.c | 139 +++++++++++++++++++++++++------------------
gtk/spice-channel-priv.h | 1 +
gtk/spice-channel.c | 9 +++
gtk/usb-device-manager.c | 15 +++--
6 files changed, 121 insertions(+), 76 deletions(-)
diff --git a/configure.ac b/configure.ac
index d2ac883..b37e864 100644
--- a/configure.ac
+++ b/configure.ac
@@ -344,7 +344,7 @@ if test "x$enable_usbredir" = "xno"; then
AM_CONDITIONAL(WITH_USBREDIR, false)
else
PKG_CHECK_MODULES([USBREDIR],
- [gudev-1.0 libusb-1.0 >= 1.0.9 libusbredirhost >= 0.3.3 libusbredirparser >= 0.3.3],
+ [gudev-1.0 libusb-1.0 >= 1.0.9 libusbredirhost >= 0.3.4 libusbredirparser >= 0.3.4],
[have_usbredir=yes],
[have_usbredir=no])
if test "x$have_usbredir" = "xno" && test "x$enable_usbredir" = "xyes"; then
diff --git a/gtk/channel-usbredir-priv.h b/gtk/channel-usbredir-priv.h
index 5d75e26..10dd479 100644
--- a/gtk/channel-usbredir-priv.h
+++ b/gtk/channel-usbredir-priv.h
@@ -26,17 +26,26 @@
G_BEGIN_DECLS
-void spice_usbredir_channel_connect_async(SpiceUsbredirChannel *channel,
- libusb_context *context,
- libusb_device *device,
- GCancellable *cancellable,
- GAsyncReadyCallback callback,
- gpointer user_data);
-gboolean spice_usbredir_channel_connect_finish(SpiceUsbredirChannel *channel,
- GAsyncResult *res,
- GError **err);
-
-void spice_usbredir_channel_disconnect(SpiceUsbredirChannel *channel);
+/* Note: this must be called before calling any other functions, and the
+ context should not be destroyed before the last device has been
+ disconnected */
+void spice_usbredir_channel_set_context(SpiceUsbredirChannel *channel,
+ libusb_context *context);
+
+/* Note the context must be set, and the channel must be brought up
+ (through spice_channel_connect()), before calling this. */
+void spice_usbredir_channel_connect_device_async(
+ SpiceUsbredirChannel *channel,
+ libusb_device *device,
+ GCancellable *cancellable,
+ GAsyncReadyCallback callback,
+ gpointer user_data);
+gboolean spice_usbredir_channel_connect_device_finish(
+ SpiceUsbredirChannel *channel,
+ GAsyncResult *res,
+ GError **err);
+
+void spice_usbredir_channel_disconnect_device(SpiceUsbredirChannel *channel);
libusb_device *spice_usbredir_channel_get_device(SpiceUsbredirChannel *channel);
diff --git a/gtk/channel-usbredir.c b/gtk/channel-usbredir.c
index ac23256..e22aa6c 100644
--- a/gtk/channel-usbredir.c
+++ b/gtk/channel-usbredir.c
@@ -59,13 +59,11 @@ enum SpiceUsbredirChannelState {
#if USE_POLKIT
STATE_WAITING_FOR_ACL_HELPER,
#endif
- STATE_CONNECTING,
STATE_CONNECTED,
STATE_DISCONNECTING,
};
struct _SpiceUsbredirChannelPrivate {
- libusb_context *context;
libusb_device *device;
struct usbredirhost *host;
/* To catch usbredirhost error messages and report them as a GError */
@@ -83,6 +81,7 @@ struct _SpiceUsbredirChannelPrivate {
static void spice_usbredir_handle_msg(SpiceChannel *channel, SpiceMsgIn *msg);
static void spice_usbredir_channel_up(SpiceChannel *channel);
static void spice_usbredir_channel_dispose(GObject *obj);
+static void spice_usbredir_channel_finalize(GObject *obj);
static void usbredir_handle_msg(SpiceChannel *channel, SpiceMsgIn *in);
static void usbredir_log(void *user_data, int level, const char *msg);
@@ -121,9 +120,10 @@ static void spice_usbredir_channel_class_init(SpiceUsbredirChannelClass *klass)
GObjectClass *gobject_class = G_OBJECT_CLASS(klass);
SpiceChannelClass *channel_class = SPICE_CHANNEL_CLASS(klass);
- gobject_class->dispose = spice_usbredir_channel_dispose;
- channel_class->handle_msg = spice_usbredir_handle_msg;
- channel_class->channel_up = spice_usbredir_channel_up;
+ gobject_class->dispose = spice_usbredir_channel_dispose;
+ gobject_class->finalize = spice_usbredir_channel_finalize;
+ channel_class->handle_msg = spice_usbredir_handle_msg;
+ channel_class->channel_up = spice_usbredir_channel_up;
channel_class->channel_reset = spice_usbredir_channel_reset;
g_type_class_add_private(klass, sizeof(SpiceUsbredirChannelPrivate));
@@ -135,7 +135,7 @@ static void spice_usbredir_channel_dispose(GObject *obj)
{
SpiceUsbredirChannel *channel = SPICE_USBREDIR_CHANNEL(obj);
- spice_usbredir_channel_disconnect(channel);
+ spice_usbredir_channel_disconnect_device(channel);
/* Chain up to the parent class */
if (G_OBJECT_CLASS(spice_usbredir_channel_parent_class)->dispose)
@@ -143,11 +143,11 @@ static void spice_usbredir_channel_dispose(GObject *obj)
}
/*
- * Note we don't have a finalize to unref our : device / context / acl_helper /
- * result references. The reason for this is that depending on our state they
- * are either:
+ * Note we don't unref our device / acl_helper / result references in our
+ * finalize. The reason for this is that depending on our state at dispose
+ * time they are either:
* 1) Already unreferenced
- * 2) Will be unreferenced by the disconnect call from dispose
+ * 2) Will be unreferenced by the disconnect_device call from dispose
* 3) Will be unreferenced by spice_usbredir_channel_open_acl_cb
*
* Now the last one may seem like an issue, since what will happen if
@@ -162,6 +162,17 @@ static void spice_usbredir_channel_dispose(GObject *obj)
* spice_usbredir_channel_open_acl_cb has run, all references we hold have
* been released even in the 3th scenario.
*/
+static void spice_usbredir_channel_finalize(GObject *obj)
+{
+ SpiceUsbredirChannel *channel = SPICE_USBREDIR_CHANNEL(obj);
+
+ if (channel->priv->host)
+ usbredirhost_close(channel->priv->host);
+
+ /* Chain up to the parent class */
+ if (G_OBJECT_CLASS(spice_usbredir_channel_parent_class)->finalize)
+ G_OBJECT_CLASS(spice_usbredir_channel_parent_class)->finalize(obj);
+}
static const spice_msg_handler usbredir_handlers[] = {
[ SPICE_MSG_SPICEVMC_DATA ] = usbredir_handle_msg,
@@ -170,12 +181,37 @@ static const spice_msg_handler usbredir_handlers[] = {
/* ------------------------------------------------------------------ */
/* private api */
+G_GNUC_INTERNAL
+void spice_usbredir_channel_set_context(SpiceUsbredirChannel *channel,
+ libusb_context *context)
+{
+ SpiceUsbredirChannelPrivate *priv = channel->priv;
+
+ g_return_if_fail(priv->host == NULL);
+
+ priv->host = usbredirhost_open_full(
+ context, NULL,
+ usbredir_log,
+ usbredir_read_callback,
+ usbredir_write_callback,
+ usbredir_write_flush_callback,
+ usbredir_alloc_lock,
+ usbredir_lock_lock,
+ usbredir_unlock_lock,
+ usbredir_free_lock,
+ channel, PACKAGE_STRING,
+ spice_util_get_debug() ? usbredirparser_debug : usbredirparser_warning,
+ usbredirhost_fl_write_cb_owns_buffer);
+ if (!priv->host)
+ g_error("Out of memory allocating usbredirhost");
+}
+
static gboolean spice_usbredir_channel_open_device(
SpiceUsbredirChannel *channel, GError **err)
{
SpiceUsbredirChannelPrivate *priv = channel->priv;
libusb_device_handle *handle = NULL;
- int rc;
+ int rc, status;
g_return_val_if_fail(priv->state == STATE_DISCONNECTED
#if USE_POLKIT
@@ -192,21 +228,9 @@ static gboolean spice_usbredir_channel_open_device(
}
priv->catch_error = err;
- priv->host = usbredirhost_open_full(
- priv->context,
- handle, usbredir_log,
- usbredir_read_callback,
- usbredir_write_callback,
- usbredir_write_flush_callback,
- usbredir_alloc_lock,
- usbredir_lock_lock,
- usbredir_unlock_lock,
- usbredir_free_lock,
- channel, PACKAGE_STRING,
- spice_util_get_debug() ? usbredirparser_debug : usbredirparser_warning,
- usbredirhost_fl_write_cb_owns_buffer);
+ status = usbredirhost_set_device(priv->host, handle);
priv->catch_error = NULL;
- if (!priv->host) {
+ if (status != usb_redir_success) {
g_return_val_if_fail(err == NULL || *err != NULL, FALSE);
return FALSE;
}
@@ -215,13 +239,11 @@ static gboolean spice_usbredir_channel_open_device(
spice_usb_device_manager_get(
spice_channel_get_session(SPICE_CHANNEL(channel)), NULL),
err)) {
- usbredirhost_close(priv->host);
- priv->host = NULL;
+ usbredirhost_set_device(priv->host, NULL);
return FALSE;
}
- spice_channel_connect(SPICE_CHANNEL(channel));
- priv->state = STATE_CONNECTING;
+ priv->state = STATE_CONNECTED;
return TRUE;
}
@@ -250,9 +272,8 @@ static void spice_usbredir_channel_open_acl_cb(
if (err) {
g_simple_async_result_take_error(priv->result, err);
libusb_unref_device(priv->device);
- priv->device = NULL;
- priv->context = NULL;
- priv->state = STATE_DISCONNECTED;
+ priv->device = NULL;
+ priv->state = STATE_DISCONNECTED;
}
spice_usb_acl_helper_close_acl(priv->acl_helper);
@@ -266,8 +287,8 @@ static void spice_usbredir_channel_open_acl_cb(
#endif
G_GNUC_INTERNAL
-void spice_usbredir_channel_connect_async(SpiceUsbredirChannel *channel,
- libusb_context *context,
+void spice_usbredir_channel_connect_device_async(
+ SpiceUsbredirChannel *channel,
libusb_device *device,
GCancellable *cancellable,
GAsyncReadyCallback callback,
@@ -277,13 +298,19 @@ void spice_usbredir_channel_connect_async(SpiceUsbredirChannel *channel,
GSimpleAsyncResult *result;
g_return_if_fail(SPICE_IS_USBREDIR_CHANNEL(channel));
- g_return_if_fail(context != NULL);
g_return_if_fail(device != NULL);
SPICE_DEBUG("connecting usb channel %p", channel);
result = g_simple_async_result_new(G_OBJECT(channel), callback, user_data,
- spice_usbredir_channel_connect_async);
+ spice_usbredir_channel_connect_device_async);
+
+ if (!priv->host) {
+ g_simple_async_result_set_error(result,
+ SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
+ "Error libusb context not set");
+ goto done;
+ }
if (priv->state != STATE_DISCONNECTED) {
g_simple_async_result_set_error(result,
@@ -292,11 +319,10 @@ void spice_usbredir_channel_connect_async(SpiceUsbredirChannel *channel,
goto done;
}
- priv->context = context;
- priv->device = libusb_ref_device(device);
+ priv->device = libusb_ref_device(device);
#if USE_POLKIT
priv->result = result;
- priv->state = STATE_WAITING_FOR_ACL_HELPER;
+ priv->state = STATE_WAITING_FOR_ACL_HELPER;
priv->acl_helper = spice_usb_acl_helper_new();
g_object_set(spice_channel_get_session(SPICE_CHANNEL(channel)),
"inhibit-keyboard-grab", TRUE, NULL);
@@ -312,8 +338,7 @@ void spice_usbredir_channel_connect_async(SpiceUsbredirChannel *channel,
if (!spice_usbredir_channel_open_device(channel, &err)) {
g_simple_async_result_take_error(result, err);
libusb_unref_device(priv->device);
- priv->device = NULL;
- priv->context = NULL;
+ priv->device = NULL;
}
#endif
@@ -323,14 +348,15 @@ done:
}
G_GNUC_INTERNAL
-gboolean spice_usbredir_channel_connect_finish(SpiceUsbredirChannel *channel,
+gboolean spice_usbredir_channel_connect_device_finish(
+ SpiceUsbredirChannel *channel,
GAsyncResult *res,
GError **err)
{
GSimpleAsyncResult *result = G_SIMPLE_ASYNC_RESULT(res);
g_return_val_if_fail(g_simple_async_result_is_valid(res, G_OBJECT(channel),
- spice_usbredir_channel_connect_async),
+ spice_usbredir_channel_connect_device_async),
FALSE);
if (g_simple_async_result_propagate_error(result, err))
@@ -340,11 +366,11 @@ gboolean spice_usbredir_channel_connect_finish(SpiceUsbredirChannel *channel,
}
G_GNUC_INTERNAL
-void spice_usbredir_channel_disconnect(SpiceUsbredirChannel *channel)
+void spice_usbredir_channel_disconnect_device(SpiceUsbredirChannel *channel)
{
SpiceUsbredirChannelPrivate *priv = channel->priv;
- SPICE_DEBUG("disconnecting usb channel %p", channel);
+ SPICE_DEBUG("disconnecting device from usb channel %p", channel);
switch (priv->state) {
case STATE_DISCONNECTED:
@@ -357,24 +383,21 @@ void spice_usbredir_channel_disconnect(SpiceUsbredirChannel *channel)
spice_usb_acl_helper_close_acl(priv->acl_helper);
break;
#endif
- case STATE_CONNECTING:
case STATE_CONNECTED:
- spice_channel_disconnect(SPICE_CHANNEL(channel), SPICE_CHANNEL_NONE);
/*
* This sets the usb event thread run condition to FALSE, therefor
- * it must be done before usbredirhost_close, as usbredirhost_close
- * will interrupt the libusb_handle_events call in the thread.
+ * it must be done before usbredirhost_set_device NULL, as
+ * usbredirhost_set_device NULL will interrupt the
+ * libusb_handle_events call in the thread.
*/
spice_usb_device_manager_stop_event_listening(
spice_usb_device_manager_get(
spice_channel_get_session(SPICE_CHANNEL(channel)), NULL));
- /* This also closes the libusb handle we passed to its _open */
- usbredirhost_close(priv->host);
- priv->host = NULL;
+ /* This also closes the libusb handle we passed from open_device */
+ usbredirhost_set_device(priv->host, NULL);
libusb_unref_device(priv->device);
- priv->device = NULL;
- priv->context = NULL;
- priv->state = STATE_DISCONNECTED;
+ priv->device = NULL;
+ priv->state = STATE_DISCONNECTED;
break;
}
}
@@ -392,7 +415,8 @@ static void usbredir_write_flush_callback(void *user_data)
SpiceUsbredirChannel *channel = SPICE_USBREDIR_CHANNEL(user_data);
SpiceUsbredirChannelPrivate *priv = channel->priv;
- if (priv->state != STATE_CONNECTED)
+ if (spice_channel_get_state(SPICE_CHANNEL(channel)) !=
+ SPICE_CHANNEL_STATE_READY)
return;
usbredirhost_write_guest_data(priv->host);
@@ -512,9 +536,6 @@ static void spice_usbredir_channel_up(SpiceChannel *c)
SpiceUsbredirChannel *channel = SPICE_USBREDIR_CHANNEL(c);
SpiceUsbredirChannelPrivate *priv = channel->priv;
- g_return_if_fail(priv->state == STATE_CONNECTING);
-
- priv->state = STATE_CONNECTED;
/* Flush any pending writes */
usbredirhost_write_guest_data(priv->host);
}
diff --git a/gtk/spice-channel-priv.h b/gtk/spice-channel-priv.h
index 5cd7ddb..a6a36ed 100644
--- a/gtk/spice-channel-priv.h
+++ b/gtk/spice-channel-priv.h
@@ -157,6 +157,7 @@ void spice_channel_up(SpiceChannel *channel);
void spice_channel_wakeup(SpiceChannel *channel, gboolean cancel);
SpiceSession* spice_channel_get_session(SpiceChannel *channel);
+enum spice_channel_state spice_channel_get_state(SpiceChannel *channel);
/* coroutine context */
typedef void (*handler_msg_in)(SpiceChannel *channel, SpiceMsgIn *msg, gpointer data);
diff --git a/gtk/spice-channel.c b/gtk/spice-channel.c
index feeeff2..972a3bb 100644
--- a/gtk/spice-channel.c
+++ b/gtk/spice-channel.c
@@ -2549,6 +2549,15 @@ SpiceSession* spice_channel_get_session(SpiceChannel *channel)
}
G_GNUC_INTERNAL
+enum spice_channel_state spice_channel_get_state(SpiceChannel *channel)
+{
+ g_return_val_if_fail(SPICE_IS_CHANNEL(channel),
+ SPICE_CHANNEL_STATE_UNCONNECTED);
+
+ return channel->priv->state;
+}
+
+G_GNUC_INTERNAL
void spice_channel_swap(SpiceChannel *channel, SpiceChannel *swap)
{
SpiceChannelPrivate *c = SPICE_CHANNEL_GET_PRIVATE(channel);
diff --git a/gtk/usb-device-manager.c b/gtk/usb-device-manager.c
index 495a15b..b6dfa20 100644
--- a/gtk/usb-device-manager.c
+++ b/gtk/usb-device-manager.c
@@ -466,8 +466,14 @@ static void channel_new(SpiceSession *session, SpiceChannel *channel,
{
SpiceUsbDeviceManager *self = user_data;
- if (SPICE_IS_USBREDIR_CHANNEL(channel))
+ if (SPICE_IS_USBREDIR_CHANNEL(channel)) {
+#ifdef USE_USBREDIR
+ spice_usbredir_channel_set_context(SPICE_USBREDIR_CHANNEL(channel),
+ self->priv->context);
+ spice_channel_connect(channel);
+#endif
g_ptr_array_add(self->priv->channels, channel);
+ }
}
static void channel_destroy(SpiceSession *session, SpiceChannel *channel,
@@ -612,7 +618,7 @@ static void spice_usb_device_manager_channel_connect_cb(
GSimpleAsyncResult *result = G_SIMPLE_ASYNC_RESULT(user_data);
GError *err = NULL;
- spice_usbredir_channel_connect_finish(channel, channel_res, &err);
+ spice_usbredir_channel_connect_device_finish(channel, channel_res, &err);
if (err) {
g_simple_async_result_take_error(result, err);
}
@@ -815,8 +821,7 @@ void spice_usb_device_manager_connect_device_async(SpiceUsbDeviceManager *self,
if (spice_usbredir_channel_get_device(channel))
continue; /* Skip already used channels */
- spice_usbredir_channel_connect_async(channel,
- priv->context,
+ spice_usbredir_channel_connect_device_async(channel,
(libusb_device *)device,
cancellable,
spice_usb_device_manager_channel_connect_cb,
@@ -870,7 +875,7 @@ void spice_usb_device_manager_disconnect_device(SpiceUsbDeviceManager *self,
channel = spice_usb_device_manager_get_channel_for_dev(self, device);
if (channel)
- spice_usbredir_channel_disconnect(channel);
+ spice_usbredir_channel_disconnect_device(channel);
#endif
}
--
1.7.7.5
More information about the Spice-devel
mailing list