Re: [Spice-devel] [PATCH spice-server 10/14] spicevmc: use SpiceCharDeviceState for writing to the guest device

[Hans de Goede]

Hi,

On 06/27/2012 05:16 PM, Yonit Halperin wrote:

With SpiceCharDeviceState, spicevmc now supports partial writes,
and storing data that is received from the client after the device is
stopped, instead of attempting to write it to the guest.
---
  server/spicevmc.c |   49 ++++++++++++++++++++++++-------------------------
  1 files changed, 24 insertions(+), 25 deletions(-)

diff --git a/server/spicevmc.c b/server/spicevmc.c
index 628a900..766b93f 100644
--- a/server/spicevmc.c
+++ b/server/spicevmc.c
@@ -32,6 +32,11 @@
  #include "red_channel.h"
  #include "reds.h"

+/* todo: add flow control. i.e.,
+ * (a) limit the amount of data on each message

This is already done, see the BUF_SIZE define below :) And I don't
think we should go smaller then 64k with the size of the messages.

+ * (b) limit the tokens available for the client
+ * (c) limit the tokens available for the server
+ */
  /* 64K should be enough for all but the largest writes + 32 bytes hdr */
  #define BUF_SIZE (64 * 1024 + 32)

@@ -50,9 +55,7 @@ typedef struct SpiceVmcState {
      SpiceCharDeviceState *chardev_st;
      SpiceCharDeviceInstance *chardev_sin;
      SpiceVmcPipeItem *pipe_item;
-    uint8_t *rcv_buf;
-    uint32_t rcv_buf_size;
-    int rcv_buf_in_use;
+    SpiceCharDeviceWriteBuffer *recv_from_client_buf;
  } SpiceVmcState;

  static SpiceVmcPipeItem *spicevmc_pipe_item_ref(SpiceVmcPipeItem *item)
@@ -204,23 +207,16 @@ static int 
spicevmc_red_channel_client_handle_message(RedChannelClient *rcc,
                                                        uint8_t *msg)
  {
      SpiceVmcState *state;
-    SpiceCharDeviceInstance *sin;
-    SpiceCharDeviceInterface *sif;

      state = SPICE_CONTAINEROF(rcc->channel, SpiceVmcState, channel);
-    sin = state->chardev_sin;
-    sif = SPICE_CONTAINEROF(sin->base.sif, SpiceCharDeviceInterface, base);

      if (type != SPICE_MSGC_SPICEVMC_DATA) {
          return red_channel_client_handle_message(rcc, size, type, msg);
      }

-    /*
-     * qemu spicevmc will consume everything we give it, no need for
-     * flow control checks (or to use a pipe).
-     */
-    sif->write(sin, msg, size);
-

Add an assert here like this? :
        spice_assert(state->recv_from_client_buf->buf == msg);

+    state->recv_from_client_buf->buf_used = size;
+    spice_char_device_write_buffer_add(state->chardev_st, 
state->recv_from_client_buf);
+    state->recv_from_client_buf = NULL;
      return TRUE;
  }

@@ -232,16 +228,16 @@ static uint8_t 
*spicevmc_red_channel_alloc_msg_rcv_buf(RedChannelClient *rcc,

      state = SPICE_CONTAINEROF(rcc->channel, SpiceVmcState, channel);

-    assert(!state->rcv_buf_in_use);
+    assert(!state->recv_from_client_buf);

-    if (size > state->rcv_buf_size) {
-        state->rcv_buf = spice_realloc(state->rcv_buf, size);
-        state->rcv_buf_size = size;
+    state->recv_from_client_buf = 
spice_char_device_write_buffer_get(state->chardev_st,
+                                                                     
rcc->client,
+                                                                     size);
+    if (!state->recv_from_client_buf) {
+        spice_error("failed to allocate write buffer");
+        return NULL;
      }
-
-    state->rcv_buf_in_use = 1;
-
-    return state->rcv_buf;
+    return state->recv_from_client_buf->buf;
  }

  static void spicevmc_red_channel_release_msg_rcv_buf(RedChannelClient *rcc,
@@ -253,8 +249,10 @@ static void 
spicevmc_red_channel_release_msg_rcv_buf(RedChannelClient *rcc,

      state = SPICE_CONTAINEROF(rcc->channel, SpiceVmcState, channel);

-    /* NOOP, we re-use the buffer every time and only free it on destruction */
-    state->rcv_buf_in_use = 0;
+    if (state->recv_from_client_buf) { /* buffer wasn't pushed to device */
+        spice_char_device_write_buffer_release(state->chardev_st, 
state->recv_from_client_buf);
+        state->recv_from_client_buf = NULL;
+    }
  }

  static void spicevmc_red_channel_hold_pipe_item(RedChannelClient *rcc,
@@ -377,12 +375,13 @@ void spicevmc_device_disconnect(SpiceCharDeviceInstance 
*sin)

      state = (SpiceVmcState *)spice_char_device_state_opaque_get(sin->st);

+    if (state->recv_from_client_buf) {
+        spice_char_device_write_buffer_release(state->chardev_st, 
state->recv_from_client_buf);
+    }
      spice_char_device_state_destroy(sin->st);
      state->chardev_st = NULL;

      reds_unregister_channel(&state->channel);
-
      free(state->pipe_item);
-    free(state->rcv_buf);
      red_channel_destroy(&state->channel);
  }

Regards,

Hans
_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel