[Spice-devel] [spice PATCH 39/55] char_device: don't connect a migrated client if the state of the device might have changed since it was created

Yonit Halperin yhalperi at redhat.com
Wed Aug 15 00:56:19 PDT 2012


If reading/writing from the device have occured before migration data
has arrived, the migration data might no longer be relvant, and we
disconnect the client.
---
 server/char_device.c |   22 ++++++++++++++--------
 server/char_device.h |   14 +++++++-------
 server/reds.c        |   44 ++++++++++++++++++++++++++++++--------------
 server/smartcard.c   |   22 +++++++++++++++-------
 server/spicevmc.c    |    8 ++++++--
 5 files changed, 72 insertions(+), 38 deletions(-)

diff --git a/server/char_device.c b/server/char_device.c
index b85a24d..ac2632d 100644
--- a/server/char_device.c
+++ b/server/char_device.c
@@ -681,20 +681,25 @@ void spice_char_device_state_destroy(SpiceCharDeviceState *char_dev)
     spice_char_device_state_unref(char_dev);
 }
 
-void spice_char_device_client_add(SpiceCharDeviceState *dev,
-                                  RedClient *client,
-                                  int do_flow_control,
-                                  uint32_t max_send_queue_size,
-                                  uint32_t num_client_tokens,
-                                  uint32_t num_send_tokens,
-                                  int wait_for_migrate_data)
+int spice_char_device_client_add(SpiceCharDeviceState *dev,
+                                 RedClient *client,
+                                 int do_flow_control,
+                                 uint32_t max_send_queue_size,
+                                 uint32_t num_client_tokens,
+                                 uint32_t num_send_tokens,
+                                 int wait_for_migrate_data)
 {
     SpiceCharDeviceClientState *dev_client;
 
     spice_assert(dev);
     spice_assert(client);
 
-    spice_assert(!wait_for_migrate_data || (dev->num_clients == 0 && !dev->active));
+    if (wait_for_migrate_data && (dev->num_clients > 0 || dev->active)) {
+        spice_warning("can't restore device %p from migration data. The device "
+                      "has already been active", dev);
+        return FALSE;
+    }
+
     dev->wait_for_migrate_data = wait_for_migrate_data;
 
     spice_debug("dev_state %p client %p", dev, client);
@@ -721,6 +726,7 @@ void spice_char_device_client_add(SpiceCharDeviceState *dev,
     dev->num_clients++;
     /* Now that we have a client, forward any pending device data */
     spice_char_device_wakeup(dev);
+    return TRUE;
 }
 
 void spice_char_device_client_remove(SpiceCharDeviceState *dev,
diff --git a/server/char_device.h b/server/char_device.h
index 6688e91..d6d75e3 100644
--- a/server/char_device.h
+++ b/server/char_device.h
@@ -148,13 +148,13 @@ void spice_char_device_reset(SpiceCharDeviceState *dev);
 
 /* max_send_queue_size = how many messages we can read from the device and enqueue for this client,
  * when we have tokens for other clients and no tokens for this one */
-void spice_char_device_client_add(SpiceCharDeviceState *dev,
-                                  RedClient *client,
-                                  int do_flow_control,
-                                  uint32_t max_send_queue_size,
-                                  uint32_t num_client_tokens,
-                                  uint32_t num_send_tokens,
-                                  int wait_for_migrate_data);
+int spice_char_device_client_add(SpiceCharDeviceState *dev,
+                                 RedClient *client,
+                                 int do_flow_control,
+                                 uint32_t max_send_queue_size,
+                                 uint32_t num_client_tokens,
+                                 uint32_t num_send_tokens,
+                                 int wait_for_migrate_data);
 
 void spice_char_device_client_remove(SpiceCharDeviceState *dev,
                                      RedClient *client);
diff --git a/server/reds.c b/server/reds.c
index 38e9298..4b3b1ee 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -1111,13 +1111,21 @@ void reds_on_main_agent_start(MainChannelClient *mcc, uint32_t num_tokens)
      * flow control, but will have no other problem.
      */
     if (!spice_char_device_client_exists(dev_state, rcc->client)) {
-        spice_char_device_client_add(dev_state,
-                                     rcc->client,
-                                     TRUE, /* flow control */
-                                     REDS_VDI_PORT_NUM_RECEIVE_BUFFS,
-                                     REDS_AGENT_WINDOW_SIZE,
-                                     num_tokens,
-                                     red_channel_client_waits_for_migrate_data(rcc));
+        int client_added;
+
+        client_added = spice_char_device_client_add(dev_state,
+                                                    rcc->client,
+                                                    TRUE, /* flow control */
+                                                    REDS_VDI_PORT_NUM_RECEIVE_BUFFS,
+                                                    REDS_AGENT_WINDOW_SIZE,
+                                                    num_tokens,
+                                                    red_channel_client_waits_for_migrate_data(rcc));
+
+        if (!client_added) {
+            spice_warning("failed to add client to agent");
+            reds_client_disconnect(rcc->client);
+            return;
+        }
     } else {
         spice_char_device_send_to_client_tokens_set(dev_state,
                                                     rcc->client,
@@ -3580,13 +3588,21 @@ static SpiceCharDeviceState *attach_to_red_agent(SpiceCharDeviceInstance *sin)
     } else {
        spice_debug("waiting for migration data");
         if (!spice_char_device_client_exists(reds->agent_state.base, reds_get_client())) {
-            spice_char_device_client_add(reds->agent_state.base,
-                                         reds_get_client(),
-                                         TRUE, /* flow control */
-                                         REDS_VDI_PORT_NUM_RECEIVE_BUFFS,
-                                         REDS_AGENT_WINDOW_SIZE,
-                                         ~0,
-                                         TRUE);
+            int client_added;
+
+            client_added = spice_char_device_client_add(reds->agent_state.base,
+                                                        reds_get_client(),
+                                                        TRUE, /* flow control */
+                                                        REDS_VDI_PORT_NUM_RECEIVE_BUFFS,
+                                                        REDS_AGENT_WINDOW_SIZE,
+                                                        ~0,
+                                                        TRUE);
+
+            if (!client_added) {
+                spice_warning("failed to add client to agent");
+                reds_disconnect();
+            }
+
         }
     }
     return state->base;
diff --git a/server/smartcard.c b/server/smartcard.c
index 0ec8a13..7c3a070 100644
--- a/server/smartcard.c
+++ b/server/smartcard.c
@@ -339,17 +339,25 @@ static void smartcard_char_device_attach_client(SpiceCharDeviceInstance *char_de
                                                 SmartCardChannelClient *scc)
 {
     SmartCardDeviceState *st = spice_char_device_state_opaque_get(char_device->st);
+    int client_added;
 
     spice_assert(!scc->smartcard_state && !st->scc);
     st->scc = scc;
-    spice_char_device_client_add(st->chardev_st,
-                                 scc->base.client,
-                                 FALSE, /* no flow control yet */
-                                 0, /* send queue size */
-                                 ~0,
-                                 ~0,
-                                 red_channel_client_waits_for_migrate_data(&scc->base));
     scc->smartcard_state = st;
+    client_added = spice_char_device_client_add(st->chardev_st,
+                                                scc->base.client,
+                                                FALSE, /* no flow control yet */
+                                                0, /* send queue size */
+                                                ~0,
+                                                ~0,
+                                                red_channel_client_waits_for_migrate_data(
+                                                    &scc->base));
+    if (!client_added) {
+        spice_warning("failed");
+        st->scc = NULL;
+        scc->smartcard_state = NULL;
+        red_channel_client_disconnect(&scc->base);
+    }
 }
 
 static void smartcard_char_device_notify_reader_remove(SmartCardDeviceState *st)
diff --git a/server/spicevmc.c b/server/spicevmc.c
index 1ce3169..b6eaa08 100644
--- a/server/spicevmc.c
+++ b/server/spicevmc.c
@@ -384,8 +384,12 @@ static void spicevmc_connect(RedChannel *channel, RedClient *client,
     state->rcc = rcc;
     red_channel_client_ack_zero_messages_window(rcc);
 
-    spice_char_device_client_add(state->chardev_st, client, FALSE, 0, ~0, ~0,
-                                 red_channel_client_waits_for_migrate_data(rcc));
+    if (!spice_char_device_client_add(state->chardev_st, client, FALSE, 0, ~0, ~0,
+                                      red_channel_client_waits_for_migrate_data(rcc))) {
+        spice_warning("failed to add client to spicevmc");
+        red_channel_client_disconnect(rcc);
+        return;
+    }
 
     if (sif->state) {
         sif->state(sin, 1);
-- 
1.7.7.6



More information about the Spice-devel mailing list