[Spice-devel] [spice-gtk PATCH 6/7] seamless migration: don't reset messages data when swapping channels
Marc-André Lureau
marcandre.lureau at gmail.com
Tue Aug 21 04:39:11 PDT 2012
On Wed, Aug 15, 2012 at 9:56 AM, Yonit Halperin <yhalperi at redhat.com> wrote:
> When swapping the src and dest channels's, we need to keep
> the xmit_queue and msg serials. Their state is expected to stay the same
> after migration.
> ---
> gtk/channel-main.c | 4 +++-
> gtk/spice-channel-priv.h | 2 +-
> gtk/spice-channel.c | 12 +++++++-----
> gtk/spice-session-priv.h | 4 +++-
> gtk/spice-session.c | 18 ++++++++++++++----
> gtk/spice-session.h | 7 +++++--
> 6 files changed, 33 insertions(+), 14 deletions(-)
>
> diff --git a/gtk/channel-main.c b/gtk/channel-main.c
> index bdcdbcb..7e98812 100644
> --- a/gtk/channel-main.c
> +++ b/gtk/channel-main.c
> @@ -1829,7 +1829,9 @@ static void main_migrate_connect(SpiceChannel *channel,
> SPICE_DEBUG("migration (semi-seamless): connections all ok");
> reply_type = SPICE_MSGC_MAIN_MIGRATE_CONNECTED;
> }
> - spice_session_set_migration(spice_channel_get_session(channel), mig.session);
> + spice_session_set_migration(spice_channel_get_session(channel),
> + mig.session,
> + mig.do_seamless);
> }
> g_object_unref(mig.session);
>
> diff --git a/gtk/spice-channel-priv.h b/gtk/spice-channel-priv.h
> index 8ed79fa..cb2b75a 100644
> --- a/gtk/spice-channel-priv.h
> +++ b/gtk/spice-channel-priv.h
> @@ -174,7 +174,7 @@ void spice_channel_handle_migrate(SpiceChannel *channel, SpiceMsgIn *in);
>
> gint spice_channel_get_channel_id(SpiceChannel *channel);
> gint spice_channel_get_channel_type(SpiceChannel *channel);
> -void spice_channel_swap(SpiceChannel *channel, SpiceChannel *swap);
> +void spice_channel_swap(SpiceChannel *channel, SpiceChannel *swap, gboolean keep_msgs);
> gboolean spice_channel_get_read_only(SpiceChannel *channel);
> void spice_channel_reset(SpiceChannel *channel, gboolean migrating);
>
> diff --git a/gtk/spice-channel.c b/gtk/spice-channel.c
> index 5f37cbc..bb37c38 100644
> --- a/gtk/spice-channel.c
> +++ b/gtk/spice-channel.c
> @@ -2591,7 +2591,7 @@ enum spice_channel_state spice_channel_get_state(SpiceChannel *channel)
> }
>
> G_GNUC_INTERNAL
> -void spice_channel_swap(SpiceChannel *channel, SpiceChannel *swap)
> +void spice_channel_swap(SpiceChannel *channel, SpiceChannel *swap, gboolean keep_msgs)
> {
> SpiceChannelPrivate *c = SPICE_CHANNEL_GET_PRIVATE(channel);
> SpiceChannelPrivate *s = SPICE_CHANNEL_GET_PRIVATE(swap);
> @@ -2614,11 +2614,13 @@ void spice_channel_swap(SpiceChannel *channel, SpiceChannel *swap)
> SWAP(ctx);
> SWAP(ssl);
> SWAP(sslverify);
> - SWAP(in_serial);
> - SWAP(out_serial);
> SWAP(use_mini_header);
> - SWAP(xmit_queue);
> - SWAP(xmit_queue_blocked);
> + if (!keep_msgs) {
Hmm ! keep_msgs? The argument name is a bit confusing. Would
"swap_msgs" makes more sense instead?
> + SWAP(xmit_queue);
> + SWAP(xmit_queue_blocked);
> + SWAP(in_serial);
> + SWAP(out_serial);
> + }
> SWAP(caps);
> SWAP(common_caps);
> SWAP(remote_caps);
> diff --git a/gtk/spice-session-priv.h b/gtk/spice-session-priv.h
> index c24ef8e..5efe846 100644
> --- a/gtk/spice-session-priv.h
> +++ b/gtk/spice-session-priv.h
> @@ -119,7 +119,9 @@ void spice_session_set_mm_time(SpiceSession *session, guint32 time);
> guint32 spice_session_get_mm_time(SpiceSession *session);
>
> void spice_session_switching_disconnect(SpiceSession *session);
> -void spice_session_set_migration(SpiceSession *session, SpiceSession *migration);
> +void spice_session_set_migration(SpiceSession *session,
> + SpiceSession *migration,
> + gboolean seamless);
It would be more obvious if you passed the enum value directly (ex:
SPICE_SESSION_MIGRATION_MIGRATING_FULL), or change the naming to make
the "migration_full" case more clear.
> void spice_session_abort_migration(SpiceSession *session);
> void spice_session_set_migration_state(SpiceSession *session, SpiceSessionMigration state);
>
> diff --git a/gtk/spice-session.c b/gtk/spice-session.c
> index 995b2ed..bfe6b89 100644
> --- a/gtk/spice-session.c
> +++ b/gtk/spice-session.c
> @@ -1194,7 +1194,9 @@ void spice_session_switching_disconnect(SpiceSession *self)
> }
>
> G_GNUC_INTERNAL
> -void spice_session_set_migration(SpiceSession *session, SpiceSession *migration)
> +void spice_session_set_migration(SpiceSession *session,
> + SpiceSession *migration,
> + gboolean seamless)
> {
> SpiceSessionPrivate *s = SPICE_SESSION_GET_PRIVATE(session);
> SpiceSessionPrivate *m = SPICE_SESSION_GET_PRIVATE(migration);
> @@ -1202,7 +1204,9 @@ void spice_session_set_migration(SpiceSession *session, SpiceSession *migration)
>
> g_return_if_fail(s != NULL);
>
> - spice_session_set_migration_state(session, SPICE_SESSION_MIGRATION_MIGRATING);
> + spice_session_set_migration_state(session,
> + seamless ? SPICE_SESSION_MIGRATION_MIGRATING_FULL :
> + SPICE_SESSION_MIGRATION_MIGRATING_SEMI);
>
> g_warn_if_fail(s->migration == NULL);
> s->migration = g_object_ref(migration);
> @@ -1259,6 +1263,7 @@ void spice_session_abort_migration(SpiceSession *session)
> SpiceSessionPrivate *s = SPICE_SESSION_GET_PRIVATE(session);
> RingItem *ring, *next;
> struct channel *c;
> + gboolean keep_channel_msgs = s->migration_state == SPICE_SESSION_MIGRATION_MIGRATING_FULL;
>
> g_return_if_fail(s != NULL);
>
> @@ -1278,7 +1283,8 @@ void spice_session_abort_migration(SpiceSession *session)
> spice_channel_swap(c->channel,
> spice_session_lookup_channel(s->migration,
> spice_channel_get_channel_id(c->channel),
> - spice_channel_get_channel_type(c->channel)));
> + spice_channel_get_channel_type(c->channel)),
> + keep_channel_msgs);
> }
>
> g_list_free(s->migration_left);
> @@ -1308,7 +1314,11 @@ void spice_session_channel_migrate(SpiceSession *session, SpiceChannel *channel)
> c = spice_session_lookup_channel(s->migration, id, type);
> g_return_if_fail(c != NULL);
>
> - spice_channel_swap(channel, c);
> + if (!g_queue_is_empty(&c->priv->xmit_queue) &&
> + s->migration_state == SPICE_SESSION_MIGRATION_MIGRATING_FULL) {
> + g_critical("mig channel xmit queue is not empty. type %s", c->priv->name);
> + }
> + spice_channel_swap(channel, c, s->migration_state == SPICE_SESSION_MIGRATION_MIGRATING_FULL);
> s->migration_left = g_list_remove(s->migration_left, channel);
>
> if (g_list_length(s->migration_left) == 0) {
> diff --git a/gtk/spice-session.h b/gtk/spice-session.h
> index 4895288..4390ff0 100644
> --- a/gtk/spice-session.h
> +++ b/gtk/spice-session.h
> @@ -52,14 +52,17 @@ typedef enum {
> *
> * @SPICE_SESSION_MIGRATION_NONE: no migration going on
> * @SPICE_SESSION_MIGRATION_SWITCHING: the session is switching host (destroy and reconnect)
> - * @SPICE_SESSION_MIGRATION_MIGRATING: the session is migrating seamlessly (reconnect)
> + * @SPICE_SESSION_MIGRATION_MIGRATING_SEMI: the session is migrating semi seamlessly (reconnect)
> + * @SPICE_SESSION_MIGRATION_MIGRATING_FULL: the session is migrating seamlessly
> + * (swap sockets, keep state)
> *
> * Session migration state.
> **/
> typedef enum {
> SPICE_SESSION_MIGRATION_NONE,
> SPICE_SESSION_MIGRATION_SWITCHING,
> - SPICE_SESSION_MIGRATION_MIGRATING,
> + SPICE_SESSION_MIGRATION_MIGRATING_SEMI,
> + SPICE_SESSION_MIGRATION_MIGRATING_FULL,
> } SpiceSessionMigration;
>
> struct _SpiceSession
You can't change public API. Perhaps you could use a private boolean
"full_migration" instead?
--
Marc-André Lureau
More information about the Spice-devel
mailing list