Re: [spice-gtk v1] Revert "channel: Abort migration in delayed unref"

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Ping :)

On Wed, Sep 04, 2019 at 02:32:09PM +0200, Victor Toso wrote:
> 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

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Index of Archives]     [Linux Virtualization]     [Linux Virtualization]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]