On Tue, Jan 29, 2019 at 03:16:43PM -0600, Jonathon Jongsma wrote:
> Due to changes in commit 65ef66e4, when the initial connection fails,
> virt-viewer just sat quietly and didn't indicate what was wrong. It also
> did not exit as it did before. This is because we were using
> virt_viewer_session_spice_channel_destroy() incorrectly. This function
> was intended to be a callback that is called to clean up the VV session
> when the SpiceSession tells us that a channel has been destroyed. It
> does not actually destroy the channel, it only cleans up references to
> that channel within virt-viewer. After calling this function, the
> channel is not affected in any way. If the channel object was valid
> before calling the function, it will be valid and unchanged after
> calling the function as well.
>
> The problem is that before commit 65ef66e4, this function
> (_channel_destroy()) also had a side-effect of emitting a signal that
> made us think that the SpiceSession was disconnected when it was not.
> The application responded to this signal by exiting even though the
> session was not properly disconnected and cleaned up.
>
> We now no longer exit the application until the SpiceSession is properly
> disconnected and cleaned up. So we need to make sure that this happens
> when our initial connection fails. Therefore, when the main channel
> receives an error channel-event, we should not call
> virt_viewer_session_spice_channel_destroy(). This function should only
> be called when a channel has actually been destroyed, and the channel is
> not destroyed at this point. We should instead explicitly disconnect
> the session, which will result in the channels being destroyed properly.
> After the session destroys all of the channels, the 'channel-destroy' signal
> will be emitted by SpiceSession, so the _channel_destroy() function will
> eventually get called by the signal handler.
>
> Fixes: rhbz#1666869
> ---
> src/virt-viewer-session-spice.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/src/virt-viewer-session-spice.c b/src/virt-viewer-session-spice.c
> index f76c7f9..b197e24 100644
> --- a/src/virt-viewer-session-spice.c
> +++ b/src/virt-viewer-session-spice.c
> @@ -776,14 +776,14 @@ virt_viewer_session_spice_main_channel_event(SpiceChannel *channel,
> spice_session_connect(self->priv->session);
> }
> } else {
> - virt_viewer_session_spice_channel_destroy(NULL, channel, session);
> + spice_session_disconnect(self->priv->session);
> }
> break;
> }
> case SPICE_CHANNEL_ERROR_IO:
> case SPICE_CHANNEL_ERROR_LINK:
> case SPICE_CHANNEL_ERROR_TLS:
> - virt_viewer_session_spice_channel_destroy(NULL, channel, session);
> + spice_session_disconnect(self->priv->session);
> break;
> default:
> g_warning("unhandled spice main channel event: %d", event);
> @@ -1111,6 +1111,7 @@ virt_viewer_session_spice_session_disconnected(G_GNUC_UNUSED SpiceSession *s,
> g_signal_emit_by_name(self, "session-disconnected", error ? error->message : NULL);
> }
>
> +/* called when the spice session indicates that a session has been destroyed */
> static void
> virt_viewer_session_spice_channel_destroy(G_GNUC_UNUSED SpiceSession *s,
> SpiceChannel *channel,
Maybe it should be renamed to xxx_destroyed?
Acked-by: Christophe Fergeau <cfergeau@xxxxxxxxxx>
Christophe
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list
