|
|
1a1a75 |
From e3f24cdc345f888730ea679e482d6217fb3d2b2a Mon Sep 17 00:00:00 2001
|
|
|
1a1a75 |
From: Victor Toso <me@victortoso.com>
|
|
|
1a1a75 |
Date: Wed, 4 Sep 2019 12:37:34 +0200
|
|
|
1a1a75 |
Subject: [PATCH] Revert "channel: Abort migration in delayed unref"
|
|
|
1a1a75 |
|
|
|
1a1a75 |
This reverts commit 109e575 "channel: Abort migration in delayed
|
|
|
1a1a75 |
unref" in 2016-05-02 by Pavel Grunt <pgrunt@redhat.com>
|
|
|
1a1a75 |
|
|
|
1a1a75 |
This commit is being reverted because it calls
|
|
|
1a1a75 |
spice_session_abort_migration() on the SpiceSession for the target
|
|
|
1a1a75 |
host while the function should receive the SpiceSession for current
|
|
|
1a1a75 |
host.
|
|
|
1a1a75 |
|
|
|
1a1a75 |
I can actually reproduce a bug where the password expires for the
|
|
|
1a1a75 |
current Spice session and the migration is going to fail because of
|
|
|
1a1a75 |
that. Before reverting this commit, remote-viewer would hang and the
|
|
|
1a1a75 |
following logs would occur:
|
|
|
1a1a75 |
|
|
|
1a1a75 |
- Migration starts on channel-main
|
|
|
1a1a75 |
> ../src/channel-main.c:2174 migrate_channel_connect 1:0
|
|
|
1a1a75 |
|
|
|
1a1a75 |
- Using TLS
|
|
|
1a1a75 |
> ../src/spice-channel.c:1934 main-1:0: switching to tls
|
|
|
1a1a75 |
|
|
|
1a1a75 |
- Following logs are related to failure to connect due Authentication
|
|
|
1a1a75 |
> ../src/spice-channel.c:2000 main-1:0: use mini header: 1
|
|
|
1a1a75 |
> ../src/spice-channel.c:1274 main-1:0: link result: reply 7
|
|
|
1a1a75 |
> ../src/spice-channel.c:2680 main-1:0: Coroutine exit main-1:0
|
|
|
1a1a75 |
> ../src/spice-channel.c:2871 main-1:0: reset
|
|
|
1a1a75 |
|
|
|
1a1a75 |
Remote-viewer would hang because we are in a state of migration and
|
|
|
1a1a75 |
SpiceMainChannel does not know that it failed because
|
|
|
1a1a75 |
spice_session_abort_migration() is being called on SpiceSession of
|
|
|
1a1a75 |
target host and no SpiceChannel::channel-event is being emitted.
|
|
|
1a1a75 |
|
|
|
1a1a75 |
Reverting this patch does abort migration thanks to
|
|
|
1a1a75 |
SpiceChannel::channel-event being emitted and caught by
|
|
|
1a1a75 |
SpiceMainChannel at migrate_channel_event_cb() with the log:
|
|
|
1a1a75 |
|
|
|
1a1a75 |
> ../src/channel-main.c:2247 main-1:0: error or unhandled channel event during migration: 23
|
|
|
1a1a75 |
> ../src/channel-main.c:2373 main-1:0: migrate failed: some channels failed to connect
|
|
|
1a1a75 |
|
|
|
1a1a75 |
Note that the reverted patch mentions the fix of bug [0] which is also
|
|
|
1a1a75 |
a virt-viewer with a hanging state. Looking back to the logs, the
|
|
|
1a1a75 |
interesting part around issues around usbredir, that is:
|
|
|
1a1a75 |
|
|
|
1a1a75 |
> GSpice-DEBUG: channel-main.c:2236 migration: channel opened chan:0x29ddce0, left 6
|
|
|
1a1a75 |
> GSpice-DEBUG: spice-channel.c:916 usbredir-9:0: Read error Error receiving data: Connection reset by peer
|
|
|
1a1a75 |
> GSpice-DEBUG: spice-channel.c:1210 usbredir-9:0: error, switching to protocol 1 (spice 0.4)
|
|
|
1a1a75 |
> GSpice-DEBUG: spice-channel.c:2433 usbredir-9:0: Coroutine exit usbredir-9:0
|
|
|
1a1a75 |
> GSpice-DEBUG: spice-channel.c:2455 usbredir-9:0: Open coroutine starting 0x29e10d0
|
|
|
1a1a75 |
> GSpice-DEBUG: spice-channel.c:2289 usbredir-9:0: Started background coroutine 0x29e0750
|
|
|
1a1a75 |
> GSpice-DEBUG: spice-channel.c:916 usbredir-9:1: Read error Error receiving data: Connection reset by peer
|
|
|
1a1a75 |
> GSpice-DEBUG: spice-channel.c:1076 usbredir-9:1: incomplete auth reply (-104/4)
|
|
|
1a1a75 |
> GSpice-DEBUG: spice-channel.c:916 playback-5:0: Read error Error receiving data: Connection reset by peer
|
|
|
1a1a75 |
> GSpice-DEBUG: spice-channel.c:1076 playback-5:0: incomplete auth reply (-104/4)
|
|
|
1a1a75 |
> GSpice-DEBUG: spice-channel.c:916 display-2:0: Read error Error receiving data: Connection reset by peer
|
|
|
1a1a75 |
> GSpice-DEBUG: spice-channel.c:1076 display-2:0: incomplete auth reply (-104/4)
|
|
|
1a1a75 |
> GSpice-DEBUG: spice-session.c:1775 connecting 0x7f12bffffa50...
|
|
|
1a1a75 |
> GSpice-DEBUG: spice-session.c:1850 open host lab.test.me:5900
|
|
|
1a1a75 |
> GSpice-DEBUG: channel-main.c:2241 error or unhandled channel event during migration: 22
|
|
|
1a1a75 |
> GSpice-DEBUG: spice-session.c:1665 session: disconnecting 0
|
|
|
1a1a75 |
> GSpice-DEBUG: spice-channel.c:2160 usbredir-9:1: channel got error
|
|
|
1a1a75 |
> GSpice-DEBUG: spice-channel.c:2433 usbredir-9:1: Coroutine exit usbredir-9:1
|
|
|
1a1a75 |
> GSpice-DEBUG: channel-main.c:2241 error or unhandled channel event during migration: 22
|
|
|
1a1a75 |
|
|
|
1a1a75 |
So, the expected behavior was happening on error, which is
|
|
|
1a1a75 |
channel-main aborting the migration due SpiceChannel::channel-event
|
|
|
1a1a75 |
being emitted with some failure.
|
|
|
1a1a75 |
|
|
|
1a1a75 |
Also note that, with this commit reverted, I don't experience any
|
|
|
1a1a75 |
hanging after migration is aborted. Likely the original bug was fixed.
|
|
|
1a1a75 |
|
|
|
1a1a75 |
[0] https://bugzilla.redhat.com/show_bug.cgi?id=1318574
|
|
|
1a1a75 |
|
|
|
1a1a75 |
Related: https://bugzilla.redhat.com/show_bug.cgi?id=1695618
|
|
|
1a1a75 |
|
|
|
1a1a75 |
Signed-off-by: Victor Toso <victortoso@redhat.com>
|
|
|
1a1a75 |
---
|
|
|
1a1a75 |
src/spice-channel.c | 8 --------
|
|
|
1a1a75 |
1 file changed, 8 deletions(-)
|
|
|
1a1a75 |
|
|
|
1a1a75 |
diff --git a/src/spice-channel.c b/src/spice-channel.c
|
|
|
1a1a75 |
index 2eec4e0..9128e36 100644
|
|
|
1a1a75 |
--- a/src/spice-channel.c
|
|
|
1a1a75 |
+++ b/src/spice-channel.c
|
|
|
1a1a75 |
@@ -2410,7 +2410,6 @@ static gboolean spice_channel_delayed_unref(gpointer data)
|
|
|
1a1a75 |
SpiceChannel *channel = SPICE_CHANNEL(data);
|
|
|
1a1a75 |
SpiceChannelPrivate *c = channel->priv;
|
|
|
1a1a75 |
gboolean was_ready = c->state == SPICE_CHANNEL_STATE_READY;
|
|
|
1a1a75 |
- SpiceSession *session;
|
|
|
1a1a75 |
|
|
|
1a1a75 |
CHANNEL_DEBUG(channel, "Delayed unref channel %p", channel);
|
|
|
1a1a75 |
|
|
|
1a1a75 |
@@ -2418,13 +2417,6 @@ static gboolean spice_channel_delayed_unref(gpointer data)
|
|
|
1a1a75 |
|
|
|
1a1a75 |
c->state = SPICE_CHANNEL_STATE_UNCONNECTED;
|
|
|
1a1a75 |
|
|
|
1a1a75 |
- session = spice_channel_get_session(channel);
|
|
|
1a1a75 |
- if (session && spice_session_is_for_migration(session)) {
|
|
|
1a1a75 |
- /* error during migration - abort migration */
|
|
|
1a1a75 |
- spice_session_abort_migration(session);
|
|
|
1a1a75 |
- return FALSE;
|
|
|
1a1a75 |
- }
|
|
|
1a1a75 |
-
|
|
|
1a1a75 |
if (c->event != SPICE_CHANNEL_NONE) {
|
|
|
1a1a75 |
g_coroutine_signal_emit(channel, signals[SPICE_CHANNEL_EVENT], 0, c->event);
|
|
|
1a1a75 |
c->event = SPICE_CHANNEL_NONE;
|
|
|
1a1a75 |
--
|
|
|
1a1a75 |
2.21.0
|
|
|
1a1a75 |
|