[Spice-devel,spice-gtk,6/7] seamless migration: don't reset messages data when swapping channels

Submitted by Yonit Halperin on Aug. 15, 2012, 7:56 a.m.

Details

Message ID 1345017402-872-66-git-send-email-yhalperi@redhat.com
State New
Headers show

Not browsing as part of any series.

Commit Message

Yonit Halperin Aug. 15, 2012, 7:56 a.m.
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(-)

Patch hide | download patch | download mbox

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) {
+        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);
 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

Comments

On Wed, Aug 15, 2012 at 9:56 AM, Yonit Halperin <yhalperi@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?