[Spice-commits] server/reds.c server/reds.h server/spicevmc.c

[Frediano Ziglio]

 server/reds.c     |   32 ++++++++++++++++++++++++++++++++
 server/reds.h     |    1 +
 server/spicevmc.c |   10 ++++++++--
 3 files changed, 41 insertions(+), 2 deletions(-)

New commits:
commit dd871c01a810409ab097658291af9a33fd8ee629
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Thu Jan 26 17:57:55 2017 +0000

    Avoid using global variable for channel IDs
    
    This patch allocates VMC IDs by finding the first ID not used
    instead of using a global variable and incrementing the value
    for each channel created.
    This solves some potential issues:
    - remove the global state potentially making possible
      to use multiple SpiceServer on the same process;
    - don't potentially overflow the variable. This can happen if
      channels are allocated/deallocated multiple times
      (currently not done by Qemu).
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Jonathon Jongsma <jjongsma at redhat.com>

diff --git a/server/reds.c b/server/reds.c
index a738afc3..b6ecc6c6 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -408,6 +408,38 @@ RedChannel *reds_find_channel(RedsState *reds, uint32_t type, uint32_t id)
     return NULL;
 }
 
+/* Search for first free channel id for a specific channel type.
+ * Return first id free or <0 if not found. */
+int reds_get_free_channel_id(RedsState *reds, uint32_t type)
+{
+    RedChannel *channel;
+
+    // this mark if some IDs are used.
+    // The size of the array limits the possible id returned but
+    // usually the IDs used for a channel type are not much.
+    bool used_ids[256];
+
+    unsigned n;
+
+    // mark id used for the specific channel type
+    memset(used_ids, 0, sizeof(used_ids));
+    GLIST_FOREACH(reds->channels, RedChannel, channel) {
+        uint32_t this_type, this_id;
+        g_object_get(channel, "channel-type", &this_type, "id", &this_id, NULL);
+        if (this_type == type && this_id < SPICE_N_ELEMENTS(used_ids)) {
+            used_ids[this_id] = true;
+        }
+    }
+
+    // find first ID not marked as used
+    for (n = 0; n < SPICE_N_ELEMENTS(used_ids); ++n) {
+        if (!used_ids[n]) {
+            return n;
+        }
+    }
+    return -1;
+}
+
 static void reds_mig_cleanup(RedsState *reds)
 {
     if (reds->mig_inprogress) {
diff --git a/server/reds.h b/server/reds.h
index ee5a46c0..4f5fc28c 100644
--- a/server/reds.h
+++ b/server/reds.h
@@ -48,6 +48,7 @@ uint32_t reds_get_mm_time(void);
 void reds_register_channel(RedsState *reds, RedChannel *channel);
 void reds_unregister_channel(RedsState *reds, RedChannel *channel);
 RedChannel *reds_find_channel(RedsState *reds, uint32_t type, uint32_t id);
+int reds_get_free_channel_id(RedsState *reds, uint32_t type);
 SpiceMouseMode reds_get_mouse_mode(RedsState *reds); // used by inputs_channel
 gboolean reds_config_get_agent_mouse(const RedsState *reds); // used by inputs_channel
 int reds_has_vdagent(RedsState *reds); // used by inputs channel
diff --git a/server/spicevmc.c b/server/spicevmc.c
index 51b6913c..6b9b96fc 100644
--- a/server/spicevmc.c
+++ b/server/spicevmc.c
@@ -272,7 +272,6 @@ red_vmc_channel_finalize(GObject *object)
 static RedVmcChannel *red_vmc_channel_new(RedsState *reds, uint8_t channel_type)
 {
     GType gtype = G_TYPE_NONE;
-    static uint8_t id[SPICE_END_CHANNEL] = { 0, };
 
     switch (channel_type) {
         case SPICE_CHANNEL_USBREDIR:
@@ -288,11 +287,18 @@ static RedVmcChannel *red_vmc_channel_new(RedsState *reds, uint8_t channel_type)
             g_error("Unsupported channel_type for red_vmc_channel_new(): %u", channel_type);
             return NULL;
     }
+
+    int id = reds_get_free_channel_id(reds, channel_type);
+    if (id < 0) {
+        g_warning("Free ID not found creating new VMC channel");
+        return NULL;
+    }
+
     return g_object_new(gtype,
                         "spice-server", reds,
                         "core-interface", reds_get_core_interface(reds),
                         "channel-type", channel_type,
-                        "id", id[channel_type]++,
+                        "id", id,
                         "handle-acks", FALSE,
                         "migration-flags",
                         (SPICE_MIGRATE_NEED_FLUSH | SPICE_MIGRATE_NEED_DATA_TRANSFER),