[Qemu-devel] [PATCH spice-server 00/13] semi-seamless migration v2 (RHBZ #738266, 725009)

[Qemu-devel] [PATCH spice-server 00/13] semi-seamless migration v2 (RHBZ #738266, 725009)

[Yonit Halperin]

same as the previous version with the following changes:

(1) I'm sending only the spice-server patches. I will send the client fixes later.
    (and then I will move the "Release 0.8.3" patch to the head).
(2) migration interface changes:
    * spice_server_migrate_connect was added.
      It is called on client_migrate_info cmd, if spice-server is new (instead of spice_server_migrate_info).
    * migrate_end_complete callback was added to the migration interface.
      In case we will need it for future seamless migration implementation
    * bump release to 0.8.3
(3) fall back to the switch host scheme in case of client failure to connect the target
(4) fix accessing the wrong ticket in the target side (legacy from old seamless migration code)
(5) fix not calling to migrate_connect_complete callback when no client is connected 


reminder:

Here is a summary of the new migration scheme (copied from the commit msg of the first patch)
    
    migration source side
    ---------------------
    (1) spice_server_migrate_connect (*): tell client to link
        to the target side - send SPICE_MSG_MAIN_MIGRATE_BEGIN.
        It will be called on client_migrate_info cmd. client_migrate_info is asynchronous.
    (2) Complete client_migrate_info only when the client has been connected
        to the target - wait for SPICE_MSGC_MAIN_MIGRATE_(CONNECTED|CONNECT_ERROR) or a timeout.
    (3) spice_server_migrate_end: tell client migration it can switch to the target - send
        SPICE_MSG_MAIN_MIGRATE_END.
    (4) client cleans up all data related to the connection to the source and switches to the target.
        It sends SPICE_MSGC_MAIN_MIGRATE_END.
    
    migration target side
    ---------------------
    (1) the server identifies itself as a migraiton target since the client is linked with (connection_id != 0)
    (2) server doesn't start the channels' logic (channel->link) till it receives SPICE_MSGC_MAIN_MIGRATE_END
        from the client.
    
    *   After migration starts, the target qemu is blocked and cannot accept new spice client
        connections. Thus, we trigger the connection to the target upon client_migrate_info
        command.

Yonit Halperin (13):
  server/spice.h: semi-seamless migration interface, RHBZ #738266
  server: handle migration interface addition
  configure: spice-protocol >= 0.8.2 (semi-seamless migration protocol)
  server,proto: tell the client to connect to the migration target
    before migraton starts
  spice.proto: add SPICE_MSG_MAIN_MIGRATE_END &
    SPICE_MSGC_MAIN_MIGRATE_END
  server: send SPICE_MSG_MAIN_MIGRATE_END on spice_server_migrate_end
  server: move SPICE_MSG_MAIN_INIT sending code to a separate routine
  server: move the linking of channels to a separate routine
  server: handling semi-seamless migration in the target side
  server: call migrate_connect_complete callback when no client is
    connected
  server: turn spice_server_migrate_start into a valid call
  server: fall back to switch host scheme in case semi-seamless
    connection to target fails
  Release 0.8.3

 NEWS                        |    7 +
 common/messages.h           |    2 +
 configure.ac                |    4 +-
 server/reds.c               |  545 +++++++++++++++++++++++++++++++++----------
 server/reds.h               |    4 +
 server/spice-experimental.h |    3 -
 server/spice-server.syms    |    4 +
 server/spice.h              |   29 +++-
 spice.proto                 |    9 +-
 9 files changed, 474 insertions(+), 133 deletions(-)

-- 
1.7.4.4

[Qemu-devel] [PATCH spice-server 01/13] server/spice.h: semi-seamless migration interface, RHBZ #738266

[Yonit Halperin]

semi-seamless migration details:

migration source side
---------------------
(1) spice_server_migrate_connect (*): tell client to link
    to the target side - send SPICE_MSG_MAIN_MIGRATE_BEGIN.
    This should be called upon client_migrate_info cmd.
    client_migrate_info is asynchronous.
(2) Complete spice_server_migrate_connect only when the client has been connected
    to the target - wait for SPICE_MSGC_MAIN_MIGRATE_(CONNECTED|CONNECT_ERROR) or a timeout.
(3) spice_server_migrate_end: tell client migration it can switch to the target - send
    SPICE_MSG_MAIN_MIGRATE_END.
(4) client cleans up all data related to the connection to the source and switches to the target.
    It sends SPICE_MSGC_MAIN_MIGRATE_END.

migration target side
---------------------
(1) the server identifies itself as a migraiton target since the client is linked with (connection_id != 0)
(2) server doesn't start the channels' logic (channel->link) till it receives SPICE_MSGC_MAIN_MIGRATE_END
    from the client.

*   After migration starts, the target qemu is blocked and cannot accept new spice client
    connections. Thus, we trigger the connection to the target upon client_migrate_info
    command.

Signed-off-by: Yonit Halperin <yhalperi@redhat.com>
---
 server/reds.c               |    9 +++++++++
 server/spice-experimental.h |    3 ---
 server/spice.h              |   27 ++++++++++++++++++++++++++-
 3 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/server/reds.c b/server/reds.c
index f082c53..9a983f8 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -4854,6 +4854,15 @@ SPICE_GNUC_VISIBLE int spice_server_set_agent_copypaste(SpiceServer *s, int enab
     return 0;
 }
 
+/* semi-seamless client migration */
+SPICE_GNUC_VISIBLE int spice_server_migrate_connect(SpiceServer *s, const char* dest,
+                                                    int port, int secure_port,
+                                                    const char* cert_subject)
+{
+    red_printf("not implemented yet");
+    return 0;
+}
+
 SPICE_GNUC_VISIBLE int spice_server_migrate_info(SpiceServer *s, const char* dest,
                                           int port, int secure_port,
                                           const char* cert_subject)
diff --git a/server/spice-experimental.h b/server/spice-experimental.h
index 482ac44..6997aa0 100644
--- a/server/spice-experimental.h
+++ b/server/spice-experimental.h
@@ -29,16 +29,13 @@ void spice_server_net_wire_recv_packet(SpiceNetWireInstance *sin,
                                        const uint8_t *pkt, int len);
 
 /* spice seamless client migration (broken) */
-
 enum {
     SPICE_MIGRATE_CLIENT_NONE = 1,
     SPICE_MIGRATE_CLIENT_WAITING,
     SPICE_MIGRATE_CLIENT_READY,
 };
 
-int spice_server_migrate_start(SpiceServer *s);
 int spice_server_migrate_client_state(SpiceServer *s);
-int spice_server_migrate_end(SpiceServer *s, int completed);
 
 #endif // __SPICE_EXPERIMENTAL_H__
 
diff --git a/server/spice.h b/server/spice.h
index ac5a41e..42ddbc6 100644
--- a/server/spice.h
+++ b/server/spice.h
@@ -469,11 +469,36 @@ int spice_server_set_agent_copypaste(SpiceServer *s, int enable);
 int spice_server_get_sock_info(SpiceServer *s, struct sockaddr *sa, socklen_t *salen);
 int spice_server_get_peer_info(SpiceServer *s, struct sockaddr *sa, socklen_t *salen);
 
-/* spice switch-host client migration */
+/* migration interface */
+#define SPICE_INTERFACE_MIGRATION "migration"
+#define SPICE_INTERFACE_MIGRATION_MAJOR 1
+#define SPICE_INTERFACE_MIGRATION_MINOR 1
+typedef struct SpiceMigrateInterface SpiceMigrateInterface;
+typedef struct SpiceMigrateInstance SpiceMigrateInstance;
+typedef struct SpiceMigrateState SpiceMigrateState;
+
+struct SpiceMigrateInterface {
+    SpiceBaseInterface base;
+    void (*migrate_connect_complete)(SpiceMigrateInstance *sin);
+    void (*migrate_end_complete)(SpiceMigrateInstance *sin);
+};
+
+struct SpiceMigrateInstance {
+    SpiceBaseInstance base;
+    SpiceMigrateState *st;
+};
 
+/* spice switch-host client migration */
 int spice_server_migrate_info(SpiceServer *s, const char* dest,
                               int port, int secure_port,
                               const char* cert_subject);
 int spice_server_migrate_switch(SpiceServer *s);
 
+/* spice (semi-)seamless client migration */
+int spice_server_migrate_connect(SpiceServer *s, const char* dest,
+                                 int port, int secure_port,
+                                 const char* cert_subject);
+int spice_server_migrate_start(SpiceServer *s);
+int spice_server_migrate_end(SpiceServer *s, int completed);
+
 #endif
-- 
1.7.4.4

[Qemu-devel] [PATCH spice-server 02/13] server: handle migration interface addition

[Yonit Halperin]

Signed-off-by: Yonit Halperin <yhalperi@redhat.com>
---
 server/reds.c |   29 +++++++++++++++++++++++++++++
 server/reds.h |    4 ++++
 2 files changed, 33 insertions(+), 0 deletions(-)

diff --git a/server/reds.c b/server/reds.c
index 9a983f8..99d52f9 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -73,6 +73,7 @@ static SpiceKbdInstance *keyboard = NULL;
 static SpiceMouseInstance *mouse = NULL;
 static SpiceTabletInstance *tablet = NULL;
 static SpiceCharDeviceInstance *vdagent = NULL;
+static SpiceMigrateInstance *migration_interface = NULL;
 
 #define MIGRATION_NOTIFY_SPICE_KEY "spice_mig_ext"
 
@@ -4345,6 +4346,20 @@ SPICE_GNUC_VISIBLE int spice_server_add_interface(SpiceServer *s,
         red_printf("unsupported net wire interface");
         return -1;
 #endif
+    } else if (strcmp(interface->type, SPICE_INTERFACE_MIGRATION) == 0) {
+        red_printf("SPICE_INTERFACE_MIGRATION");
+        if (migration_interface) {
+            red_printf("already have migration");
+            return -1;
+        }
+
+        if (interface->major_version != SPICE_INTERFACE_MIGRATION_MAJOR ||
+            interface->minor_version > SPICE_INTERFACE_MIGRATION_MINOR) {
+            red_printf("unsupported migration interface");
+            return -1;
+        }
+        migration_interface = SPICE_CONTAINEROF(sin, SpiceMigrateInstance, base);
+        migration_interface->st = spice_new0(SpiceMigrateState, 1);
     }
 
     return 0;
@@ -4859,7 +4874,15 @@ SPICE_GNUC_VISIBLE int spice_server_migrate_connect(SpiceServer *s, const char*
                                                     int port, int secure_port,
                                                     const char* cert_subject)
 {
+    SpiceMigrateInterface *sif;
+    red_printf("");
+    ASSERT(migration_interface);
+    ASSERT(reds == s);
+
     red_printf("not implemented yet");
+    sif = SPICE_CONTAINEROF(migration_interface->base.sif, SpiceMigrateInterface, base);
+    sif->migrate_connect_complete(migration_interface);
+
     return 0;
 }
 
@@ -4920,8 +4943,14 @@ SPICE_GNUC_VISIBLE int spice_server_migrate_client_state(SpiceServer *s)
 
 SPICE_GNUC_VISIBLE int spice_server_migrate_end(SpiceServer *s, int completed)
 {
+    SpiceMigrateInterface *sif;
+    ASSERT(migration_interface);
     ASSERT(reds == s);
     reds_mig_finished(completed);
+    sif = SPICE_CONTAINEROF(migration_interface->base.sif, SpiceMigrateInterface, base);
+    if (sif->migrate_end_complete) {
+        sif->migrate_end_complete(migration_interface);
+    }
     return 0;
 }
 
diff --git a/server/reds.h b/server/reds.h
index 463c94f..b60681a 100644
--- a/server/reds.h
+++ b/server/reds.h
@@ -119,6 +119,10 @@ struct SpiceNetWireState {
     struct TunnelWorker *worker;
 };
 
+struct SpiceMigrateState {
+    int dummy;
+};
+
 void reds_channel_dispose(Channel *channel);
 
 ssize_t reds_stream_read(RedsStream *s, void *buf, size_t nbyte);
-- 
1.7.4.4

[Qemu-devel] [PATCH spice-server 03/13] configure: spice-protocol >= 0.8.2 (semi-seamless migration protocol)

[Yonit Halperin]

Signed-off-by: Yonit Halperin <yhalperi@redhat.com>
---
 configure.ac |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/configure.ac b/configure.ac
index 3a86515..e169f36 100644
--- a/configure.ac
+++ b/configure.ac
@@ -126,7 +126,7 @@ fi
 dnl =========================================================================
 dnl Check deps
 
-PKG_CHECK_MODULES(PROTOCOL, spice-protocol >= 0.8.1)
+PKG_CHECK_MODULES(PROTOCOL, spice-protocol >= 0.8.2)
 AC_SUBST(PROTOCOL_CFLAGS)
 
 AC_CHECK_LIBM
-- 
1.7.4.4

[Qemu-devel] [PATCH spice-server 04/13] server, proto: tell the client to connect to the migration target before migraton starts

[Yonit Halperin]

(1) send SPICE_MSG_MAIN_MIGRATE_BEGIN upon spice_server_migrate_connect
(2) wait for SPICE_MSGC_MAIN_MIGRATE_(CONNECTED|CONNECT_ERROR), or a timeout, in order
    to complete client_migrate_info monitor command

Signed-off-by: Yonit Halperin <yhalperi@redhat.com>
---
 common/messages.h |    2 +
 server/reds.c     |  148 +++++++++++++++++++++++++++++++++++++++-------------
 spice.proto       |    5 +-
 3 files changed, 115 insertions(+), 40 deletions(-)

diff --git a/common/messages.h b/common/messages.h
index 6fcd8be..16ae05b 100644
--- a/common/messages.h
+++ b/common/messages.h
@@ -66,6 +66,8 @@ typedef struct SpiceMsgMainMigrationBegin {
     uint16_t pub_key_type;
     uint32_t pub_key_size;
     uint8_t *pub_key_data;
+    uint32_t cert_subject_size;
+    uint8_t *cert_subject_data;
 } SpiceMsgMainMigrationBegin;
 
 typedef struct SpiceMsgMainMigrationSwitchHost {
diff --git a/server/reds.c b/server/reds.c
index 99d52f9..845b0ee 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -271,6 +271,7 @@ typedef struct RedsState {
     VDIPortState agent_state;
     InputsState *inputs_state;
 
+    int client_semi_mig_cap;
     int mig_wait_connect;
     int mig_wait_disconnect;
     int mig_inprogress;
@@ -280,6 +281,7 @@ typedef struct RedsState {
     IncomingHandler in_handler;
     RedsOutgoingData outgoing;
     Channel *channels;
+    Channel main_channel;
     int mouse_mode;
     int is_client_mouse_allowed;
     int dispatcher_allows_client_mouse;
@@ -378,6 +380,7 @@ static uint8_t zero_page[ZERO_BUF_SIZE] = {0};
 
 static void reds_push();
 static void reds_out_item_free(RedsOutItem *item);
+static void migrate_timeout(void *opaque);
 
 static ChannelSecurityOptions *channels_security = NULL;
 static int default_channel_security =
@@ -644,6 +647,12 @@ static void reds_shatdown_channels()
 static void reds_mig_cleanup()
 {
     if (reds->mig_inprogress) {
+        if (reds->mig_wait_connect) {
+            SpiceMigrateInterface *sif;
+            ASSERT(migration_interface);
+            sif = SPICE_CONTAINEROF(migration_interface->base.sif, SpiceMigrateInterface, base);
+            sif->migrate_connect_complete(migration_interface);
+        }
         reds->mig_inprogress = FALSE;
         reds->mig_wait_connect = FALSE;
         reds->mig_wait_disconnect = FALSE;
@@ -1700,7 +1709,7 @@ static void reds_main_handle_message(void *opaque, size_t size, uint32_t type, v
         reds_send_channels();
         break;
     case SPICE_MSGC_MAIN_MIGRATE_CONNECTED:
-        red_printf("connected");
+        red_printf("client connected to migration target");
         if (reds->mig_wait_connect) {
             reds_mig_cleanup();
         }
@@ -1891,6 +1900,23 @@ static int sync_write(RedsStream *stream, const void *in_buf, size_t n)
     return TRUE;
 }
 
+static void reds_channel_set_caps(Channel *channel, int cap, int active)
+{
+    int nbefore, n;
+
+    nbefore = channel->num_caps;
+    n = cap / 32;
+    channel->num_caps = MAX(channel->num_caps, n + 1);
+    channel->caps = spice_renew(uint32_t, channel->caps, channel->num_caps);
+    memset(channel->caps + nbefore, 0,
+           (channel->num_caps - nbefore) * sizeof(uint32_t));
+    if (active) {
+        channel->caps[n] |= (1 << cap);
+    } else {
+        channel->caps[n] &= ~(1 << cap);
+    }
+}
+
 static void reds_channel_set_common_caps(Channel *channel, int cap, int active)
 {
     int nbefore, n;
@@ -1933,7 +1959,6 @@ static int reds_send_link_ack(RedLinkInfo *link)
 {
     SpiceLinkHeader header;
     SpiceLinkReply ack;
-    Channel caps = { 0, };
     Channel *channel;
     BUF_MEM *bmBuf;
     BIO *bio;
@@ -1948,7 +1973,8 @@ static int reds_send_link_ack(RedLinkInfo *link)
 
     channel = reds_find_channel(link->link_mess->channel_type, 0);
     if (!channel) {
-        channel = &caps;
+        ASSERT(link->link_mess->channel_type == SPICE_CHANNEL_MAIN);
+        channel = &reds->main_channel;
     }
 
     reds_channel_init_auth_caps(channel); /* make sure common caps are set */
@@ -1989,7 +2015,6 @@ static int reds_send_link_ack(RedLinkInfo *link)
     ret = TRUE;
 
 end:
-    reds_channel_dispose(&caps);
     BIO_free(bio);
     return ret;
 }
@@ -2042,28 +2067,41 @@ static void reds_start_net_test()
     }
 }
 
+static int test_capability(uint32_t *caps, uint32_t num_caps, uint32_t cap)
+{
+    uint32_t index = cap / 32;
+    if (num_caps < index + 1) {
+        return FALSE;
+    }
+
+    return (caps[index] & (1 << (cap % 32))) != 0;
+}
+
 static void reds_handle_main_link(RedLinkInfo *link)
 {
+    SpiceLinkMess *link_mess = link->link_mess;
     uint32_t connection_id;
+    uint32_t *channel_caps;
+    uint32_t num_channel_caps;
 
     red_printf("");
 
     reds_disconnect();
 
-    if (!link->link_mess->connection_id) {
+    if (!link_mess->connection_id) {
         reds_send_link_result(link, SPICE_LINK_ERR_OK);
         while((connection_id = rand()) == 0);
         reds->agent_state.num_tokens = 0;
         memcpy(&(reds->taTicket), &taTicket, sizeof(reds->taTicket));
         reds->mig_target = FALSE;
     } else {
-        if (link->link_mess->connection_id != reds->link_id) {
+        if (link_mess->connection_id != reds->link_id) {
             reds_send_link_result(link, SPICE_LINK_ERR_BAD_CONNECTION_ID);
             reds_link_free(link);
             return;
         }
         reds_send_link_result(link, SPICE_LINK_ERR_OK);
-        connection_id = link->link_mess->connection_id;
+        connection_id = link_mess->connection_id;
         reds->mig_target = TRUE;
     }
 
@@ -2074,6 +2112,17 @@ static void reds_handle_main_link(RedLinkInfo *link)
     reds->stream = link->stream;
     reds->in_handler.shut = FALSE;
 
+    num_channel_caps = link_mess->num_channel_caps;
+    channel_caps = num_channel_caps ? (uint32_t *)((uint8_t *)link_mess +
+                                                    link_mess->caps_offset) + link_mess->num_common_caps :
+                                      NULL;
+    reds->client_semi_mig_cap = test_capability(channel_caps, num_channel_caps, SPICE_MAIN_CAP_SEMI_SEAMLESS_MIGRATE);
+    if (reds->client_semi_mig_cap && (SPICE_VERSION_MAJOR == 2) && (reds->peer_minor_version < 1)) {
+        red_printf("warning: client claims to support semi seamless migration,"
+                   "but its version is incompatible");
+        reds->client_semi_mig_cap = FALSE;
+    }
+
     reds_show_new_channel(link, connection_id);
     reds_stream_remove_watch(link->stream);
     link->stream = NULL;
@@ -2534,6 +2583,12 @@ static void openssl_init(RedLinkInfo *link)
     BN_set_word(link->tiTicketing.bn, f4);
 }
 
+static void main_init()
+{
+    reds->main_channel.type = SPICE_CHANNEL_MAIN;
+    reds_channel_set_caps(&reds->main_channel, SPICE_MAIN_CAP_SEMI_SEAMLESS_MIGRATE, TRUE);
+}
+
 static void inputs_init()
 {
     Channel *channel;
@@ -3926,15 +3981,11 @@ static void set_one_channel_security(int id, uint32_t security)
 #define REDS_SAVE_VERSION 1
 
 struct RedsMigSpice {
-    char pub_key[SPICE_TICKET_PUBKEY_BYTES];
     uint32_t mig_key;
     char *host;
     char *cert_subject;
     int port;
     int sport;
-    uint16_t cert_pub_key_type;
-    uint32_t cert_pub_key_len;
-    uint8_t* cert_pub_key;
 };
 
 typedef struct RedsMigSpiceMessage {
@@ -3963,15 +4014,20 @@ static void reds_mig_continue(void)
     RedsOutItem *item;
 
     red_printf("");
+    ASSERT(reds->client_semi_mig_cap);
     item = new_out_item(SPICE_MSG_MAIN_MIGRATE_BEGIN);
 
     migrate.port = s->port;
     migrate.sport = s->sport;
     migrate.host_size = strlen(s->host) + 1;
     migrate.host_data = (uint8_t *)s->host;
-    migrate.pub_key_type = s->cert_pub_key_type;
-    migrate.pub_key_size = s->cert_pub_key_len;
-    migrate.pub_key_data = s->cert_pub_key;
+    if (s->cert_subject) {
+        migrate.cert_subject_size = strlen(s->cert_subject) + 1;
+        migrate.cert_subject_data = (uint8_t *)s->cert_subject;
+    } else {
+        migrate.cert_subject_size = 0;
+        migrate.cert_subject_data = NULL;
+    }
     spice_marshall_msg_main_migrate_begin(item->m, &migrate);
 
     reds_push_pipe_item(item);
@@ -3985,6 +4041,7 @@ static void reds_mig_continue(void)
 static void reds_mig_started(void)
 {
     red_printf("");
+    ASSERT(reds->mig_spice);
 
     reds->mig_inprogress = TRUE;
 
@@ -4001,12 +4058,6 @@ static void reds_mig_started(void)
         goto error;
     }
 
-    if ((SPICE_VERSION_MAJOR == 1) && (reds->peer_minor_version < 2)) {
-        red_printf("minor version mismatch client %u server %u",
-                   reds->peer_minor_version, SPICE_VERSION_MINOR);
-        goto error;
-    }
-
     reds_mig_continue();
     return;
 
@@ -4089,7 +4140,7 @@ static void reds_mig_switch(void)
     reds_mig_release();
 }
 
-static void migrate_timout(void *opaque)
+static void migrate_timeout(void *opaque)
 {
     red_printf("");
     ASSERT(reds->mig_wait_connect || reds->mig_wait_disconnect);
@@ -4477,7 +4528,7 @@ static int do_spice_init(SpiceCoreInterface *core_interface)
 
     init_vd_agent_resources();
 
-    if (!(reds->mig_timer = core->timer_add(migrate_timout, NULL))) {
+    if (!(reds->mig_timer = core->timer_add(migrate_timeout, NULL))) {
         red_error("migration timer create failed");
     }
     if (!(reds->key_modifiers_timer = core->timer_add(key_modifiers_sender, NULL))) {
@@ -4540,6 +4591,7 @@ static int do_spice_init(SpiceCoreInterface *core_interface)
     }
 #endif
 
+    main_init();
     inputs_init();
 
     reds->mouse_mode = SPICE_MOUSE_MODE_SERVER;
@@ -4869,6 +4921,31 @@ SPICE_GNUC_VISIBLE int spice_server_set_agent_copypaste(SpiceServer *s, int enab
     return 0;
 }
 
+/* returns FALSE if info is invalid */
+static int reds_set_migration_dest_info(const char* dest,
+                                        int port, int secure_port,
+                                        const char* cert_subject)
+{
+    RedsMigSpice *spice_migration = NULL;
+
+    reds_mig_release();
+    if ((port == -1 && secure_port == -1) || !dest) {
+        return FALSE;
+    }
+
+    spice_migration = spice_new0(RedsMigSpice, 1);
+    spice_migration->port = port;
+    spice_migration->sport = secure_port;
+    spice_migration->host = strdup(dest);
+    if (cert_subject) {
+        spice_migration->cert_subject = strdup(cert_subject);
+    }
+
+    reds->mig_spice = spice_migration;
+
+    return TRUE;
+}
+
 /* semi-seamless client migration */
 SPICE_GNUC_VISIBLE int spice_server_migrate_connect(SpiceServer *s, const char* dest,
                                                     int port, int secure_port,
@@ -4879,10 +4956,18 @@ SPICE_GNUC_VISIBLE int spice_server_migrate_connect(SpiceServer *s, const char*
     ASSERT(migration_interface);
     ASSERT(reds == s);
 
-    red_printf("not implemented yet");
     sif = SPICE_CONTAINEROF(migration_interface->base.sif, SpiceMigrateInterface, base);
-    sif->migrate_connect_complete(migration_interface);
 
+    if (!reds_set_migration_dest_info(dest, port, secure_port, cert_subject)) {
+        sif->migrate_connect_complete(migration_interface);
+        return -1;
+    }
+
+    if (reds->client_semi_mig_cap) {
+        reds_mig_started();
+    } else {
+        sif->migrate_connect_complete(migration_interface);
+    }
     return 0;
 }
 
@@ -4890,27 +4975,16 @@ SPICE_GNUC_VISIBLE int spice_server_migrate_info(SpiceServer *s, const char* des
                                           int port, int secure_port,
                                           const char* cert_subject)
 {
-    RedsMigSpice *spice_migration = NULL;
-
+    ASSERT(!migration_interface);
     ASSERT(reds == s);
 
-    if ((port == -1 && secure_port == -1) || !dest)
+    if (!reds_set_migration_dest_info(dest, port, secure_port, cert_subject)) {
         return -1;
-
-    spice_migration = spice_new0(RedsMigSpice, 1);
-    spice_migration->port = port;
-    spice_migration->sport = secure_port;
-    spice_migration->host = strdup(dest);
-    if (cert_subject) {
-        spice_migration->cert_subject = strdup(cert_subject);
     }
 
-    reds_mig_release();
-    reds->mig_spice = spice_migration;
     return 0;
 }
 
-/* interface for seamless migration */
 SPICE_GNUC_VISIBLE int spice_server_migrate_start(SpiceServer *s)
 {
     ASSERT(reds == s);
diff --git a/spice.proto b/spice.proto
index 6160de1..d5b954e 100644
--- a/spice.proto
+++ b/spice.proto
@@ -167,9 +167,8 @@ channel MainChannel : BaseChannel {
 	uint16 sport;
 	uint32 host_size;
 	uint8 *host_data[host_size] @zero_terminated @marshall @nonnull;
-	pubkey_type pub_key_type;
-	uint32 pub_key_size;
-	uint8 *pub_key_data[pub_key_size] @zero_terminated  @marshall @nonnull;
+	uint32 cert_subject_size;
+	uint8 *cert_subject_data[cert_subject_size] @zero_terminated @marshall;
     } @ctype(SpiceMsgMainMigrationBegin) migrate_begin = 101;
 
     Empty migrate_cancel;
-- 
1.7.4.4

[Qemu-devel] [PATCH spice-server 05/13] spice.proto: add SPICE_MSG_MAIN_MIGRATE_END & SPICE_MSGC_MAIN_MIGRATE_END

[Yonit Halperin]

Signed-off-by: Yonit Halperin <yhalperi@redhat.com>
---
 spice.proto |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/spice.proto b/spice.proto
index d5b954e..235ec95 100644
--- a/spice.proto
+++ b/spice.proto
@@ -219,6 +219,8 @@ channel MainChannel : BaseChannel {
       uint8 *cert_subject_data[cert_subject_size] @zero_terminated  @marshall;
     } @ctype(SpiceMsgMainMigrationSwitchHost) migrate_switch_host;
 
+    Empty migrate_end;
+
  client:
     message {
 	uint64 cache_size;
@@ -243,6 +245,8 @@ channel MainChannel : BaseChannel {
     message {
         uint32 num_tokens;
     } @ctype(SpiceMsgcMainAgentTokens) agent_token;
+
+    Empty migrate_end;
 };
 
 enum8 clip_type {
-- 
1.7.4.4

[Qemu-devel] [PATCH spice-server 06/13] server: send SPICE_MSG_MAIN_MIGRATE_END on spice_server_migrate_end

[Yonit Halperin]

Signed-off-by: Yonit Halperin <yhalperi@redhat.com>
---
 server/reds.c |   86 +++++++++++++++++++++++++++++++++++++++++++++------------
 1 files changed, 68 insertions(+), 18 deletions(-)

diff --git a/server/reds.c b/server/reds.c
index 845b0ee..e088b08 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -275,6 +275,7 @@ typedef struct RedsState {
     int mig_wait_connect;
     int mig_wait_disconnect;
     int mig_inprogress;
+    int expect_migrate;
     int mig_target;
     RedsMigSpice *mig_spice;
     int num_of_channels;
@@ -4038,13 +4039,21 @@ static void reds_mig_continue(void)
     core->timer_start(reds->mig_timer, MIGRATE_TIMEOUT);
 }
 
-static void reds_mig_started(void)
+static void reds_listen_start(void)
 {
-    red_printf("");
-    ASSERT(reds->mig_spice);
+    ASSERT(reds);
+    if (reds->listen_watch != NULL) {
+        core->watch_update_mask(reds->listen_watch, SPICE_WATCH_EVENT_READ);
+    }
 
-    reds->mig_inprogress = TRUE;
+    if (reds->secure_listen_watch != NULL) {
+        core->watch_update_mask(reds->secure_listen_watch, SPICE_WATCH_EVENT_READ);
+    }
+}
 
+static void reds_listen_stop(void)
+{
+    ASSERT(reds);
     if (reds->listen_watch != NULL) {
         core->watch_update_mask(reds->listen_watch, 0);
     }
@@ -4052,6 +4061,14 @@ static void reds_mig_started(void)
     if (reds->secure_listen_watch != NULL) {
         core->watch_update_mask(reds->secure_listen_watch, 0);
     }
+}
+
+static void reds_mig_started(void)
+{
+    red_printf("");
+    ASSERT(reds->mig_spice);
+
+    reds->mig_inprogress = TRUE;
 
     if (reds->stream == NULL) {
         red_printf("not connected to stream");
@@ -4071,13 +4088,6 @@ static void reds_mig_finished(int completed)
     RedsOutItem *item;
 
     red_printf("");
-    if (reds->listen_watch != NULL) {
-        core->watch_update_mask(reds->listen_watch, SPICE_WATCH_EVENT_READ);
-    }
-
-    if (reds->secure_listen_watch != NULL) {
-        core->watch_update_mask(reds->secure_listen_watch, SPICE_WATCH_EVENT_READ);
-    }
 
     if (reds->stream == NULL) {
         red_printf("no stream connected");
@@ -4086,22 +4096,25 @@ static void reds_mig_finished(int completed)
     reds->mig_inprogress = TRUE;
 
     if (completed) {
+#ifdef SPICE_SEAMLESS_MIGRATION
         Channel *channel;
         SpiceMsgMigrate migrate;
 
-        reds->mig_wait_disconnect = TRUE;
-        core->timer_start(reds->mig_timer, MIGRATE_TIMEOUT);
-
         item = new_out_item(SPICE_MSG_MIGRATE);
         migrate.flags = SPICE_MIGRATE_NEED_FLUSH | SPICE_MIGRATE_NEED_DATA_TRANSFER;
         spice_marshall_msg_migrate(item->m, &migrate);
-
         reds_push_pipe_item(item);
         channel = reds->channels;
         while (channel) {
             channel->migrate(channel);
             channel = channel->next;
         }
+#else
+        item = new_out_item(SPICE_MSG_MAIN_MIGRATE_END);
+        reds_push_pipe_item(item);
+#endif
+        reds->mig_wait_disconnect = TRUE;
+        core->timer_start(reds->mig_timer, MIGRATE_TIMEOUT);
     } else {
         item = new_out_item(SPICE_MSG_MAIN_MIGRATE_CANCEL);
         reds_push_pipe_item(item);
@@ -4116,6 +4129,8 @@ static void reds_mig_switch(void)
     RedsOutItem *item;
 
     if (s == NULL) {
+        red_printf("warning: migration target was not set. disconnecting client");
+        reds_disconnect();
         return;
     }
 
@@ -4956,6 +4971,12 @@ SPICE_GNUC_VISIBLE int spice_server_migrate_connect(SpiceServer *s, const char*
     ASSERT(migration_interface);
     ASSERT(reds == s);
 
+
+    if (reds->expect_migrate && reds->client_semi_mig_cap) {
+        red_printf("warning: consecutive calls without migration. Canceling previous call");
+        reds_mig_finished(FALSE);
+    }
+
     sif = SPICE_CONTAINEROF(migration_interface->base.sif, SpiceMigrateInterface, base);
 
     if (!reds_set_migration_dest_info(dest, port, secure_port, cert_subject)) {
@@ -4963,6 +4984,9 @@ SPICE_GNUC_VISIBLE int spice_server_migrate_connect(SpiceServer *s, const char*
         return -1;
     }
 
+    reds->expect_migrate = TRUE;
+    reds_listen_stop();
+
     if (reds->client_semi_mig_cap) {
         reds_mig_started();
     } else {
@@ -4975,13 +4999,13 @@ SPICE_GNUC_VISIBLE int spice_server_migrate_info(SpiceServer *s, const char* des
                                           int port, int secure_port,
                                           const char* cert_subject)
 {
+    red_printf("");
     ASSERT(!migration_interface);
     ASSERT(reds == s);
 
     if (!reds_set_migration_dest_info(dest, port, secure_port, cert_subject)) {
         return -1;
     }
-
     return 0;
 }
 
@@ -5018,20 +5042,46 @@ SPICE_GNUC_VISIBLE int spice_server_migrate_client_state(SpiceServer *s)
 SPICE_GNUC_VISIBLE int spice_server_migrate_end(SpiceServer *s, int completed)
 {
     SpiceMigrateInterface *sif;
+    int ret = 0;
+
+    red_printf("");
+
     ASSERT(migration_interface);
     ASSERT(reds == s);
-    reds_mig_finished(completed);
+
+    reds_listen_start();
     sif = SPICE_CONTAINEROF(migration_interface->base.sif, SpiceMigrateInterface, base);
+    if (!reds->expect_migrate && reds->stream) {
+        red_printf("spice_server_migrate_info was not called, disconnecting client");
+        reds_disconnect("");
+        ret = -1;
+        goto complete;
+    }
+
+    reds->expect_migrate = FALSE;
+    if (reds->client_semi_mig_cap) {
+        reds_mig_finished(completed);
+    } else {
+        ret = spice_server_migrate_switch(s);
+        goto complete;
+    }
+    ret = 0;
+complete:
     if (sif->migrate_end_complete) {
         sif->migrate_end_complete(migration_interface);
     }
-    return 0;
+    return ret;
 }
 
 /* interface for switch-host migration */
 SPICE_GNUC_VISIBLE int spice_server_migrate_switch(SpiceServer *s)
 {
     ASSERT(reds == s);
+    red_printf("");
+    if (!reds->stream) {
+       return 0;
+    }
+    reds->expect_migrate = FALSE;
     reds_mig_switch();
     return 0;
 }
-- 
1.7.4.4

[Qemu-devel] [PATCH spice-server 07/13] server: move SPICE_MSG_MAIN_INIT sending code to a separate routine

[Yonit Halperin]

Signed-off-by: Yonit Halperin <yhalperi@redhat.com>
---
 server/reds.c |   56 +++++++++++++++++++++++++++++++++-----------------------
 1 files changed, 33 insertions(+), 23 deletions(-)

diff --git a/server/reds.c b/server/reds.c
index e088b08..bea0eb0 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -2078,6 +2078,35 @@ static int test_capability(uint32_t *caps, uint32_t num_caps, uint32_t cap)
     return (caps[index] & (1 << (cap % 32))) != 0;
 }
 
+static void reds_main_channel_init(int do_net_test)
+{
+    RedsOutItem *item;
+    SpiceMsgMainInit init;
+
+    item = new_out_item(SPICE_MSG_MAIN_INIT);
+    init.session_id = reds->link_id;
+    init.display_channels_hint = red_dispatcher_count();
+    init.current_mouse_mode = reds->mouse_mode;
+    init.supported_mouse_modes = SPICE_MOUSE_MODE_SERVER;
+    if (reds->is_client_mouse_allowed) {
+        init.supported_mouse_modes |= SPICE_MOUSE_MODE_CLIENT;
+    }
+    init.agent_connected = !!vdagent;
+    init.agent_tokens = REDS_AGENT_WINDOW_SIZE;
+    reds->agent_state.num_client_tokens = REDS_AGENT_WINDOW_SIZE;
+    init.multi_media_time = reds_get_mm_time() - MM_TIME_DELTA;
+    init.ram_hint = red_dispatcher_qxl_ram_size();
+
+    spice_marshall_msg_main_init(item->m, &init);
+
+    reds_push_pipe_item(item);
+    if (do_net_test) {
+        reds_start_net_test();
+    }
+    /* Now that we have a client, forward any pending agent data */
+    while (read_from_vdi_port());
+}
+
 static void reds_handle_main_link(RedLinkInfo *link)
 {
     SpiceLinkMess *link_mess = link->link_mess;
@@ -2144,29 +2173,10 @@ static void reds_handle_main_link(RedLinkInfo *link)
                                         reds_main_event, NULL);
 
     if (!reds->mig_target) {
-        RedsOutItem *item;
-        SpiceMsgMainInit init;
-
-        item = new_out_item(SPICE_MSG_MAIN_INIT);
-        init.session_id = connection_id;
-        init.display_channels_hint = red_dispatcher_count();
-        init.current_mouse_mode = reds->mouse_mode;
-        init.supported_mouse_modes = SPICE_MOUSE_MODE_SERVER;
-        if (reds->is_client_mouse_allowed) {
-            init.supported_mouse_modes |= SPICE_MOUSE_MODE_CLIENT;
-        }
-        init.agent_connected = !!vdagent;
-        init.agent_tokens = REDS_AGENT_WINDOW_SIZE;
-        reds->agent_state.num_client_tokens = REDS_AGENT_WINDOW_SIZE;
-        init.multi_media_time = reds_get_mm_time() - MM_TIME_DELTA;
-        init.ram_hint = red_dispatcher_qxl_ram_size();
-
-        spice_marshall_msg_main_init(item->m, &init);
-
-        reds_push_pipe_item(item);
-        reds_start_net_test();
-        /* Now that we have a client, forward any pending agent data */
-        while (read_from_vdi_port());
+        reds_main_channel_init(TRUE);
+    }
+    else {
+        ASSERT(reds->client_semi_mig_cap);
     }
 }
 
-- 
1.7.4.4

[Qemu-devel] [PATCH spice-server 08/13] server: move the linking of channels to a separate routine

[Yonit Halperin]

Signed-off-by: Yonit Halperin <yhalperi@redhat.com>
---
 server/reds.c |   68 +++++++++++++++++++++++++++++++++++---------------------
 1 files changed, 42 insertions(+), 26 deletions(-)

diff --git a/server/reds.c b/server/reds.c
index bea0eb0..e7388a0 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -2612,12 +2612,48 @@ static void inputs_init()
     reds_register_channel(channel);
 }
 
+static void reds_send_input_channel_insecure_warn()
+{
+    RedsOutItem *item;
+    SpiceMsgNotify notify;
+    char *mess = "keyboard channel is insecure";
+    const int mess_len = strlen(mess);
+
+    item = new_out_item(SPICE_MSG_NOTIFY);
+
+    notify.time_stamp = get_time_stamp();
+    notify.severity = SPICE_NOTIFY_SEVERITY_WARN;
+    notify.visibilty = SPICE_NOTIFY_VISIBILITY_HIGH;
+    notify.what = SPICE_WARN_GENERAL;
+    notify.message_len = mess_len;
+
+    spice_marshall_msg_notify(item->m, &notify);
+    spice_marshaller_add(item->m, (uint8_t *)mess, mess_len + 1);
+
+    reds_push_pipe_item(item);
+}
+
+static void reds_channel_do_link(Channel *channel, SpiceLinkMess *link_msg, RedsStream *stream)
+{
+    uint32_t *caps;
+
+    ASSERT(channel);
+    ASSERT(link_msg);
+    ASSERT(stream);
+
+    if (link_msg->channel_type == SPICE_CHANNEL_INPUTS && !stream->ssl) {
+        reds_send_input_channel_insecure_warn();
+    }
+    caps = (uint32_t *)((uint8_t *)link_msg + link_msg->caps_offset);
+    channel->link(channel, stream, FALSE, link_msg->num_common_caps,
+                  link_msg->num_common_caps ? caps : NULL, link_msg->num_channel_caps,
+                  link_msg->num_channel_caps ? caps + link_msg->num_common_caps : NULL);
+}
+
 static void reds_handle_other_links(RedLinkInfo *link)
 {
     Channel *channel;
-    RedsStream *stream;
     SpiceLinkMess *link_mess;
-    uint32_t *caps;
 
     link_mess = link->link_mess;
 
@@ -2635,36 +2671,16 @@ static void reds_handle_other_links(RedLinkInfo *link)
     }
 
     reds_send_link_result(link, SPICE_LINK_ERR_OK);
+    // TODO: if mig_target, where this call should be? here or only when the links are handled
     reds_show_new_channel(link, reds->link_id);
-    if (link_mess->channel_type == SPICE_CHANNEL_INPUTS && !link->stream->ssl) {
-        RedsOutItem *item;
-        SpiceMsgNotify notify;
-        char *mess = "keyboard channel is insecure";
-        const int mess_len = strlen(mess);
-
-        item = new_out_item(SPICE_MSG_NOTIFY);
-
-        notify.time_stamp = get_time_stamp();
-        notify.severity = SPICE_NOTIFY_SEVERITY_WARN;
-        notify.visibilty = SPICE_NOTIFY_VISIBILITY_HIGH;
-        notify.what = SPICE_WARN_GENERAL;
-        notify.message_len = mess_len;
+    reds_stream_remove_watch(link->stream);
 
-        spice_marshall_msg_notify(item->m, &notify);
-        spice_marshaller_add(item->m, (uint8_t *)mess, mess_len + 1);
+    reds_channel_do_link(channel, link->link_mess, link->stream);
+    free(link_mess);
 
-        reds_push_pipe_item(item);
-    }
-    stream = link->stream;
-    reds_stream_remove_watch(stream);
     link->stream = NULL;
     link->link_mess = NULL;
     reds_link_free(link);
-    caps = (uint32_t *)((uint8_t *)link_mess + link_mess->caps_offset);
-    channel->link(channel, stream, reds->mig_target, link_mess->num_common_caps,
-                  link_mess->num_common_caps ? caps : NULL, link_mess->num_channel_caps,
-                  link_mess->num_channel_caps ? caps + link_mess->num_common_caps : NULL);
-    free(link_mess);
 }
 
 static void reds_handle_link(RedLinkInfo *link)
-- 
1.7.4.4

[Qemu-devel] [PATCH spice-server 09/13] server: handling semi-seamless migration in the target side

[Yonit Halperin]

(1) not sending anything to the client till we recieve SPICE_MSGC_MIGRATE_END
(2) start a new migration (handle client_migrate_info) only after SPICE_MSGC_MIGRATE_END
    from the previous migration has been received
(3) use the correct ticket

Signed-off-by: Yonit Halperin <yhalperi@redhat.com>
---
 server/reds.c |  137 ++++++++++++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 116 insertions(+), 21 deletions(-)

diff --git a/server/reds.c b/server/reds.c
index e7388a0..ca4e1d1 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -258,6 +258,13 @@ typedef struct RedsStatValue {
 #endif
 
 typedef struct RedsMigSpice RedsMigSpice;
+typedef struct RedsMigPendingLink RedsMigPendingLink;
+
+struct RedsMigPendingLink {
+    RedsMigPendingLink *next;
+    SpiceLinkMess *link_msg;
+    RedsStream *stream;
+};
 
 typedef struct RedsState {
     int listen_socket;
@@ -274,10 +281,13 @@ typedef struct RedsState {
     int client_semi_mig_cap;
     int mig_wait_connect;
     int mig_wait_disconnect;
+    int mig_wait_prev_complete;
     int mig_inprogress;
     int expect_migrate;
     int mig_target;
     RedsMigSpice *mig_spice;
+    RedsMigPendingLink *mig_pending_links;
+    int num_mig_links;
     int num_of_channels;
     IncomingHandler in_handler;
     RedsOutgoingData outgoing;
@@ -293,7 +303,6 @@ typedef struct RedsState {
     SpiceTimer *vdi_port_write_timer;
     int vdi_port_write_timer_started;
 
-    TicketAuthentication taTicket;
     SSL_CTX *ctx;
 
 #ifdef RED_STATISTICS
@@ -382,6 +391,9 @@ static uint8_t zero_page[ZERO_BUF_SIZE] = {0};
 static void reds_push();
 static void reds_out_item_free(RedsOutItem *item);
 static void migrate_timeout(void *opaque);
+static void reds_mig_pending_links_free(void);
+static void reds_handle_client_migrate_complete(void);
+static void reds_mig_started(void);
 
 static ChannelSecurityOptions *channels_security = NULL;
 static int default_channel_security =
@@ -648,7 +660,7 @@ static void reds_shatdown_channels()
 static void reds_mig_cleanup()
 {
     if (reds->mig_inprogress) {
-        if (reds->mig_wait_connect) {
+        if (reds->mig_wait_connect || reds->mig_wait_prev_complete) {
             SpiceMigrateInterface *sif;
             ASSERT(migration_interface);
             sif = SPICE_CONTAINEROF(migration_interface->base.sif, SpiceMigrateInterface, base);
@@ -657,8 +669,13 @@ static void reds_mig_cleanup()
         reds->mig_inprogress = FALSE;
         reds->mig_wait_connect = FALSE;
         reds->mig_wait_disconnect = FALSE;
+        reds->mig_wait_prev_complete = FALSE;
         core->timer_cancel(reds->mig_timer);
     }
+    if (reds->num_mig_links) {
+        ASSERT(reds->mig_target);
+        reds_mig_pending_links_free();
+    }
 }
 
 static void reds_reset_vdp()
@@ -925,6 +942,11 @@ static void reds_send_channels()
     Channel *channel;
     int i;
 
+    if (reds->mig_target) {
+        red_printf("warning: unexpected SPICE_MSGC_MAIN_ATTACH_CHANNELS during migration");
+        return;
+    }
+
     item = new_out_item(SPICE_MSG_MAIN_CHANNELS_LIST);
     channels_info = (SpiceMsgChannels *)spice_malloc(sizeof(SpiceMsgChannels) + reds->num_of_channels * sizeof(SpiceChannelId));
     channels_info->num_of_channels = reds->num_of_channels;
@@ -1008,7 +1030,7 @@ static void reds_send_mouse_mode()
     SpiceMsgMainMouseMode mouse_mode;
     RedsOutItem *item;
 
-    if (!reds->stream) {
+    if (!reds->stream || reds->mig_target) {
         return;
     }
 
@@ -1077,6 +1099,7 @@ static void reds_agent_remove()
     SpiceCharDeviceInstance *sin = vdagent;
     SpiceCharDeviceInterface *sif;
 
+    // TODO: is this cond needed
     if (!reds->mig_target) {
         reds_reset_vdp();
     }
@@ -1103,7 +1126,7 @@ static void reds_send_tokens()
     SpiceMsgMainAgentTokens tokens;
     RedsOutItem *item;
 
-    if (!reds->stream) {
+    if (!reds->stream || reds->mig_target) {
         return;
     }
 
@@ -1798,6 +1821,18 @@ static void reds_main_handle_message(void *opaque, size_t size, uint32_t type, v
         break;
     case SPICE_MSGC_DISCONNECTING:
         break;
+    case SPICE_MSGC_MAIN_MIGRATE_END:
+        if (!reds->mig_target) {
+            red_printf("unexpected SPICE_MSGC_MIGRATE_END, not target");
+            return;
+        }
+        if (!reds->client_semi_mig_cap) {
+            red_printf("unexpected SPICE_MSGC_MIGRATE_END"
+                       ",client does not support semi-seamless migration");
+            return;
+        }
+        reds_handle_client_migrate_complete();
+        break;
     default:
         red_printf("unexpected type %d", type);
     }
@@ -2122,14 +2157,10 @@ static void reds_handle_main_link(RedLinkInfo *link)
         reds_send_link_result(link, SPICE_LINK_ERR_OK);
         while((connection_id = rand()) == 0);
         reds->agent_state.num_tokens = 0;
-        memcpy(&(reds->taTicket), &taTicket, sizeof(reds->taTicket));
         reds->mig_target = FALSE;
     } else {
-        if (link_mess->connection_id != reds->link_id) {
-            reds_send_link_result(link, SPICE_LINK_ERR_BAD_CONNECTION_ID);
-            reds_link_free(link);
-            return;
-        }
+        // TODO: make sure link_mess->connection_id is the same
+        // connection id the migration src had (use vmstate to store the connection id)
         reds_send_link_result(link, SPICE_LINK_ERR_OK);
         connection_id = link_mess->connection_id;
         reds->mig_target = TRUE;
@@ -2158,6 +2189,7 @@ static void reds_handle_main_link(RedLinkInfo *link)
     link->stream = NULL;
     link->link_mess = NULL;
     reds_link_free(link);
+    // TODO: should this be moved to be done only after mig completed (reds_main_channel_init)?
     if (vdagent) {
         SpiceCharDeviceInterface *sif;
         sif = SPICE_CONTAINEROF(vdagent->base.sif, SpiceCharDeviceInterface, base);
@@ -2612,6 +2644,32 @@ static void inputs_init()
     reds_register_channel(channel);
 }
 
+static void reds_mig_pending_link_add(SpiceLinkMess *link_msg, RedsStream *stream)
+{
+    RedsMigPendingLink *mig_link;
+
+    ASSERT(reds);
+    mig_link = spice_malloc0(sizeof(RedsMigPendingLink));
+    mig_link->link_msg = link_msg;
+    mig_link->stream = stream;
+
+    mig_link->next = reds->mig_pending_links;
+    reds->mig_pending_links = mig_link;
+    reds->num_mig_links++;
+}
+
+static void reds_mig_pending_links_free(void)
+{
+    red_printf("");
+    while(reds->mig_pending_links) {
+        RedsMigPendingLink *tmp = reds->mig_pending_links;
+        reds->mig_pending_links = tmp->next;
+        free(tmp->link_msg);
+        free(tmp);
+    }
+
+    reds->num_mig_links = 0;
+}
 static void reds_send_input_channel_insecure_warn()
 {
     RedsOutItem *item;
@@ -2650,6 +2708,35 @@ static void reds_channel_do_link(Channel *channel, SpiceLinkMess *link_msg, Reds
                   link_msg->num_channel_caps ? caps + link_msg->num_common_caps : NULL);
 }
 
+static void reds_handle_client_migrate_complete(void)
+{
+    RedsMigPendingLink *cur_link;
+
+    red_printf("");
+    // TODO: not doing net test. consider doing it on client_migrate_info
+    reds_main_channel_init(FALSE);
+
+    for (cur_link = reds->mig_pending_links; cur_link; cur_link = cur_link->next) {
+        Channel *channel = reds_find_channel(cur_link->link_msg->channel_type,
+                                             cur_link->link_msg->channel_id);
+        if (!channel) {
+            red_printf("warning: channel (%d, %d) (type,id) wasn't found",
+                       cur_link->link_msg->channel_type, cur_link->link_msg->channel_id);
+            continue;
+       }
+       reds_channel_do_link(channel, cur_link->link_msg, cur_link->stream);
+    }
+
+    reds_mig_pending_links_free();
+    reds->mig_target = FALSE;
+    if (reds->mig_wait_prev_complete) {
+        reds->mig_wait_prev_complete = FALSE;
+        core->timer_cancel(reds->mig_timer);
+        // starting a pending migrate info command
+        reds_mig_started();
+    }
+}
+
 static void reds_handle_other_links(RedLinkInfo *link)
 {
     Channel *channel;
@@ -2675,8 +2762,12 @@ static void reds_handle_other_links(RedLinkInfo *link)
     reds_show_new_channel(link, reds->link_id);
     reds_stream_remove_watch(link->stream);
 
-    reds_channel_do_link(channel, link->link_mess, link->stream);
-    free(link_mess);
+    if (reds->mig_target) {
+        reds_mig_pending_link_add(link->link_mess, link->stream);
+    } else {
+        reds_channel_do_link(channel, link->link_mess, link->stream);
+        free(link_mess);
+    }
 
     link->stream = NULL;
     link->link_mess = NULL;
@@ -2705,10 +2796,9 @@ static void reds_handle_ticket(void *opaque)
                         (unsigned char *)password, link->tiTicketing.rsa, RSA_PKCS1_OAEP_PADDING);
 
     if (ticketing_enabled) {
-        int expired = !link->link_mess->connection_id && taTicket.expiration_time < ltime;
-        char *actual_sever_pass = link->link_mess->connection_id ? reds->taTicket.password :
-                                                                   taTicket.password;
-        if (strlen(actual_sever_pass) == 0) {
+        int expired =  taTicket.expiration_time < ltime;
+
+        if (strlen(taTicket.password) == 0) {
             reds_send_link_result(link, SPICE_LINK_ERR_PERMISSION_DENIED);
             red_printf("Ticketing is enabled, but no password is set. "
                        "please set a ticket first");
@@ -2716,7 +2806,7 @@ static void reds_handle_ticket(void *opaque)
             return;
         }
 
-        if (expired || strncmp(password, actual_sever_pass, SPICE_MAX_PASSWORD_LENGTH) != 0) {
+        if (expired || strncmp(password, taTicket.password, SPICE_MAX_PASSWORD_LENGTH) != 0) {
             reds_send_link_result(link, SPICE_LINK_ERR_PERMISSION_DENIED);
             reds_link_free(link);
             return;
@@ -4184,7 +4274,7 @@ static void reds_mig_switch(void)
 static void migrate_timeout(void *opaque)
 {
     red_printf("");
-    ASSERT(reds->mig_wait_connect || reds->mig_wait_disconnect);
+    ASSERT(reds->mig_wait_connect || reds->mig_wait_disconnect || reds->mig_wait_prev_complete);
     reds_mig_disconnect();
 }
 
@@ -4211,7 +4301,7 @@ void reds_enable_mm_timer(void)
     RedsOutItem *item;
 
     core->timer_start(reds->mm_timer, MM_TIMER_GRANULARITY_MS);
-    if (!reds->stream) {
+    if (!reds->stream || reds->mig_target) {
         return;
     }
 
@@ -4997,7 +5087,6 @@ SPICE_GNUC_VISIBLE int spice_server_migrate_connect(SpiceServer *s, const char*
     ASSERT(migration_interface);
     ASSERT(reds == s);
 
-
     if (reds->expect_migrate && reds->client_semi_mig_cap) {
         red_printf("warning: consecutive calls without migration. Canceling previous call");
         reds_mig_finished(FALSE);
@@ -5014,7 +5103,13 @@ SPICE_GNUC_VISIBLE int spice_server_migrate_connect(SpiceServer *s, const char*
     reds_listen_stop();
 
     if (reds->client_semi_mig_cap) {
-        reds_mig_started();
+        if (!reds->mig_target) {
+            reds_mig_started();
+        } else {
+            red_printf("previous spice migration hasn't completed yet. Waiting for client");
+            reds->mig_wait_prev_complete = TRUE;
+            core->timer_start(reds->mig_timer, MIGRATE_TIMEOUT);
+        }
     } else {
         sif->migrate_connect_complete(migration_interface);
     }
-- 
1.7.4.4

[Qemu-devel] [PATCH spice-server 10/13] server: call migrate_connect_complete callback when no client is connected

[Yonit Halperin]

Signed-off-by: Yonit Halperin <yhalperi@redhat.com>
---
 server/reds.c |   72 ++++++++++++++++++++++++++++++++++-----------------------
 1 files changed, 43 insertions(+), 29 deletions(-)

diff --git a/server/reds.c b/server/reds.c
index ca4e1d1..6d2269c 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -761,6 +761,7 @@ static void reds_disconnect()
     reds->net_test_id = 0;
     reds->net_test_stage = NET_TEST_STAGE_INVALID;
     reds->in_handler.end_pos = 0;
+    reds->expect_migrate = FALSE;
 
     bitrate_per_sec = ~0;
     latency = 0;
@@ -1739,6 +1740,7 @@ static void reds_main_handle_message(void *opaque, size_t size, uint32_t type, v
         }
         break;
     case SPICE_MSGC_MAIN_MIGRATE_CONNECT_ERROR:
+        // TODO: fall into switch host in case of connect error or timeout
         red_printf("mig connect error");
         if (reds->mig_wait_connect) {
             reds_mig_cleanup();
@@ -4124,7 +4126,7 @@ static void reds_mig_release(void)
     }
 }
 
-static void reds_mig_continue(void)
+static void reds_mig_connect(void)
 {
     RedsMigSpice *s = reds->mig_spice;
     SpiceMsgMainMigrationBegin migrate;
@@ -4181,22 +4183,43 @@ static void reds_listen_stop(void)
 
 static void reds_mig_started(void)
 {
+    SpiceMigrateInterface *sif = NULL;
+
     red_printf("");
     ASSERT(reds->mig_spice);
 
-    reds->mig_inprogress = TRUE;
+    if (!migration_interface) {
+        return;
+    }
+
+    reds_listen_stop();
+    sif = SPICE_CONTAINEROF(migration_interface->base.sif, SpiceMigrateInterface, base);
 
     if (reds->stream == NULL) {
         red_printf("not connected to stream");
-        goto error;
+        reds_mig_release();
+        sif->migrate_connect_complete(migration_interface);
+        return;
     }
 
-    reds_mig_continue();
-    return;
+    reds->expect_migrate = TRUE;
+    if (reds->client_semi_mig_cap) {
+        if (reds->mig_target) {
+            red_printf("previous spice migration hasn't completed yet. Waiting for client");
+            reds->mig_wait_prev_complete = TRUE;
+            core->timer_start(reds->mig_timer, MIGRATE_TIMEOUT);
+            return;
+        }
+    } else if (sif) {
+        // switch host msg will be sent after migration completes
+        sif->migrate_connect_complete(migration_interface);
+        return;
+    }
 
-error:
-    reds_mig_release();
-    reds_mig_disconnect();
+    reds->mig_inprogress = TRUE;
+
+    reds_mig_connect();
+    return;
 }
 
 static void reds_mig_finished(int completed)
@@ -5060,6 +5083,7 @@ static int reds_set_migration_dest_info(const char* dest,
     RedsMigSpice *spice_migration = NULL;
 
     reds_mig_release();
+
     if ((port == -1 && secure_port == -1) || !dest) {
         return FALSE;
     }
@@ -5082,7 +5106,6 @@ SPICE_GNUC_VISIBLE int spice_server_migrate_connect(SpiceServer *s, const char*
                                                     int port, int secure_port,
                                                     const char* cert_subject)
 {
-    SpiceMigrateInterface *sif;
     red_printf("");
     ASSERT(migration_interface);
     ASSERT(reds == s);
@@ -5092,27 +5115,14 @@ SPICE_GNUC_VISIBLE int spice_server_migrate_connect(SpiceServer *s, const char*
         reds_mig_finished(FALSE);
     }
 
-    sif = SPICE_CONTAINEROF(migration_interface->base.sif, SpiceMigrateInterface, base);
-
     if (!reds_set_migration_dest_info(dest, port, secure_port, cert_subject)) {
+        SpiceMigrateInterface *sif;
+        sif = SPICE_CONTAINEROF(migration_interface->base.sif, SpiceMigrateInterface, base);
         sif->migrate_connect_complete(migration_interface);
         return -1;
     }
+    reds_mig_started();
 
-    reds->expect_migrate = TRUE;
-    reds_listen_stop();
-
-    if (reds->client_semi_mig_cap) {
-        if (!reds->mig_target) {
-            reds_mig_started();
-        } else {
-            red_printf("previous spice migration hasn't completed yet. Waiting for client");
-            reds->mig_wait_prev_complete = TRUE;
-            core->timer_start(reds->mig_timer, MIGRATE_TIMEOUT);
-        }
-    } else {
-        sif->migrate_connect_complete(migration_interface);
-    }
     return 0;
 }
 
@@ -5166,20 +5176,24 @@ SPICE_GNUC_VISIBLE int spice_server_migrate_end(SpiceServer *s, int completed)
     int ret = 0;
 
     red_printf("");
-
     ASSERT(migration_interface);
     ASSERT(reds == s);
 
     reds_listen_start();
     sif = SPICE_CONTAINEROF(migration_interface->base.sif, SpiceMigrateInterface, base);
-    if (!reds->expect_migrate && reds->stream) {
-        red_printf("spice_server_migrate_info was not called, disconnecting client");
+
+    if (!reds->stream) {
+        ret = 0;
+        goto complete;
+    }
+
+    if (!reds->expect_migrate) {
+        red_printf("spice_server_migrate_info failed or was not called, disconnecting client");
         reds_disconnect("");
         ret = -1;
         goto complete;
     }
 
-    reds->expect_migrate = FALSE;
     if (reds->client_semi_mig_cap) {
         reds_mig_finished(completed);
     } else {
-- 
1.7.4.4

[Qemu-devel] [PATCH spice-server 11/13] server: turn spice_server_migrate_start into a valid call

[Yonit Halperin]

We will add a qemu call to spice_server_migrate_start when migration starts.
For now, it does nothing, but we may need this notification in the future.

Signed-off-by: Yonit Halperin <yhalperi@redhat.com>
---
 server/reds.c |    8 +-------
 1 files changed, 1 insertions(+), 7 deletions(-)

diff --git a/server/reds.c b/server/reds.c
index 6d2269c..76aa0ed 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -5143,16 +5143,10 @@ SPICE_GNUC_VISIBLE int spice_server_migrate_info(SpiceServer *s, const char* des
 SPICE_GNUC_VISIBLE int spice_server_migrate_start(SpiceServer *s)
 {
     ASSERT(reds == s);
-
-    if (1) {
-        /* seamless doesn't work, fixing needs protocol change. */
-        return -1;
-    }
-
+    red_printf("");
     if (!reds->mig_spice) {
         return -1;
     }
-    reds_mig_started();
     return 0;
 }
 
-- 
1.7.4.4

[Qemu-devel] [PATCH spice-server 12/13] server: fall back to switch host scheme in case semi-seamless connection to target fails

[Yonit Halperin]

Signed-off-by: Yonit Halperin <yhalperi@redhat.com>
---
 server/reds.c |   18 +++++++++++++-----
 1 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/server/reds.c b/server/reds.c
index 76aa0ed..54c06d1 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -283,6 +283,7 @@ typedef struct RedsState {
     int mig_wait_disconnect;
     int mig_wait_prev_complete;
     int mig_inprogress;
+    int mig_connect_ok;
     int expect_migrate;
     int mig_target;
     RedsMigSpice *mig_spice;
@@ -1736,6 +1737,7 @@ static void reds_main_handle_message(void *opaque, size_t size, uint32_t type, v
     case SPICE_MSGC_MAIN_MIGRATE_CONNECTED:
         red_printf("client connected to migration target");
         if (reds->mig_wait_connect) {
+            reds->mig_connect_ok = TRUE;
             reds_mig_cleanup();
         }
         break;
@@ -1743,6 +1745,7 @@ static void reds_main_handle_message(void *opaque, size_t size, uint32_t type, v
         // TODO: fall into switch host in case of connect error or timeout
         red_printf("mig connect error");
         if (reds->mig_wait_connect) {
+            reds->mig_connect_ok = FALSE;
             reds_mig_cleanup();
         }
         break;
@@ -2172,6 +2175,7 @@ static void reds_handle_main_link(RedLinkInfo *link)
     reds->mig_inprogress = FALSE;
     reds->mig_wait_connect = FALSE;
     reds->mig_wait_disconnect = FALSE;
+    reds->mig_connect_ok = FALSE;
     reds->stream = link->stream;
     reds->in_handler.shut = FALSE;
 
@@ -4151,8 +4155,6 @@ static void reds_mig_connect(void)
 
     reds_push_pipe_item(item);
 
-    reds_mig_release();
-
     reds->mig_wait_connect = TRUE;
     core->timer_start(reds->mig_timer, MIGRATE_TIMEOUT);
 }
@@ -4194,6 +4196,7 @@ static void reds_mig_started(void)
 
     reds_listen_stop();
     sif = SPICE_CONTAINEROF(migration_interface->base.sif, SpiceMigrateInterface, base);
+    reds->mig_connect_ok = FALSE;
 
     if (reds->stream == NULL) {
         red_printf("not connected to stream");
@@ -4227,7 +4230,7 @@ static void reds_mig_finished(int completed)
     RedsOutItem *item;
 
     red_printf("");
-
+    reds_mig_release();
     if (reds->stream == NULL) {
         red_printf("no stream connected");
         return;
@@ -4298,7 +4301,12 @@ static void migrate_timeout(void *opaque)
 {
     red_printf("");
     ASSERT(reds->mig_wait_connect || reds->mig_wait_disconnect || reds->mig_wait_prev_complete);
-    reds_mig_disconnect();
+    if (reds->mig_wait_connect) {
+        reds->mig_connect_ok = FALSE;
+        reds_mig_cleanup();
+    } else {
+        reds_mig_disconnect();
+    }
 }
 
 static void key_modifiers_sender(void *opaque)
@@ -5188,7 +5196,7 @@ SPICE_GNUC_VISIBLE int spice_server_migrate_end(SpiceServer *s, int completed)
         goto complete;
     }
 
-    if (reds->client_semi_mig_cap) {
+    if (reds->client_semi_mig_cap && reds->mig_connect_ok) {
         reds_mig_finished(completed);
     } else {
         ret = spice_server_migrate_switch(s);
-- 
1.7.4.4

[Qemu-devel] [PATCH spice-server 13/13] Release 0.8.3

[Yonit Halperin]

Signed-off-by: Yonit Halperin <yhalperi@redhat.com>
---
 NEWS                     |    7 +++++++
 configure.ac             |    2 +-
 server/spice-server.syms |    4 ++++
 server/spice.h           |    2 +-
 4 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index ee6ceec..18168cb 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,10 @@
+Major changes in 0.8.3:
+=======================
+* server Bug fixes (RHBZ): 718713
+* client Bug fixes (RHBZ): 726441, 727969, 728252
+* server: semi-seamless migration support (RHBZ 738266)
+* require spice-protocol >= 0.8.2
+
 Major changes in 0.8.2:
 =======================
 * server: sasl support (fdo bz 34795)
diff --git a/configure.ac b/configure.ac
index e169f36..7fb636f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2,7 +2,7 @@ AC_PREREQ([2.57])
 
 m4_define([SPICE_MAJOR], 0)
 m4_define([SPICE_MINOR], 8)
-m4_define([SPICE_MICRO], 2)
+m4_define([SPICE_MICRO], 3)
 
 AC_INIT(spice, [SPICE_MAJOR.SPICE_MINOR.SPICE_MICRO], [], spice)
 
diff --git a/server/spice-server.syms b/server/spice-server.syms
index 826c562..62cdc18 100644
--- a/server/spice-server.syms
+++ b/server/spice-server.syms
@@ -81,3 +81,7 @@ global:
     spice_qxl_destroy_surface_async;
     spice_qxl_flush_surfaces_async;
 } SPICE_SERVER_0.8.1;
+
+SPICE_SERVER_0.8.3 {
+    spice_server_migrate_connect;
+} SPICE_SERVER_0.8.2;
diff --git a/server/spice.h b/server/spice.h
index 42ddbc6..ce2d149 100644
--- a/server/spice.h
+++ b/server/spice.h
@@ -22,7 +22,7 @@
 #include <sys/socket.h>
 #include <spice/qxl_dev.h>
 
-#define SPICE_SERVER_VERSION 0x000802 /* release 0.8.2 */
+#define SPICE_SERVER_VERSION 0x000803 /* release 0.8.3 */
 
 /* interface base type */
 
-- 
1.7.4.4

Re: [Qemu-devel] [PATCH spice-server 08/13] server: move the linking of channels to a separate routine

[Alon Levy]

On Wed, Sep 21, 2011 at 06:51:18PM +0300, Yonit Halperin wrote:
> 
> Signed-off-by: Yonit Halperin <yhalperi@redhat.com>
> ---
>  server/reds.c |   68 +++++++++++++++++++++++++++++++++++---------------------
>  1 files changed, 42 insertions(+), 26 deletions(-)
> 
> diff --git a/server/reds.c b/server/reds.c
> index bea0eb0..e7388a0 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -2612,12 +2612,48 @@ static void inputs_init()
>      reds_register_channel(channel);
>  }
>  
> +static void reds_send_input_channel_insecure_warn()
> +{
> +    RedsOutItem *item;
> +    SpiceMsgNotify notify;
> +    char *mess = "keyboard channel is insecure";
> +    const int mess_len = strlen(mess);
> +
> +    item = new_out_item(SPICE_MSG_NOTIFY);
> +
> +    notify.time_stamp = get_time_stamp();
> +    notify.severity = SPICE_NOTIFY_SEVERITY_WARN;
> +    notify.visibilty = SPICE_NOTIFY_VISIBILITY_HIGH;
> +    notify.what = SPICE_WARN_GENERAL;
> +    notify.message_len = mess_len;
> +
> +    spice_marshall_msg_notify(item->m, &notify);
> +    spice_marshaller_add(item->m, (uint8_t *)mess, mess_len + 1);
> +
> +    reds_push_pipe_item(item);
> +}
> +
> +static void reds_channel_do_link(Channel *channel, SpiceLinkMess *link_msg, RedsStream *stream)
> +{
> +    uint32_t *caps;
> +
> +    ASSERT(channel);
> +    ASSERT(link_msg);
> +    ASSERT(stream);
> +
> +    if (link_msg->channel_type == SPICE_CHANNEL_INPUTS && !stream->ssl) {
> +        reds_send_input_channel_insecure_warn();
> +    }
> +    caps = (uint32_t *)((uint8_t *)link_msg + link_msg->caps_offset);
> +    channel->link(channel, stream, FALSE, link_msg->num_common_caps,
> +                  link_msg->num_common_caps ? caps : NULL, link_msg->num_channel_caps,
> +                  link_msg->num_channel_caps ? caps + link_msg->num_common_caps : NULL);
> +}
> +
>  static void reds_handle_other_links(RedLinkInfo *link)
>  {
>      Channel *channel;
> -    RedsStream *stream;
>      SpiceLinkMess *link_mess;
> -    uint32_t *caps;
>  
>      link_mess = link->link_mess;
>  
> @@ -2635,36 +2671,16 @@ static void reds_handle_other_links(RedLinkInfo *link)
>      }
>  
>      reds_send_link_result(link, SPICE_LINK_ERR_OK);
> +    // TODO: if mig_target, where this call should be? here or only when the links are handled
Both? add a prefix argument to reds_show_new_channel maybe.

>      reds_show_new_channel(link, reds->link_id);
> -    if (link_mess->channel_type == SPICE_CHANNEL_INPUTS && !link->stream->ssl) {
> -        RedsOutItem *item;
> -        SpiceMsgNotify notify;
> -        char *mess = "keyboard channel is insecure";
> -        const int mess_len = strlen(mess);
> -
> -        item = new_out_item(SPICE_MSG_NOTIFY);
> -
> -        notify.time_stamp = get_time_stamp();
> -        notify.severity = SPICE_NOTIFY_SEVERITY_WARN;
> -        notify.visibilty = SPICE_NOTIFY_VISIBILITY_HIGH;
> -        notify.what = SPICE_WARN_GENERAL;
> -        notify.message_len = mess_len;
> +    reds_stream_remove_watch(link->stream);
>  
> -        spice_marshall_msg_notify(item->m, &notify);
> -        spice_marshaller_add(item->m, (uint8_t *)mess, mess_len + 1);
> +    reds_channel_do_link(channel, link->link_mess, link->stream);

Would be nice to consistently use link_mess or link->link_mess, not both.

> +    free(link_mess);
>  
> -        reds_push_pipe_item(item);
> -    }
> -    stream = link->stream;
> -    reds_stream_remove_watch(stream);
>      link->stream = NULL;
>      link->link_mess = NULL;
>      reds_link_free(link);
> -    caps = (uint32_t *)((uint8_t *)link_mess + link_mess->caps_offset);
> -    channel->link(channel, stream, reds->mig_target, link_mess->num_common_caps,
> -                  link_mess->num_common_caps ? caps : NULL, link_mess->num_channel_caps,
> -                  link_mess->num_channel_caps ? caps + link_mess->num_common_caps : NULL);
> -    free(link_mess);
>  }
>  
>  static void reds_handle_link(RedLinkInfo *link)
> -- 
> 1.7.4.4
> 

Re: [Qemu-devel] [PATCH spice-server 09/13] server: handling semi-seamless migration in the target side

[Alon Levy]

On Wed, Sep 21, 2011 at 06:51:19PM +0300, Yonit Halperin wrote:
> (1) not sending anything to the client till we recieve SPICE_MSGC_MIGRATE_END
> (2) start a new migration (handle client_migrate_info) only after SPICE_MSGC_MIGRATE_END
>     from the previous migration has been received
> (3) use the correct ticket
> 

One comment below.

> Signed-off-by: Yonit Halperin <yhalperi@redhat.com>
> ---
>  server/reds.c |  137 ++++++++++++++++++++++++++++++++++++++++++++++++---------
>  1 files changed, 116 insertions(+), 21 deletions(-)
> 
> diff --git a/server/reds.c b/server/reds.c
> index e7388a0..ca4e1d1 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -258,6 +258,13 @@ typedef struct RedsStatValue {
>  #endif
>  
>  typedef struct RedsMigSpice RedsMigSpice;
> +typedef struct RedsMigPendingLink RedsMigPendingLink;
> +
> +struct RedsMigPendingLink {
> +    RedsMigPendingLink *next;
> +    SpiceLinkMess *link_msg;
> +    RedsStream *stream;
> +};
>  
>  typedef struct RedsState {
>      int listen_socket;
> @@ -274,10 +281,13 @@ typedef struct RedsState {
>      int client_semi_mig_cap;
>      int mig_wait_connect;
>      int mig_wait_disconnect;
> +    int mig_wait_prev_complete;
>      int mig_inprogress;
>      int expect_migrate;
>      int mig_target;
>      RedsMigSpice *mig_spice;
> +    RedsMigPendingLink *mig_pending_links;
> +    int num_mig_links;
>      int num_of_channels;
>      IncomingHandler in_handler;
>      RedsOutgoingData outgoing;
> @@ -293,7 +303,6 @@ typedef struct RedsState {
>      SpiceTimer *vdi_port_write_timer;
>      int vdi_port_write_timer_started;
>  
> -    TicketAuthentication taTicket;
>      SSL_CTX *ctx;
>  
>  #ifdef RED_STATISTICS
> @@ -382,6 +391,9 @@ static uint8_t zero_page[ZERO_BUF_SIZE] = {0};
>  static void reds_push();
>  static void reds_out_item_free(RedsOutItem *item);
>  static void migrate_timeout(void *opaque);
> +static void reds_mig_pending_links_free(void);
> +static void reds_handle_client_migrate_complete(void);
> +static void reds_mig_started(void);
>  
>  static ChannelSecurityOptions *channels_security = NULL;
>  static int default_channel_security =
> @@ -648,7 +660,7 @@ static void reds_shatdown_channels()
>  static void reds_mig_cleanup()
>  {
>      if (reds->mig_inprogress) {
> -        if (reds->mig_wait_connect) {
> +        if (reds->mig_wait_connect || reds->mig_wait_prev_complete) {
>              SpiceMigrateInterface *sif;
>              ASSERT(migration_interface);
>              sif = SPICE_CONTAINEROF(migration_interface->base.sif, SpiceMigrateInterface, base);
> @@ -657,8 +669,13 @@ static void reds_mig_cleanup()
>          reds->mig_inprogress = FALSE;
>          reds->mig_wait_connect = FALSE;
>          reds->mig_wait_disconnect = FALSE;
> +        reds->mig_wait_prev_complete = FALSE;
>          core->timer_cancel(reds->mig_timer);
>      }
> +    if (reds->num_mig_links) {
> +        ASSERT(reds->mig_target);
> +        reds_mig_pending_links_free();
> +    }
>  }
>  
>  static void reds_reset_vdp()
> @@ -925,6 +942,11 @@ static void reds_send_channels()
>      Channel *channel;
>      int i;
>  
> +    if (reds->mig_target) {
> +        red_printf("warning: unexpected SPICE_MSGC_MAIN_ATTACH_CHANNELS during migration");
> +        return;
> +    }
> +
Can you make a note somewhere (I guess docs of migration would do if we had them, maybe this stuff
shold be in docs/migrate.txt?) that this relies on the target vm having the exact same channels
as the source vm, because you do not accept a SPICE_MSGC_MAIN_ATTACH_CHANNELS at the target, expecting
the client to reuse the channels it already has. I know we talked about it and it is totally reasonable,
I'm just not sure the only, way to behave, so I'd rather at least document it (maybe just in the commit
message).

>      item = new_out_item(SPICE_MSG_MAIN_CHANNELS_LIST);
>      channels_info = (SpiceMsgChannels *)spice_malloc(sizeof(SpiceMsgChannels) + reds->num_of_channels * sizeof(SpiceChannelId));
>      channels_info->num_of_channels = reds->num_of_channels;
> @@ -1008,7 +1030,7 @@ static void reds_send_mouse_mode()
>      SpiceMsgMainMouseMode mouse_mode;
>      RedsOutItem *item;
>  
> -    if (!reds->stream) {
> +    if (!reds->stream || reds->mig_target) {
>          return;
>      }
>  
> @@ -1077,6 +1099,7 @@ static void reds_agent_remove()
>      SpiceCharDeviceInstance *sin = vdagent;
>      SpiceCharDeviceInterface *sif;
>  
> +    // TODO: is this cond needed
>      if (!reds->mig_target) {
>          reds_reset_vdp();
>      }
> @@ -1103,7 +1126,7 @@ static void reds_send_tokens()
>      SpiceMsgMainAgentTokens tokens;
>      RedsOutItem *item;
>  
> -    if (!reds->stream) {
> +    if (!reds->stream || reds->mig_target) {
>          return;
>      }
>  
> @@ -1798,6 +1821,18 @@ static void reds_main_handle_message(void *opaque, size_t size, uint32_t type, v
>          break;
>      case SPICE_MSGC_DISCONNECTING:
>          break;
> +    case SPICE_MSGC_MAIN_MIGRATE_END:
> +        if (!reds->mig_target) {
> +            red_printf("unexpected SPICE_MSGC_MIGRATE_END, not target");
> +            return;
> +        }
> +        if (!reds->client_semi_mig_cap) {
> +            red_printf("unexpected SPICE_MSGC_MIGRATE_END"
> +                       ",client does not support semi-seamless migration");
> +            return;
> +        }
> +        reds_handle_client_migrate_complete();
> +        break;
>      default:
>          red_printf("unexpected type %d", type);
>      }
> @@ -2122,14 +2157,10 @@ static void reds_handle_main_link(RedLinkInfo *link)
>          reds_send_link_result(link, SPICE_LINK_ERR_OK);
>          while((connection_id = rand()) == 0);
>          reds->agent_state.num_tokens = 0;
> -        memcpy(&(reds->taTicket), &taTicket, sizeof(reds->taTicket));
>          reds->mig_target = FALSE;
>      } else {
> -        if (link_mess->connection_id != reds->link_id) {
> -            reds_send_link_result(link, SPICE_LINK_ERR_BAD_CONNECTION_ID);
> -            reds_link_free(link);
> -            return;
> -        }
> +        // TODO: make sure link_mess->connection_id is the same
> +        // connection id the migration src had (use vmstate to store the connection id)
>          reds_send_link_result(link, SPICE_LINK_ERR_OK);
>          connection_id = link_mess->connection_id;
>          reds->mig_target = TRUE;
> @@ -2158,6 +2189,7 @@ static void reds_handle_main_link(RedLinkInfo *link)
>      link->stream = NULL;
>      link->link_mess = NULL;
>      reds_link_free(link);
> +    // TODO: should this be moved to be done only after mig completed (reds_main_channel_init)?
>      if (vdagent) {
>          SpiceCharDeviceInterface *sif;
>          sif = SPICE_CONTAINEROF(vdagent->base.sif, SpiceCharDeviceInterface, base);
> @@ -2612,6 +2644,32 @@ static void inputs_init()
>      reds_register_channel(channel);
>  }
>  
> +static void reds_mig_pending_link_add(SpiceLinkMess *link_msg, RedsStream *stream)
> +{
> +    RedsMigPendingLink *mig_link;
> +
> +    ASSERT(reds);
> +    mig_link = spice_malloc0(sizeof(RedsMigPendingLink));
> +    mig_link->link_msg = link_msg;
> +    mig_link->stream = stream;
> +
> +    mig_link->next = reds->mig_pending_links;
> +    reds->mig_pending_links = mig_link;
> +    reds->num_mig_links++;
> +}
> +
> +static void reds_mig_pending_links_free(void)
> +{
> +    red_printf("");
> +    while(reds->mig_pending_links) {
> +        RedsMigPendingLink *tmp = reds->mig_pending_links;
> +        reds->mig_pending_links = tmp->next;
> +        free(tmp->link_msg);
> +        free(tmp);
> +    }
> +
> +    reds->num_mig_links = 0;
> +}
>  static void reds_send_input_channel_insecure_warn()
>  {
>      RedsOutItem *item;
> @@ -2650,6 +2708,35 @@ static void reds_channel_do_link(Channel *channel, SpiceLinkMess *link_msg, Reds
>                    link_msg->num_channel_caps ? caps + link_msg->num_common_caps : NULL);
>  }
>  
> +static void reds_handle_client_migrate_complete(void)
> +{
> +    RedsMigPendingLink *cur_link;
> +
> +    red_printf("");
> +    // TODO: not doing net test. consider doing it on client_migrate_info
> +    reds_main_channel_init(FALSE);
> +
> +    for (cur_link = reds->mig_pending_links; cur_link; cur_link = cur_link->next) {
> +        Channel *channel = reds_find_channel(cur_link->link_msg->channel_type,
> +                                             cur_link->link_msg->channel_id);
> +        if (!channel) {
> +            red_printf("warning: channel (%d, %d) (type,id) wasn't found",
> +                       cur_link->link_msg->channel_type, cur_link->link_msg->channel_id);
> +            continue;
> +       }
> +       reds_channel_do_link(channel, cur_link->link_msg, cur_link->stream);
> +    }
> +
> +    reds_mig_pending_links_free();
> +    reds->mig_target = FALSE;
> +    if (reds->mig_wait_prev_complete) {
> +        reds->mig_wait_prev_complete = FALSE;
> +        core->timer_cancel(reds->mig_timer);
> +        // starting a pending migrate info command
> +        reds_mig_started();
> +    }
> +}
> +
>  static void reds_handle_other_links(RedLinkInfo *link)
>  {
>      Channel *channel;
> @@ -2675,8 +2762,12 @@ static void reds_handle_other_links(RedLinkInfo *link)
>      reds_show_new_channel(link, reds->link_id);
>      reds_stream_remove_watch(link->stream);
>  
> -    reds_channel_do_link(channel, link->link_mess, link->stream);
> -    free(link_mess);
> +    if (reds->mig_target) {
> +        reds_mig_pending_link_add(link->link_mess, link->stream);
> +    } else {
> +        reds_channel_do_link(channel, link->link_mess, link->stream);
> +        free(link_mess);
> +    }
>  
>      link->stream = NULL;
>      link->link_mess = NULL;
> @@ -2705,10 +2796,9 @@ static void reds_handle_ticket(void *opaque)
>                          (unsigned char *)password, link->tiTicketing.rsa, RSA_PKCS1_OAEP_PADDING);
>  
>      if (ticketing_enabled) {
> -        int expired = !link->link_mess->connection_id && taTicket.expiration_time < ltime;
> -        char *actual_sever_pass = link->link_mess->connection_id ? reds->taTicket.password :
> -                                                                   taTicket.password;
> -        if (strlen(actual_sever_pass) == 0) {
> +        int expired =  taTicket.expiration_time < ltime;
> +
> +        if (strlen(taTicket.password) == 0) {
>              reds_send_link_result(link, SPICE_LINK_ERR_PERMISSION_DENIED);
>              red_printf("Ticketing is enabled, but no password is set. "
>                         "please set a ticket first");
> @@ -2716,7 +2806,7 @@ static void reds_handle_ticket(void *opaque)
>              return;
>          }
>  
> -        if (expired || strncmp(password, actual_sever_pass, SPICE_MAX_PASSWORD_LENGTH) != 0) {
> +        if (expired || strncmp(password, taTicket.password, SPICE_MAX_PASSWORD_LENGTH) != 0) {
>              reds_send_link_result(link, SPICE_LINK_ERR_PERMISSION_DENIED);
>              reds_link_free(link);
>              return;
> @@ -4184,7 +4274,7 @@ static void reds_mig_switch(void)
>  static void migrate_timeout(void *opaque)
>  {
>      red_printf("");
> -    ASSERT(reds->mig_wait_connect || reds->mig_wait_disconnect);
> +    ASSERT(reds->mig_wait_connect || reds->mig_wait_disconnect || reds->mig_wait_prev_complete);
>      reds_mig_disconnect();
>  }
>  
> @@ -4211,7 +4301,7 @@ void reds_enable_mm_timer(void)
>      RedsOutItem *item;
>  
>      core->timer_start(reds->mm_timer, MM_TIMER_GRANULARITY_MS);
> -    if (!reds->stream) {
> +    if (!reds->stream || reds->mig_target) {
>          return;
>      }
>  
> @@ -4997,7 +5087,6 @@ SPICE_GNUC_VISIBLE int spice_server_migrate_connect(SpiceServer *s, const char*
>      ASSERT(migration_interface);
>      ASSERT(reds == s);
>  
> -
>      if (reds->expect_migrate && reds->client_semi_mig_cap) {
>          red_printf("warning: consecutive calls without migration. Canceling previous call");
>          reds_mig_finished(FALSE);
> @@ -5014,7 +5103,13 @@ SPICE_GNUC_VISIBLE int spice_server_migrate_connect(SpiceServer *s, const char*
>      reds_listen_stop();
>  
>      if (reds->client_semi_mig_cap) {
> -        reds_mig_started();
> +        if (!reds->mig_target) {
> +            reds_mig_started();
> +        } else {
> +            red_printf("previous spice migration hasn't completed yet. Waiting for client");
> +            reds->mig_wait_prev_complete = TRUE;
> +            core->timer_start(reds->mig_timer, MIGRATE_TIMEOUT);
> +        }
>      } else {
>          sif->migrate_connect_complete(migration_interface);
>      }
> -- 
> 1.7.4.4
> 

Re: [Qemu-devel] [PATCH spice-server 12/13] server: fall back to switch host scheme in case semi-seamless connection to target fails

[Alon Levy]

On Wed, Sep 21, 2011 at 06:51:22PM +0300, Yonit Halperin wrote:
> 
> Signed-off-by: Yonit Halperin <yhalperi@redhat.com>
> ---
>  server/reds.c |   18 +++++++++++++-----
>  1 files changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/server/reds.c b/server/reds.c
> index 76aa0ed..54c06d1 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -283,6 +283,7 @@ typedef struct RedsState {
>      int mig_wait_disconnect;
>      int mig_wait_prev_complete;
>      int mig_inprogress;
> +    int mig_connect_ok;
>      int expect_migrate;
>      int mig_target;
>      RedsMigSpice *mig_spice;
> @@ -1736,6 +1737,7 @@ static void reds_main_handle_message(void *opaque, size_t size, uint32_t type, v
>      case SPICE_MSGC_MAIN_MIGRATE_CONNECTED:
>          red_printf("client connected to migration target");
>          if (reds->mig_wait_connect) {
> +            reds->mig_connect_ok = TRUE;
>              reds_mig_cleanup();
>          }
>          break;
> @@ -1743,6 +1745,7 @@ static void reds_main_handle_message(void *opaque, size_t size, uint32_t type, v
>          // TODO: fall into switch host in case of connect error or timeout
So this is no longer true?

>          red_printf("mig connect error");
>          if (reds->mig_wait_connect) {
> +            reds->mig_connect_ok = FALSE;
>              reds_mig_cleanup();
>          }
>          break;
> @@ -2172,6 +2175,7 @@ static void reds_handle_main_link(RedLinkInfo *link)
>      reds->mig_inprogress = FALSE;
>      reds->mig_wait_connect = FALSE;
>      reds->mig_wait_disconnect = FALSE;
> +    reds->mig_connect_ok = FALSE;
>      reds->stream = link->stream;
>      reds->in_handler.shut = FALSE;
>  
> @@ -4151,8 +4155,6 @@ static void reds_mig_connect(void)
>  
>      reds_push_pipe_item(item);
>  
> -    reds_mig_release();
> -
>      reds->mig_wait_connect = TRUE;
>      core->timer_start(reds->mig_timer, MIGRATE_TIMEOUT);
>  }
> @@ -4194,6 +4196,7 @@ static void reds_mig_started(void)
>  
>      reds_listen_stop();
>      sif = SPICE_CONTAINEROF(migration_interface->base.sif, SpiceMigrateInterface, base);
> +    reds->mig_connect_ok = FALSE;
>  
>      if (reds->stream == NULL) {
>          red_printf("not connected to stream");
> @@ -4227,7 +4230,7 @@ static void reds_mig_finished(int completed)
>      RedsOutItem *item;
>  
>      red_printf("");
> -
> +    reds_mig_release();
>      if (reds->stream == NULL) {
>          red_printf("no stream connected");
>          return;
> @@ -4298,7 +4301,12 @@ static void migrate_timeout(void *opaque)
>  {
>      red_printf("");
>      ASSERT(reds->mig_wait_connect || reds->mig_wait_disconnect || reds->mig_wait_prev_complete);
> -    reds_mig_disconnect();
> +    if (reds->mig_wait_connect) {
> +        reds->mig_connect_ok = FALSE;
> +        reds_mig_cleanup();
> +    } else {
> +        reds_mig_disconnect();
> +    }
>  }
>  
>  static void key_modifiers_sender(void *opaque)
> @@ -5188,7 +5196,7 @@ SPICE_GNUC_VISIBLE int spice_server_migrate_end(SpiceServer *s, int completed)
>          goto complete;
>      }
>  
> -    if (reds->client_semi_mig_cap) {
> +    if (reds->client_semi_mig_cap && reds->mig_connect_ok) {
>          reds_mig_finished(completed);
>      } else {
>          ret = spice_server_migrate_switch(s);
> -- 
> 1.7.4.4
> 

Re: [Qemu-devel] [PATCH spice-server 00/13] semi-seamless migration v2 (RHBZ #738266, 725009)

[Alon Levy]

On Wed, Sep 21, 2011 at 06:51:10PM +0300, Yonit Halperin wrote:
> same as the previous version with the following changes:
> 

Reviewed-by: Alon Levy <alevy@redhat.com>

Looks good.

> (1) I'm sending only the spice-server patches. I will send the client fixes later.
>     (and then I will move the "Release 0.8.3" patch to the head).
> (2) migration interface changes:
>     * spice_server_migrate_connect was added.
>       It is called on client_migrate_info cmd, if spice-server is new (instead of spice_server_migrate_info).
>     * migrate_end_complete callback was added to the migration interface.
>       In case we will need it for future seamless migration implementation
>     * bump release to 0.8.3
> (3) fall back to the switch host scheme in case of client failure to connect the target
> (4) fix accessing the wrong ticket in the target side (legacy from old seamless migration code)
> (5) fix not calling to migrate_connect_complete callback when no client is connected 
> 
> 
> reminder:
> 
> Here is a summary of the new migration scheme (copied from the commit msg of the first patch)
>     
>     migration source side
>     ---------------------
>     (1) spice_server_migrate_connect (*): tell client to link
>         to the target side - send SPICE_MSG_MAIN_MIGRATE_BEGIN.
>         It will be called on client_migrate_info cmd. client_migrate_info is asynchronous.
>     (2) Complete client_migrate_info only when the client has been connected
>         to the target - wait for SPICE_MSGC_MAIN_MIGRATE_(CONNECTED|CONNECT_ERROR) or a timeout.
>     (3) spice_server_migrate_end: tell client migration it can switch to the target - send
>         SPICE_MSG_MAIN_MIGRATE_END.
>     (4) client cleans up all data related to the connection to the source and switches to the target.
>         It sends SPICE_MSGC_MAIN_MIGRATE_END.
>     
>     migration target side
>     ---------------------
>     (1) the server identifies itself as a migraiton target since the client is linked with (connection_id != 0)
>     (2) server doesn't start the channels' logic (channel->link) till it receives SPICE_MSGC_MAIN_MIGRATE_END
>         from the client.
>     
>     *   After migration starts, the target qemu is blocked and cannot accept new spice client
>         connections. Thus, we trigger the connection to the target upon client_migrate_info
>         command.
> 
> Yonit Halperin (13):
>   server/spice.h: semi-seamless migration interface, RHBZ #738266
>   server: handle migration interface addition
>   configure: spice-protocol >= 0.8.2 (semi-seamless migration protocol)
>   server,proto: tell the client to connect to the migration target
>     before migraton starts
>   spice.proto: add SPICE_MSG_MAIN_MIGRATE_END &
>     SPICE_MSGC_MAIN_MIGRATE_END
>   server: send SPICE_MSG_MAIN_MIGRATE_END on spice_server_migrate_end
>   server: move SPICE_MSG_MAIN_INIT sending code to a separate routine
>   server: move the linking of channels to a separate routine
>   server: handling semi-seamless migration in the target side
>   server: call migrate_connect_complete callback when no client is
>     connected
>   server: turn spice_server_migrate_start into a valid call
>   server: fall back to switch host scheme in case semi-seamless
>     connection to target fails
>   Release 0.8.3
> 
>  NEWS                        |    7 +
>  common/messages.h           |    2 +
>  configure.ac                |    4 +-
>  server/reds.c               |  545 +++++++++++++++++++++++++++++++++----------
>  server/reds.h               |    4 +
>  server/spice-experimental.h |    3 -
>  server/spice-server.syms    |    4 +
>  server/spice.h              |   29 +++-
>  spice.proto                 |    9 +-
>  9 files changed, 474 insertions(+), 133 deletions(-)
> 
> -- 
> 1.7.4.4
> 

Re: [Qemu-devel] [PATCH spice-server 08/13] server: move the linking of channels to a separate routine

[Yonit Halperin]

On 09/22/2011 05:01 PM, Alon Levy wrote:
> On Wed, Sep 21, 2011 at 06:51:18PM +0300, Yonit Halperin wrote:
>>
>> Signed-off-by: Yonit Halperin<yhalperi@redhat.com>
>> ---
>>   server/reds.c |   68 +++++++++++++++++++++++++++++++++++---------------------
>>   1 files changed, 42 insertions(+), 26 deletions(-)
>>
>> diff --git a/server/reds.c b/server/reds.c
>> index bea0eb0..e7388a0 100644
>> --- a/server/reds.c
>> +++ b/server/reds.c
>> @@ -2612,12 +2612,48 @@ static void inputs_init()
>>       reds_register_channel(channel);
>>   }
>>
>> +static void reds_send_input_channel_insecure_warn()
>> +{
>> +    RedsOutItem *item;
>> +    SpiceMsgNotify notify;
>> +    char *mess = "keyboard channel is insecure";
>> +    const int mess_len = strlen(mess);
>> +
>> +    item = new_out_item(SPICE_MSG_NOTIFY);
>> +
>> +    notify.time_stamp = get_time_stamp();
>> +    notify.severity = SPICE_NOTIFY_SEVERITY_WARN;
>> +    notify.visibilty = SPICE_NOTIFY_VISIBILITY_HIGH;
>> +    notify.what = SPICE_WARN_GENERAL;
>> +    notify.message_len = mess_len;
>> +
>> +    spice_marshall_msg_notify(item->m,&notify);
>> +    spice_marshaller_add(item->m, (uint8_t *)mess, mess_len + 1);
>> +
>> +    reds_push_pipe_item(item);
>> +}
>> +
>> +static void reds_channel_do_link(Channel *channel, SpiceLinkMess *link_msg, RedsStream *stream)
>> +{
>> +    uint32_t *caps;
>> +
>> +    ASSERT(channel);
>> +    ASSERT(link_msg);
>> +    ASSERT(stream);
>> +
>> +    if (link_msg->channel_type == SPICE_CHANNEL_INPUTS&&  !stream->ssl) {
>> +        reds_send_input_channel_insecure_warn();
>> +    }
>> +    caps = (uint32_t *)((uint8_t *)link_msg + link_msg->caps_offset);
>> +    channel->link(channel, stream, FALSE, link_msg->num_common_caps,
>> +                  link_msg->num_common_caps ? caps : NULL, link_msg->num_channel_caps,
>> +                  link_msg->num_channel_caps ? caps + link_msg->num_common_caps : NULL);
>> +}
>> +
>>   static void reds_handle_other_links(RedLinkInfo *link)
>>   {
>>       Channel *channel;
>> -    RedsStream *stream;
>>       SpiceLinkMess *link_mess;
>> -    uint32_t *caps;
>>
>>       link_mess = link->link_mess;
>>
>> @@ -2635,36 +2671,16 @@ static void reds_handle_other_links(RedLinkInfo *link)
>>       }
>>
>>       reds_send_link_result(link, SPICE_LINK_ERR_OK);
>> +    // TODO: if mig_target, where this call should be? here or only when the links are handled
> Both? add a prefix argument to reds_show_new_channel maybe.
>
Hi, my concern was more about the red_channel_event called from 
reds_show_new_channel. I think it should be called only once, and at the 
same place it is in these patches - when the client makes the initial 
connection to the target, i.e., before migration.
>>       reds_show_new_channel(link, reds->link_id);
>> -    if (link_mess->channel_type == SPICE_CHANNEL_INPUTS&&  !link->stream->ssl) {
>> -        RedsOutItem *item;
>> -        SpiceMsgNotify notify;
>> -        char *mess = "keyboard channel is insecure";
>> -        const int mess_len = strlen(mess);
>> -
>> -        item = new_out_item(SPICE_MSG_NOTIFY);
>> -
>> -        notify.time_stamp = get_time_stamp();
>> -        notify.severity = SPICE_NOTIFY_SEVERITY_WARN;
>> -        notify.visibilty = SPICE_NOTIFY_VISIBILITY_HIGH;
>> -        notify.what = SPICE_WARN_GENERAL;
>> -        notify.message_len = mess_len;
>> +    reds_stream_remove_watch(link->stream);
>>
>> -        spice_marshall_msg_notify(item->m,&notify);
>> -        spice_marshaller_add(item->m, (uint8_t *)mess, mess_len + 1);
>> +    reds_channel_do_link(channel, link->link_mess, link->stream);
>
> Would be nice to consistently use link_mess or link->link_mess, not both.
>
>> +    free(link_mess);
>>
>> -        reds_push_pipe_item(item);
>> -    }
>> -    stream = link->stream;
>> -    reds_stream_remove_watch(stream);
>>       link->stream = NULL;
>>       link->link_mess = NULL;
>>       reds_link_free(link);
>> -    caps = (uint32_t *)((uint8_t *)link_mess + link_mess->caps_offset);
>> -    channel->link(channel, stream, reds->mig_target, link_mess->num_common_caps,
>> -                  link_mess->num_common_caps ? caps : NULL, link_mess->num_channel_caps,
>> -                  link_mess->num_channel_caps ? caps + link_mess->num_common_caps : NULL);
>> -    free(link_mess);
>>   }
>>
>>   static void reds_handle_link(RedLinkInfo *link)
>> --
>> 1.7.4.4
>>