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