[Spice-commits] 6 commits - gtk/spice-channel-priv.h gtk/spice-channel.c gtk/spice-session.c
Marc-André Lureau
elmarco at kemper.freedesktop.org
Tue Dec 16 09:40:39 PST 2014
gtk/spice-channel-priv.h | 2
gtk/spice-channel.c | 112 ++++++++++++++++++++++++-----------------------
gtk/spice-session.c | 30 +++++++-----
3 files changed, 78 insertions(+), 66 deletions(-)
New commits:
commit 43fc492149767bdade118d586572a2ce5abe63fc
Author: Marc-André Lureau <marcandre.lureau at redhat.com>
Date: Tue Dec 16 18:30:02 2014 +0100
channel: clear channel error after auth error
When entered authentication details are wrong, spice-gtk will reset
channel error, which will result in the following warning:
(remote-viewer:20753): GLib-WARNING **: GError set over the top of a
previous GError or uninitialized memory.
This indicates a bug in someone's code. You must ensure an error is NULL
before it's set.
Clear channel error after reporting authentication error.
diff --git a/gtk/spice-channel.c b/gtk/spice-channel.c
index c00bb42..fb7b0d5 100644
--- a/gtk/spice-channel.c
+++ b/gtk/spice-channel.c
@@ -2180,8 +2180,10 @@ static gboolean spice_channel_delayed_unref(gpointer data)
g_return_val_if_fail(c->coroutine.coroutine.exited == TRUE, FALSE);
- if (c->state == SPICE_CHANNEL_STATE_FAILED_AUTHENTICATION)
+ if (c->state == SPICE_CHANNEL_STATE_FAILED_AUTHENTICATION) {
g_coroutine_signal_emit(channel, signals[SPICE_CHANNEL_EVENT], 0, SPICE_CHANNEL_ERROR_AUTH);
+ g_clear_error(&c->error);
+ }
g_object_unref(G_OBJECT(data));
commit 201a8c2e36e250f49d5c729b11563730dfa4c484
Author: Marc-André Lureau <marcandre.lureau at redhat.com>
Date: Tue Dec 16 18:04:53 2014 +0100
channel: throw auth error when coroutine ends
It is common that clients attempt to reconnect during the
SPICE_CHANNEL_ERROR_AUTH callback. However, the channel must exit
the coroutine first before reconnection can happen.
diff --git a/gtk/spice-channel.c b/gtk/spice-channel.c
index 4081e0b..c00bb42 100644
--- a/gtk/spice-channel.c
+++ b/gtk/spice-channel.c
@@ -1069,8 +1069,6 @@ static void spice_channel_failed_authentication(SpiceChannel *channel)
c->state = SPICE_CHANNEL_STATE_FAILED_AUTHENTICATION;
- g_coroutine_signal_emit(channel, signals[SPICE_CHANNEL_EVENT], 0, SPICE_CHANNEL_ERROR_AUTH);
-
c->has_error = TRUE; /* force disconnect */
}
@@ -2182,6 +2180,9 @@ static gboolean spice_channel_delayed_unref(gpointer data)
g_return_val_if_fail(c->coroutine.coroutine.exited == TRUE, FALSE);
+ if (c->state == SPICE_CHANNEL_STATE_FAILED_AUTHENTICATION)
+ g_coroutine_signal_emit(channel, signals[SPICE_CHANNEL_EVENT], 0, SPICE_CHANNEL_ERROR_AUTH);
+
g_object_unref(G_OBJECT(data));
return FALSE;
@@ -2458,6 +2459,7 @@ static gboolean connect_delayed(gpointer data)
return FALSE;
}
+/* any context */
static gboolean channel_connect(SpiceChannel *channel)
{
SpiceChannelPrivate *c = channel->priv;
@@ -2639,7 +2641,6 @@ static void channel_disconnect(SpiceChannel *channel)
if (c->state == SPICE_CHANNEL_STATE_READY)
g_coroutine_signal_emit(channel, signals[SPICE_CHANNEL_EVENT], 0, SPICE_CHANNEL_CLOSED);
- c->state = SPICE_CHANNEL_STATE_UNCONNECTED;
spice_channel_reset(channel, FALSE);
g_return_if_fail(SPICE_IS_CHANNEL(channel));
commit 9969d042bbedfddfb9589bdcc05d877a3bd1fa84
Author: Marc-André Lureau <marcandre.lureau at redhat.com>
Date: Tue Dec 16 17:56:37 2014 +0100
channel: introduce SPICE_CHANNEL_STATE_RECONNECTING
Add a new state that permits reconnection, because it's < CONNECTING.
It also simplifies some code by removing unneeded variables in
spice_channel_coroutine(): the channel.tls and session.protocol version
properties are already modified during initial connection steps.
diff --git a/gtk/spice-channel-priv.h b/gtk/spice-channel-priv.h
index 671e9fe..bd7f490 100644
--- a/gtk/spice-channel-priv.h
+++ b/gtk/spice-channel-priv.h
@@ -67,6 +67,7 @@ struct _SpiceMsgIn {
enum spice_channel_state {
SPICE_CHANNEL_STATE_UNCONNECTED = 0,
SPICE_CHANNEL_STATE_FAILED_AUTHENTICATION,
+ SPICE_CHANNEL_STATE_RECONNECTING,
SPICE_CHANNEL_STATE_CONNECTING,
SPICE_CHANNEL_STATE_READY,
SPICE_CHANNEL_STATE_SWITCHING,
diff --git a/gtk/spice-channel.c b/gtk/spice-channel.c
index 524a13e..4081e0b 100644
--- a/gtk/spice-channel.c
+++ b/gtk/spice-channel.c
@@ -1183,12 +1183,11 @@ static void spice_channel_send_link(SpiceChannel *channel)
}
/* coroutine context */
-static gboolean spice_channel_recv_link_hdr(SpiceChannel *channel, gboolean *switch_protocol)
+static gboolean spice_channel_recv_link_hdr(SpiceChannel *channel)
{
SpiceChannelPrivate *c = channel->priv;
int rc;
- *switch_protocol = FALSE;
rc = spice_channel_read(channel, &c->peer_hdr, sizeof(c->peer_hdr));
if (rc != sizeof(c->peer_hdr)) {
g_warning("incomplete link header (%d/%" G_GSIZE_FORMAT ")",
@@ -1221,7 +1220,7 @@ error:
incompatible. Try with the oldest protocol in this case: */
if (c->link_hdr.major_version != 1) {
SPICE_DEBUG("%s: error, switching to protocol 1 (spice 0.4)", c->name);
- *switch_protocol = TRUE;
+ c->state = SPICE_CHANNEL_STATE_RECONNECTING;
g_object_set(c->session, "protocol", 1, NULL);
return FALSE;
}
@@ -1680,7 +1679,7 @@ cleanup:
#endif /* HAVE_SASL */
/* coroutine context */
-static gboolean spice_channel_recv_link_msg(SpiceChannel *channel, gboolean *switch_tls)
+static gboolean spice_channel_recv_link_msg(SpiceChannel *channel)
{
SpiceChannelPrivate *c;
int rc, num_caps, i;
@@ -1705,8 +1704,9 @@ static gboolean spice_channel_recv_link_msg(SpiceChannel *channel, gboolean *swi
/* nothing */
break;
case SPICE_LINK_ERR_NEED_SECURED:
- *switch_tls = true;
+ c->state = SPICE_CHANNEL_STATE_RECONNECTING;
CHANNEL_DEBUG(channel, "switching to tls");
+ c->tls = TRUE;
return FALSE;
default:
g_warning("%s: %s: unhandled error %d",
@@ -2280,8 +2280,6 @@ static void *spice_channel_coroutine(void *data)
SpiceChannelPrivate *c = channel->priv;
guint verify;
int rc, delay_val = 1;
- gboolean switch_tls = FALSE;
- gboolean switch_protocol = FALSE;
/* When some other SSL/TLS version becomes obsolete, add it to this
* variable. */
long ssl_options = SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3;
@@ -2415,8 +2413,8 @@ connected:
}
spice_channel_send_link(channel);
- if (!spice_channel_recv_link_hdr(channel, &switch_protocol) ||
- !spice_channel_recv_link_msg(channel, &switch_tls) ||
+ if (!spice_channel_recv_link_hdr(channel) ||
+ !spice_channel_recv_link_msg(channel) ||
!spice_channel_recv_auth(channel))
goto cleanup;
@@ -2428,8 +2426,7 @@ cleanup:
SPICE_CHANNEL_GET_CLASS(channel)->channel_disconnect(channel);
- if (switch_protocol || (switch_tls && !c->tls)) {
- c->tls = switch_tls;
+ if (c->state == SPICE_CHANNEL_STATE_RECONNECTING) {
spice_channel_connect(channel);
g_object_unref(channel);
} else
commit 14ce8b8dabf47ec208148a83490503084445ba1d
Author: Marc-André Lureau <marcandre.lureau at redhat.com>
Date: Tue Dec 16 17:19:00 2014 +0100
channel: do not enter channel iterate on early error
There is no need to enter channel_iterate() if we found an early
connection steps error.
diff --git a/gtk/spice-channel.c b/gtk/spice-channel.c
index d0e6df8..524a13e 100644
--- a/gtk/spice-channel.c
+++ b/gtk/spice-channel.c
@@ -1075,7 +1075,7 @@ static void spice_channel_failed_authentication(SpiceChannel *channel)
}
/* coroutine context */
-static void spice_channel_recv_auth(SpiceChannel *channel)
+static gboolean spice_channel_recv_auth(SpiceChannel *channel)
{
SpiceChannelPrivate *c = channel->priv;
uint32_t link_res;
@@ -1086,13 +1086,13 @@ static void spice_channel_recv_auth(SpiceChannel *channel)
CHANNEL_DEBUG(channel, "incomplete auth reply (%d/%" G_GSIZE_FORMAT ")",
rc, sizeof(link_res));
g_coroutine_signal_emit(channel, signals[SPICE_CHANNEL_EVENT], 0, SPICE_CHANNEL_ERROR_LINK);
- return;
+ return FALSE;
}
if (link_res != SPICE_LINK_ERR_OK) {
CHANNEL_DEBUG(channel, "link result: reply %d", link_res);
spice_channel_failed_authentication(channel);
- return;
+ return FALSE;
}
c->state = SPICE_CHANNEL_STATE_READY;
@@ -1105,6 +1105,8 @@ static void spice_channel_recv_auth(SpiceChannel *channel)
if (c->state != SPICE_CHANNEL_STATE_MIGRATING)
spice_channel_up(channel);
+
+ return TRUE;
}
G_GNUC_INTERNAL
@@ -1678,14 +1680,14 @@ cleanup:
#endif /* HAVE_SASL */
/* coroutine context */
-static void spice_channel_recv_link_msg(SpiceChannel *channel, gboolean *switch_tls)
+static gboolean spice_channel_recv_link_msg(SpiceChannel *channel, gboolean *switch_tls)
{
SpiceChannelPrivate *c;
int rc, num_caps, i;
uint32_t *caps;
- g_return_if_fail(channel != NULL);
- g_return_if_fail(channel->priv != NULL);
+ g_return_val_if_fail(channel != NULL, FALSE);
+ g_return_val_if_fail(channel->priv != NULL, FALSE);
c = channel->priv;
@@ -1696,7 +1698,7 @@ static void spice_channel_recv_link_msg(SpiceChannel *channel, gboolean *switch_
g_critical("%s: %s: incomplete link reply (%d/%d)",
c->name, __FUNCTION__, rc, c->peer_hdr.size);
g_coroutine_signal_emit(channel, signals[SPICE_CHANNEL_EVENT], 0, SPICE_CHANNEL_ERROR_LINK);
- return;
+ return FALSE;
}
switch (c->peer_msg->error) {
case SPICE_LINK_ERR_OK:
@@ -1705,7 +1707,7 @@ static void spice_channel_recv_link_msg(SpiceChannel *channel, gboolean *switch_
case SPICE_LINK_ERR_NEED_SECURED:
*switch_tls = true;
CHANNEL_DEBUG(channel, "switching to tls");
- return;
+ return FALSE;
default:
g_warning("%s: %s: unhandled error %d",
c->name, __FUNCTION__, c->peer_msg->error);
@@ -1744,7 +1746,8 @@ static void spice_channel_recv_link_msg(SpiceChannel *channel, gboolean *switch_
CHANNEL_DEBUG(channel, "Choosing SASL mechanism");
auth.auth_mechanism = SPICE_COMMON_CAP_AUTH_SASL;
spice_channel_write(channel, &auth, sizeof(auth));
- spice_channel_perform_auth_sasl(channel);
+ if (!spice_channel_perform_auth_sasl(channel))
+ return FALSE;
} else
#endif
if (spice_channel_test_common_capability(channel, SPICE_COMMON_CAP_AUTH_SPICE)) {
@@ -1759,11 +1762,12 @@ static void spice_channel_recv_link_msg(SpiceChannel *channel, gboolean *switch_
c->use_mini_header = spice_channel_test_common_capability(channel,
SPICE_COMMON_CAP_MINI_HEADER);
CHANNEL_DEBUG(channel, "use mini header: %d", c->use_mini_header);
- return;
+ return TRUE;
error:
SPICE_CHANNEL_GET_CLASS(channel)->channel_disconnect(channel);
g_coroutine_signal_emit(channel, signals[SPICE_CHANNEL_EVENT], 0, SPICE_CHANNEL_ERROR_LINK);
+ return FALSE;
}
/* system context */
@@ -2411,12 +2415,10 @@ connected:
}
spice_channel_send_link(channel);
- if (spice_channel_recv_link_hdr(channel, &switch_protocol) == FALSE)
- goto cleanup;
- spice_channel_recv_link_msg(channel, &switch_tls);
- if (switch_tls)
+ if (!spice_channel_recv_link_hdr(channel, &switch_protocol) ||
+ !spice_channel_recv_link_msg(channel, &switch_tls) ||
+ !spice_channel_recv_auth(channel))
goto cleanup;
- spice_channel_recv_auth(channel);
while (spice_channel_iterate(channel))
;
commit 21acda1c51241ff2cce013509900e0a8373d409d
Author: Marc-André Lureau <marcandre.lureau at redhat.com>
Date: Tue Dec 16 14:24:24 2014 +0100
channel: factorize failed authentication
There are a few things that should be common to all wrong authentication
cases. Let's put them all in the same function.
diff --git a/gtk/spice-channel-priv.h b/gtk/spice-channel-priv.h
index 07012db..671e9fe 100644
--- a/gtk/spice-channel-priv.h
+++ b/gtk/spice-channel-priv.h
@@ -66,6 +66,7 @@ struct _SpiceMsgIn {
enum spice_channel_state {
SPICE_CHANNEL_STATE_UNCONNECTED = 0,
+ SPICE_CHANNEL_STATE_FAILED_AUTHENTICATION,
SPICE_CHANNEL_STATE_CONNECTING,
SPICE_CHANNEL_STATE_READY,
SPICE_CHANNEL_STATE_SWITCHING,
diff --git a/gtk/spice-channel.c b/gtk/spice-channel.c
index ea0ed34..d0e6df8 100644
--- a/gtk/spice-channel.c
+++ b/gtk/spice-channel.c
@@ -1052,6 +1052,29 @@ static void spice_channel_send_spice_ticket(SpiceChannel *channel)
}
/* coroutine context */
+static void spice_channel_failed_authentication(SpiceChannel *channel)
+{
+ SpiceChannelPrivate *c = channel->priv;
+
+ if (c->auth_needs_username_and_password)
+ g_set_error_literal(&c->error,
+ SPICE_CLIENT_ERROR,
+ SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD_AND_USERNAME,
+ _("Authentication failed: password and username are required"));
+ else
+ g_set_error_literal(&c->error,
+ SPICE_CLIENT_ERROR,
+ SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD,
+ _("Authentication failed: password is required"));
+
+ c->state = SPICE_CHANNEL_STATE_FAILED_AUTHENTICATION;
+
+ g_coroutine_signal_emit(channel, signals[SPICE_CHANNEL_EVENT], 0, SPICE_CHANNEL_ERROR_AUTH);
+
+ c->has_error = TRUE; /* force disconnect */
+}
+
+/* coroutine context */
static void spice_channel_recv_auth(SpiceChannel *channel)
{
SpiceChannelPrivate *c = channel->priv;
@@ -1068,7 +1091,7 @@ static void spice_channel_recv_auth(SpiceChannel *channel)
if (link_res != SPICE_LINK_ERR_OK) {
CHANNEL_DEBUG(channel, "link result: reply %d", link_res);
- g_coroutine_signal_emit(channel, signals[SPICE_CHANNEL_EVENT], 0, SPICE_CHANNEL_ERROR_AUTH);
+ spice_channel_failed_authentication(channel);
return;
}
@@ -1310,22 +1333,6 @@ spice_channel_gather_sasl_credentials(SpiceChannel *channel,
#define SASL_MAX_MECHNAME_LEN 100
#define SASL_MAX_DATA_LEN (1024 * 1024)
-static void spice_channel_set_detailed_authentication_error(SpiceChannel *channel)
-{
- SpiceChannelPrivate *c = channel->priv;
-
- if (c->auth_needs_username_and_password)
- g_set_error_literal(&c->error,
- SPICE_CLIENT_ERROR,
- SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD_AND_USERNAME,
- _("Authentication failed: password and username are required"));
- else
- g_set_error_literal(&c->error,
- SPICE_CLIENT_ERROR,
- SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD,
- _("Authentication failed: password is required"));
-}
-
/* Perform the SASL authentication process
*/
static gboolean spice_channel_perform_auth_sasl(SpiceChannel *channel)
@@ -1644,23 +1651,20 @@ restart:
complete:
CHANNEL_DEBUG(channel, "%s", "SASL authentication complete");
spice_channel_read(channel, &len, sizeof(len));
- if (len != SPICE_LINK_ERR_OK) {
- spice_channel_set_detailed_authentication_error(channel);
- g_coroutine_signal_emit(channel, signals[SPICE_CHANNEL_EVENT], 0, SPICE_CHANNEL_ERROR_AUTH);
+ if (len == SPICE_LINK_ERR_OK) {
+ ret = TRUE;
+ /* This must come *after* check-auth-result, because the former
+ * is defined to be sent unencrypted, and setting saslconn turns
+ * on the SSF layer encryption processing */
+ c->sasl_conn = saslconn;
+ goto cleanup;
}
- ret = len == SPICE_LINK_ERR_OK;
- /* This must come *after* check-auth-result, because the former
- * is defined to be sent unencrypted, and setting saslconn turns
- * on the SSF layer encryption processing */
- c->sasl_conn = saslconn;
- goto cleanup;
error:
if (saslconn)
sasl_dispose(&saslconn);
- spice_channel_set_detailed_authentication_error(channel);
- g_coroutine_signal_emit(channel, signals[SPICE_CHANNEL_EVENT], 0, SPICE_CHANNEL_ERROR_AUTH);
- c->has_error = TRUE; /* force disconnect */
+
+ spice_channel_failed_authentication(channel);
ret = FALSE;
cleanup:
commit 6b475802d78f2a4f2110738cb360b496dc85336c
Author: Marc-André Lureau <marcandre.lureau at redhat.com>
Date: Tue Dec 16 15:11:19 2014 +0100
session: keep main channel on reconnect
For legacy reasons, spice-gtk should keep at least one channel in the
session when reconnecting (clients may decide that the session is
disconnected when all channels are gone). The most obvious is to
keep and reuse the main channel.
diff --git a/gtk/spice-session.c b/gtk/spice-session.c
index c80c8dc..7971f3c 100644
--- a/gtk/spice-session.c
+++ b/gtk/spice-session.c
@@ -258,19 +258,23 @@ static void spice_session_init(SpiceSession *session)
}
static void
-session_disconnect(SpiceSession *self)
+session_disconnect(SpiceSession *self, gboolean keep_main)
{
SpiceSessionPrivate *s;
struct channel *item;
RingItem *ring, *next;
s = self->priv;
- s->cmain = NULL;
for (ring = ring_get_head(&s->channels); ring != NULL; ring = next) {
next = ring_next(&s->channels, ring);
item = SPICE_CONTAINEROF(ring, struct channel, link);
- spice_session_channel_destroy(self, item->channel);
+
+ if (keep_main && item->channel == s->cmain) {
+ spice_channel_disconnect(item->channel, SPICE_CHANNEL_NONE);
+ } else {
+ spice_session_channel_destroy(self, item->channel);
+ }
}
s->connection_id = 0;
@@ -290,7 +294,7 @@ spice_session_dispose(GObject *gobject)
SPICE_DEBUG("session dispose");
- session_disconnect(session);
+ session_disconnect(session, FALSE);
g_warn_if_fail(s->migration == NULL);
g_warn_if_fail(s->migration_left == NULL);
@@ -1410,12 +1414,12 @@ gboolean spice_session_connect(SpiceSession *session)
s = session->priv;
g_return_val_if_fail(!s->disconnecting, FALSE);
- session_disconnect(session);
+ session_disconnect(session, TRUE);
s->client_provided_sockets = FALSE;
- g_warn_if_fail(s->cmain == NULL);
- s->cmain = spice_channel_new(session, SPICE_CHANNEL_MAIN, 0);
+ if (s->cmain == NULL)
+ s->cmain = spice_channel_new(session, SPICE_CHANNEL_MAIN, 0);
glz_decoder_window_clear(s->glz_window);
return spice_channel_connect(s->cmain);
@@ -1445,12 +1449,12 @@ gboolean spice_session_open_fd(SpiceSession *session, int fd)
s = session->priv;
g_return_val_if_fail(!s->disconnecting, FALSE);
- session_disconnect(session);
+ session_disconnect(session, TRUE);
s->client_provided_sockets = TRUE;
- g_warn_if_fail(s->cmain == NULL);
- s->cmain = spice_channel_new(session, SPICE_CHANNEL_MAIN, 0);
+ if (s->cmain == NULL)
+ s->cmain = spice_channel_new(session, SPICE_CHANNEL_MAIN, 0);
glz_decoder_window_clear(s->glz_window);
return spice_channel_open_fd(s->cmain, fd);
@@ -1602,7 +1606,7 @@ void spice_session_abort_migration(SpiceSession *session)
end:
g_list_free(s->migration_left);
s->migration_left = NULL;
- session_disconnect(s->migration);
+ session_disconnect(s->migration, FALSE);
g_object_unref(s->migration);
s->migration = NULL;
@@ -1642,7 +1646,7 @@ void spice_session_channel_migrate(SpiceSession *session, SpiceChannel *channel)
if (g_list_length(s->migration_left) == 0) {
CHANNEL_DEBUG(channel, "migration: all channel migrated, success");
- session_disconnect(s->migration);
+ session_disconnect(s->migration, FALSE);
g_object_unref(s->migration);
s->migration = NULL;
spice_session_set_migration_state(session, SPICE_SESSION_MIGRATION_NONE);
@@ -1749,7 +1753,7 @@ static gboolean session_disconnect_idle(SpiceSession *self)
{
SpiceSessionPrivate *s = self->priv;
- session_disconnect(self);
+ session_disconnect(self, FALSE);
s->disconnecting = 0;
g_object_unref(self);
More information about the Spice-commits
mailing list