[Spice-devel] [RFCv5 16/47] server/main_channel: move connection_id from reds

Alon Levy alevy at redhat.com
Sun May 8 06:11:12 PDT 2011


Expose additional api to find a client given a connection_id. The connection_id
is first set when the first channel connects, which is the main channel.
It could also be kept in the RedClient instead, not sure.

TODO:
 multiple todo's added for multiclient handling. I don't remember why
 I wrote them exactly, and besides if I did any migration tests. So: TODO.
---
 server/main_channel.c |   22 +++++++++++++++++++---
 server/main_channel.h |    5 +++--
 server/reds.c         |   33 ++++++++++++++++++---------------
 3 files changed, 40 insertions(+), 20 deletions(-)

diff --git a/server/main_channel.c b/server/main_channel.c
index 1d0b6c3..21c7b0b 100644
--- a/server/main_channel.c
+++ b/server/main_channel.c
@@ -121,6 +121,7 @@ typedef struct MultiMediaTimePipeItem {
 
 struct MainChannelClient {
     RedChannelClient base;
+    uint32_t connection_id;
     uint32_t ping_id;
     uint32_t net_test_id;
     int net_test_stage;
@@ -154,6 +155,20 @@ static void main_disconnect(MainChannel *main_chan)
     red_channel_destroy(&main_chan->base);
 }
 
+RedClient *main_channel_get_client_by_link_id(MainChannel *main_chan, uint32_t connection_id)
+{
+    MainChannelClient *mcc;
+
+    if (!main_chan || !main_chan->base.rcc) {
+        return NULL;
+    }
+    mcc = SPICE_CONTAINEROF(main_chan->base.rcc, MainChannelClient, base);
+    if (mcc->connection_id == connection_id) {
+        return mcc->base.client;
+    }
+    return NULL;
+}
+
 static int main_channel_client_push_ping(MainChannelClient *rcc, int size);
 
 void main_channel_start_net_test(MainChannelClient *mcc)
@@ -861,11 +876,12 @@ static void ping_timer_cb(void *opaque)
 #endif /* RED_STATISTICS */
 
 MainChannelClient *main_channel_client_create(MainChannel *main_chan,
-    RedClient *client, RedsStream *stream)
+    RedClient *client, RedsStream *stream, uint32_t connection_id)
 {
     MainChannelClient *mcc = (MainChannelClient*)red_channel_client_create(
         sizeof(MainChannelClient), &main_chan->base, client, stream);
 
+    mcc->connection_id = connection_id;
     mcc->bitrate_per_sec = ~0;
 #ifdef RED_STATISTICS
     if (!(mcc->ping_timer = core->timer_add(ping_timer_cb, NULL))) {
@@ -877,7 +893,7 @@ MainChannelClient *main_channel_client_create(MainChannel *main_chan,
 }
 
 MainChannelClient *main_channel_link(Channel *channel, RedClient *client,
-                        RedsStream *stream, int migration,
+                        RedsStream *stream, uint32_t connection_id, int migration,
                         int num_common_caps, uint32_t *common_caps, int num_caps,
                         uint32_t *caps)
 {
@@ -904,7 +920,7 @@ MainChannelClient *main_channel_link(Channel *channel, RedClient *client,
         ASSERT(channel->data);
     }
     red_printf("add main channel client");
-    mcc = main_channel_client_create(channel->data, client, stream);
+    mcc = main_channel_client_create(channel->data, client, stream, connection_id);
     return mcc;
 }
 
diff --git a/server/main_channel.h b/server/main_channel.h
index 651334d..fd7e38b 100644
--- a/server/main_channel.h
+++ b/server/main_channel.h
@@ -47,10 +47,11 @@ struct MainMigrateData {
 typedef struct MainChannel MainChannel;
 
 Channel *main_channel_init(void);
+RedClient *main_channel_get_client_by_link_id(MainChannel *main_chan, uint32_t link_id);
 /* This is a 'clone' from the reds.h Channel.link callback */
 MainChannelClient *main_channel_link(struct Channel *, RedClient *client,
-                 RedsStream *stream, int migration, int num_common_caps,
-                 uint32_t *common_caps, int num_caps, uint32_t *caps);
+     RedsStream *stream, uint32_t link_id, int migration, int num_common_caps,
+     uint32_t *common_caps, int num_caps, uint32_t *caps);
 void main_channel_close(MainChannel *main_chan); // not destroy, just socket close
 int main_channel_push_ping(MainChannel *main_chan, int size);
 void main_channel_push_mouse_mode(MainChannel *main_chan, int current_mode, int is_client_mouse_allowed);
diff --git a/server/reds.c b/server/reds.c
index 8a2b532..c2a40f5 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -200,7 +200,6 @@ typedef struct RedsState {
     int pending_mouse_event;
     Ring clients;
     int num_clients;
-    uint32_t link_id;
     Channel *main_channel_factory;
     MainChannel *main_channel;
 
@@ -587,7 +586,6 @@ void reds_client_disconnect(RedClient *client)
 
     red_printf("");
     client->disconnecting = TRUE;
-    reds->link_id = 0;
 
     /* Reset write filter to start with clean state on client reconnect */
     agent_msg_filter_init(&reds->agent_state.write_filter, agent_copypaste,
@@ -1502,6 +1500,12 @@ static void reds_send_link_result(RedLinkInfo *link, uint32_t error)
     sync_write(link->stream, &error, sizeof(error));
 }
 
+int reds_expects_link_id()
+{
+    red_printf("TODO: keep a list of connection_id's from migration, compare to them");
+    return 1;
+}
+
 // TODO: now that main is a separate channel this should
 // actually be joined with reds_handle_other_links, become reds_handle_link
 static void reds_handle_main_link(RedLinkInfo *link)
@@ -1524,7 +1528,8 @@ static void reds_handle_main_link(RedLinkInfo *link)
         memcpy(&(reds->taTicket), &taTicket, sizeof(reds->taTicket));
         reds->mig_target = FALSE;
     } else {
-        if (link_mess->connection_id != reds->link_id) {
+        // migration - check if this is one of the expected connection_id's
+        if (!reds_expects_link_id(link_mess->connection_id)) {
             reds_send_link_result(link, SPICE_LINK_ERR_BAD_CONNECTION_ID);
             reds_link_free(link);
             return;
@@ -1534,7 +1539,6 @@ static void reds_handle_main_link(RedLinkInfo *link)
         reds->mig_target = TRUE;
     }
 
-    reds->link_id = connection_id;
     reds->mig_inprogress = FALSE;
     reds->mig_wait_connect = FALSE;
     reds->mig_wait_disconnect = FALSE;
@@ -1553,7 +1557,7 @@ static void reds_handle_main_link(RedLinkInfo *link)
     ring_add(&reds->clients, &client->link);
     reds->num_clients++;
     mcc = main_channel_link(reds->main_channel_factory, client,
-                  stream, reds->mig_target, link_mess->num_common_caps,
+                  stream, connection_id, 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);
     reds->main_channel = (MainChannel*)reds->main_channel_factory->data;
@@ -1627,22 +1631,21 @@ static void reds_handle_other_links(RedLinkInfo *link)
     uint32_t *caps;
 
     link_mess = link->link_mess;
-    if (reds->num_clients == 1) {
-        client = SPICE_CONTAINEROF(ring_get_head(&reds->clients), RedClient, link);
+    if (reds->main_channel) {
+        client = main_channel_get_client_by_link_id(reds->main_channel,
+            link_mess->connection_id);
     }
 
+    // TODO: MC: broke migration (at least for the dont-drop-connection kind).
+    // On migration we should get a connection_id to expect (must be a security measure)
+    // where do we store it? on reds, but should be a list (MC).
     if (!client) {
         reds_send_link_result(link, SPICE_LINK_ERR_BAD_CONNECTION_ID);
         reds_link_free(link);
         return;
     }
 
-    if (!reds->link_id || reds->link_id != link_mess->connection_id) {
-        reds_send_link_result(link, SPICE_LINK_ERR_BAD_CONNECTION_ID);
-        reds_link_free(link);
-        return;
-    }
-
+    // TODO: MC: be less lenient. Tally connections from same connection_id (by same client).
     if (!(channel = reds_find_channel(link_mess->channel_type,
                                       link_mess->channel_id))) {
         reds_send_link_result(link, SPICE_LINK_ERR_CHANNEL_NOT_AVAILABLE);
@@ -1651,7 +1654,7 @@ static void reds_handle_other_links(RedLinkInfo *link)
     }
 
     reds_send_link_result(link, SPICE_LINK_ERR_OK);
-    reds_show_new_channel(link, reds->link_id);
+    reds_show_new_channel(link, link_mess->connection_id);
     if (link_mess->channel_type == SPICE_CHANNEL_INPUTS && !link->stream->ssl) {
         char *mess = "keyboard channel is insecure";
         const int mess_len = strlen(mess);
@@ -3007,7 +3010,7 @@ struct RedsMigSpice {
 };
 
 typedef struct RedsMigSpiceMessage {
-    uint32_t link_id;
+    uint32_t connection_id;
 } RedsMigSpiceMessage;
 
 typedef struct RedsMigCertPubKeyInfo {
-- 
1.7.5.1



More information about the Spice-devel mailing list