[Spice-commits] src/spice-channel.c

[GitLab Mirror]

 src/spice-channel.c |    8 --------
 1 file changed, 8 deletions(-)

New commits:
commit 8c86fd98df5aa273a6ea7682e6d664e72ea00224
Author: Victor Toso <me at victortoso.com>
Date:   Wed Sep 4 12:37:34 2019 +0200

    Revert "channel: Abort migration in delayed unref"
    
    This reverts commit 109e575 "channel: Abort migration in delayed
    unref" in 2016-05-02 by Pavel Grunt <pgrunt at redhat.com>
    
    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 at redhat.com>
    Acked-by: Frediano Ziglio <fziglio at redhat.com>

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;