[Spice-commits] 4 commits - src/channel-usbredir.c src/spice-channel.c src/usb-device-manager.c
GitLab Mirror
gitlab-mirror at kemper.freedesktop.org
Wed Sep 26 15:31:56 UTC 2018
src/channel-usbredir.c | 34 ++++++++++++++++++++++------------
src/spice-channel.c | 12 ++++++++----
src/usb-device-manager.c | 32 ++++++++++++++++++++++++++++++++
3 files changed, 62 insertions(+), 16 deletions(-)
New commits:
commit 3afee7ae8db387e7f4339f2a4afaa63babf358b7
Author: Victor Toso <me at victortoso.com>
Date: Wed Sep 5 15:43:28 2018 +0200
channel-usbredir: Chain-up with parent's channel-reset
Otherwise spice-channel is left with a broken state.
This code moves parent's call to channel_reset() into
_channel_reset_finish() - Note that spice-channel's channel_reset()
can be called from GMainContext.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1625550
Signed-off-by: Victor Toso <victortoso at redhat.com>
Acked-by: Frediano Ziglio <fziglio at redhat.com>
diff --git a/src/channel-usbredir.c b/src/channel-usbredir.c
index 2c4a386..182edc4 100644
--- a/src/channel-usbredir.c
+++ b/src/channel-usbredir.c
@@ -122,7 +122,7 @@ static void spice_usbredir_channel_init(SpiceUsbredirChannel *channel)
#ifdef USE_USBREDIR
-static void _channel_reset_finish(SpiceUsbredirChannel *channel)
+static void _channel_reset_finish(SpiceUsbredirChannel *channel, gboolean migrating)
{
SpiceUsbredirChannelPrivate *priv = channel->priv;
@@ -135,6 +135,8 @@ static void _channel_reset_finish(SpiceUsbredirChannel *channel)
spice_usbredir_channel_set_context(channel, priv->context);
spice_usbredir_channel_unlock(channel);
+
+ SPICE_CHANNEL_CLASS(spice_usbredir_channel_parent_class)->channel_reset(SPICE_CHANNEL(channel), migrating);
}
static void _channel_reset_cb(GObject *gobject,
@@ -146,9 +148,7 @@ static void _channel_reset_cb(GObject *gobject,
gboolean migrating = GPOINTER_TO_UINT(user_data);
GError *err = NULL;
- _channel_reset_finish(channel);
-
- SPICE_CHANNEL_CLASS(spice_usbredir_channel_parent_class)->channel_reset(spice_channel, migrating);
+ _channel_reset_finish(channel, migrating);
spice_usbredir_channel_disconnect_device_finish(channel, result, &err);
}
@@ -177,8 +177,7 @@ static void spice_usbredir_channel_reset(SpiceChannel *c, gboolean migrating)
return;
}
- /* FIXME: This does not chain-up with parent's channel-reset, which is a must */
- _channel_reset_finish(channel);
+ _channel_reset_finish(channel, migrating);
}
#endif
commit f1aee52b098440273f749042bde23d61b73e4da5
Author: Victor Toso <me at victortoso.com>
Date: Wed Sep 5 15:39:56 2018 +0200
channel-usbredir: Add FIXMEs on channel-reset issues
This should not change code behavior, only add some comments.
This is a preparatory patch for bug below, introduced with:
commit 9fbf679453d8dbfe797a738cb536136599d7adab
Author: Kirill Moizik <kmoizik at redhat.com>
Date: Tue Mar 8 16:05:57 2016 +0200
usbredir: Disconnect USB device asynchronously
Related: https://bugzilla.redhat.com/show_bug.cgi?id=1625550
Signed-off-by: Victor Toso <victortoso at redhat.com>
Acked-by: Frediano Ziglio <fziglio at redhat.com>
diff --git a/src/channel-usbredir.c b/src/channel-usbredir.c
index 1d9c380..2c4a386 100644
--- a/src/channel-usbredir.c
+++ b/src/channel-usbredir.c
@@ -158,16 +158,27 @@ static void spice_usbredir_channel_reset(SpiceChannel *c, gboolean migrating)
SpiceUsbredirChannel *channel = SPICE_USBREDIR_CHANNEL(c);
SpiceUsbredirChannelPrivate *priv = channel->priv;
- if (priv->host) {
- if (priv->state == STATE_CONNECTED) {
- spice_usbredir_channel_disconnect_device_async(channel, NULL,
- _channel_reset_cb, GUINT_TO_POINTER(migrating));
- } else {
- _channel_reset_finish(channel);
- }
- } else {
+ /* Host isn't running, just reset */
+ if (!priv->host) {
SPICE_CHANNEL_CLASS(spice_usbredir_channel_parent_class)->channel_reset(c, migrating);
+ return;
}
+
+ /* Host is running, so we might need to disconnect the usb devices async.
+ * This should not block channel_reset() otherwise we might run in reconnection
+ * problems such as https://bugzilla.redhat.com/show_bug.cgi?id=1625550
+ * No operation from here on should rely on SpiceChannel as its coroutine
+ * might be terminated. */
+
+ if (priv->state == STATE_CONNECTED) {
+ /* FIXME: We should chain-up parent's channel-reset here */
+ spice_usbredir_channel_disconnect_device_async(channel, NULL,
+ _channel_reset_cb, GUINT_TO_POINTER(migrating));
+ return;
+ }
+
+ /* FIXME: This does not chain-up with parent's channel-reset, which is a must */
+ _channel_reset_finish(channel);
}
#endif
commit bd195d3f76f115b5ef5d1ad0e83e9ccea5a04d18
Author: Victor Toso <me at victortoso.com>
Date: Thu Sep 6 15:37:46 2018 +0200
usb-device-manager: Handle connectionless channel
As we are not able to redirect anything in case that usbredir channel
is not connected.
Related: https://bugzilla.redhat.com/show_bug.cgi?id=1625550
Signed-off-by: Victor Toso <victortoso at gnome.org>
Acked-by: Frediano Ziglio <fziglio at redhat.com>
diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c
index 50fb491..45adf5a 100644
--- a/src/usb-device-manager.c
+++ b/src/usb-device-manager.c
@@ -158,6 +158,8 @@ static void channel_new(SpiceSession *session, SpiceChannel *channel,
gpointer user_data);
static void channel_destroy(SpiceSession *session, SpiceChannel *channel,
gpointer user_data);
+static void channel_event(SpiceChannel *channel, SpiceChannelEvent event,
+ gpointer user_data);
#ifdef USE_GUDEV
static void spice_usb_device_manager_uevent_cb(GUdevClient *client,
const gchar *action,
@@ -849,6 +851,8 @@ static void channel_new(SpiceSession *session, SpiceChannel *channel,
spice_channel_connect(channel);
g_ptr_array_add(self->priv->channels, channel);
+ g_signal_connect(channel, "channel-event", G_CALLBACK(channel_event), self);
+
spice_usb_device_manager_check_redir_on_connect(self, channel);
/*
@@ -871,6 +875,34 @@ static void channel_destroy(SpiceSession *session, SpiceChannel *channel,
g_ptr_array_remove(self->priv->channels, channel);
}
+static void channel_event(SpiceChannel *channel, SpiceChannelEvent event,
+ gpointer user_data)
+
+{
+ SpiceUsbDeviceManager *self = user_data;
+
+ switch (event) {
+ case SPICE_CHANNEL_NONE:
+ case SPICE_CHANNEL_OPENED:
+ return;
+
+ case SPICE_CHANNEL_SWITCHING:
+ case SPICE_CHANNEL_CLOSED:
+ case SPICE_CHANNEL_ERROR_CONNECT:
+ case SPICE_CHANNEL_ERROR_TLS:
+ case SPICE_CHANNEL_ERROR_LINK:
+ case SPICE_CHANNEL_ERROR_AUTH:
+ case SPICE_CHANNEL_ERROR_IO:
+ g_signal_handlers_disconnect_by_func(channel, channel_event, user_data);
+ g_ptr_array_remove(self->priv->channels, channel);
+ return;
+ default:
+ g_warning("Unhandled SpiceChannelEvent %d, disconnecting usbredir %p", event, channel);
+ g_signal_handlers_disconnect_by_func(channel, channel_event, user_data);
+ g_ptr_array_remove(self->priv->channels, channel);
+ }
+}
+
static void spice_usb_device_manager_auto_connect_cb(GObject *gobject,
GAsyncResult *res,
gpointer user_data)
commit 879926622d764a02b43a9147fb2a976765385115
Author: Victor Toso <me at victortoso.com>
Date: Thu Sep 6 14:04:53 2018 +0200
spice-channel: Properly error out if reconnect fails
The channel_connect() function could fail leading to a spice-channel
existing as zombie (its coroutine return soon after).
Check if channel_connect() fails and give a proper error signal to
user when that happens.
Related: https://bugzilla.redhat.com/show_bug.cgi?id=1625550
Signed-off-by: Victor Toso <victortoso at redhat.com>
Acked-by: Frediano Ziglio <fziglio at redhat.com>
diff --git a/src/spice-channel.c b/src/spice-channel.c
index 0a5437c..7de1298 100644
--- a/src/spice-channel.c
+++ b/src/spice-channel.c
@@ -2675,11 +2675,15 @@ cleanup:
if (c->state == SPICE_CHANNEL_STATE_RECONNECTING ||
c->state == SPICE_CHANNEL_STATE_SWITCHING) {
g_warn_if_fail(c->event == SPICE_CHANNEL_NONE);
- channel_connect(channel, c->tls);
- g_object_unref(channel);
- } else
- g_idle_add(spice_channel_delayed_unref, data);
+ if (channel_connect(channel, c->tls)) {
+ g_object_unref(channel);
+ return NULL;
+ }
+
+ c->event = SPICE_CHANNEL_ERROR_CONNECT;
+ }
+ g_idle_add(spice_channel_delayed_unref, channel);
/* Co-routine exits now - the SpiceChannel object may no longer exist,
so don't do anything else now unless you like SEGVs */
return NULL;
More information about the Spice-commits
mailing list