Re: [PATCH spice-server 2/2] stream-device: Handle capabilities

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 03/19/2018 06:46 PM, Frediano Ziglio wrote:
Handle capabilities from guest device.
Send capability to the guest when device is opened.
Currently there's no capabilities set on the message sent.
On the tests we need to discard the capability message before
reading the error.

Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
---
  server/red-stream-device.c        | 66 +++++++++++++++++++++++++++++++++++++--
  server/tests/test-stream-device.c | 22 +++++++++++++
  2 files changed, 85 insertions(+), 3 deletions(-)

Changes since v1:
- rebased on master (with minor fix due to rename).

diff --git a/server/red-stream-device.c b/server/red-stream-device.c
index e91df88d..1732b888 100644
--- a/server/red-stream-device.c
+++ b/server/red-stream-device.c
@@ -1,6 +1,6 @@
  /* spice-server character device to handle a video stream
- Copyright (C) 2017 Red Hat, Inc.
+   Copyright (C) 2017-2018 Red Hat, Inc.
This library is free software; you can redistribute it and/or
     modify it under the terms of the GNU Lesser General Public
@@ -25,6 +25,8 @@
  #include "cursor-channel.h"
  #include "reds.h"
+#define MAX_GUEST_CAPABILITIES_BYTES ((STREAM_CAP_END+7)/8)
+
  struct StreamDevice {
      RedCharDevice parent;
@@ -42,6 +44,7 @@ struct StreamDevice {
      bool has_error;
      bool opened;
      bool flow_stopped;
+    uint8_t guest_capabilities[MAX_GUEST_CAPABILITIES_BYTES];
      StreamChannel *stream_channel;
      CursorChannel *cursor_channel;
      SpiceTimer *close_timer;
@@ -61,7 +64,7 @@ typedef bool StreamMsgHandler(StreamDevice *dev, SpiceCharDeviceInstance *sin)
      SPICE_GNUC_WARN_UNUSED_RESULT;
static StreamMsgHandler handle_msg_format, handle_msg_data, handle_msg_cursor_set,
-    handle_msg_cursor_move;
+    handle_msg_cursor_move, handle_msg_capabilities;
static bool handle_msg_invalid(StreamDevice *dev, SpiceCharDeviceInstance *sin,
                                 const char *error_msg) SPICE_GNUC_WARN_UNUSED_RESULT;
@@ -147,7 +150,8 @@ stream_device_partial_read(StreamDevice *dev, SpiceCharDeviceInstance *sin)
          }
          break;
      case STREAM_TYPE_CAPABILITIES:
-        /* FIXME */
+        handled = handle_msg_capabilities(dev, sin);
+        break;
      default:
          handled = handle_msg_invalid(dev, sin, "Invalid message type");
          break;
@@ -254,6 +258,38 @@ handle_msg_format(StreamDevice *dev, SpiceCharDeviceInstance *sin)
      return true;
  }
+static bool
+handle_msg_capabilities(StreamDevice *dev, SpiceCharDeviceInstance *sin)
+{
+    SpiceCharDeviceInterface *sif = spice_char_device_get_interface(sin);
+
+    if (spice_extra_checks) {
+        spice_assert(dev->hdr_pos >= sizeof(StreamDevHeader));
+        spice_assert(dev->hdr.type == STREAM_TYPE_CAPABILITIES);
+    }
+
+    if (dev->hdr.size > STREAM_MSG_CAPABILITIES_MAX_BYTES) {
(*)
+        return handle_msg_invalid(dev, sin, "Wrong size for StreamMsgCapabilities") > +    }
+
+    int n = sif->read(sin, dev->msg->buf + dev->msg_pos, dev->hdr.size - dev->msg_pos);
+    if (n < 0) {
+        return handle_msg_invalid(dev, sin, NULL);
+    }
+
+    dev->msg_pos += n;
+
+    if (dev->msg_pos < dev->hdr.size) {
+        return false;
+    }
+
+    // copy only capabilities we care about
+    memset(dev->guest_capabilities, 0, sizeof(dev->guest_capabilities));
+    memcpy(dev->guest_capabilities, dev->msg->buf, MIN(MAX_GUEST_CAPABILITIES_BYTES, dev->hdr.size));

nits:
1. If the this line is reached (see *), there is no need for MIN as
   dev->hdr.size <= MAX_GUEST_CAPABILITIES_BYTES

2. All other messages got a structure in spice-protocol.
   This message can have one too (similar to StreamMsgData)

Uri.

+
+    return true;
+}
+
  static bool
  handle_msg_data(StreamDevice *dev, SpiceCharDeviceInstance *sin)
  {
@@ -586,6 +622,28 @@ char_device_set_state(RedCharDevice *char_dev, int state)
      }
  }
+static void
+send_capabilities(RedCharDevice *char_dev)
+{
+    int msg_size = MAX_GUEST_CAPABILITIES_BYTES;
+    int total_size = sizeof(StreamDevHeader) + msg_size;
+
+    RedCharDeviceWriteBuffer *buf =
+        red_char_device_write_buffer_get_server_no_token(char_dev, total_size);
+    buf->buf_used = total_size;
+
+    StreamDevHeader *const hdr = (StreamDevHeader *)buf->buf;
+    hdr->protocol_version = STREAM_DEVICE_PROTOCOL;
+    hdr->padding = 0;
+    hdr->type = GUINT16_TO_LE(STREAM_TYPE_CAPABILITIES);
+    hdr->size = GUINT32_TO_LE(msg_size);
+
+    StreamMsgCapabilities *const caps = (StreamMsgCapabilities *)(hdr+1);
+    memset(caps, 0, msg_size);
+
+    red_char_device_write_buffer_add(char_dev, buf);
+}
+
  static void
  stream_device_port_event(RedCharDevice *char_dev, uint8_t event)
  {
@@ -599,6 +657,8 @@ stream_device_port_event(RedCharDevice *char_dev, uint8_t event)
      dev->opened = (event == SPICE_PORT_EVENT_OPENED);
      if (dev->opened) {
          stream_device_create_channel(dev);
+
+        send_capabilities(char_dev);
      }
      dev->hdr_pos = 0;
      dev->msg_pos = 0;
diff --git a/server/tests/test-stream-device.c b/server/tests/test-stream-device.c
index 3c9209a4..43011f9d 100644
--- a/server/tests/test-stream-device.c
+++ b/server/tests/test-stream-device.c
@@ -135,12 +135,33 @@ static uint8_t *add_format(uint8_t *p, uint32_t w, uint32_t h, SpiceVideoCodecTy
      return p + sizeof(fmt);
  }
+// remove capabilities from server reply
+static void
+consume_server_capabilities(void)
+{
+    StreamDevHeader hdr;
+
+    if (vmc_write_pos == 0) {
+        return;
+    }
+    g_assert(vmc_write_pos >= sizeof(hdr));
+
+    memcpy(&hdr, vmc_write_buf, sizeof(hdr));
+    if (GUINT16_FROM_LE(hdr.type) == STREAM_TYPE_CAPABILITIES) {
+        g_assert_cmpint(GUINT32_FROM_LE(hdr.size), <=, vmc_write_pos - sizeof(hdr));
+        vmc_write_pos -= GUINT32_FROM_LE(hdr.size) + sizeof(hdr);
+        memmove(vmc_write_buf, vmc_write_buf + GUINT32_FROM_LE(hdr.size) + sizeof(hdr), vmc_write_pos);
+    }
+}
+
  // check we have an error message on the write buffer
  static void
  check_vmc_error_message(void)
  {
      StreamDevHeader hdr;
+ consume_server_capabilities();
+
      g_assert(vmc_write_pos >= sizeof(hdr));
memcpy(&hdr, vmc_write_buf, sizeof(hdr));
@@ -245,6 +266,7 @@ static void test_stream_device_unfinished(void)
      g_assert(message_sizes_curr - message_sizes == 1);
// we should have no data from the device
+    consume_server_capabilities();
      g_assert_cmpint(vmc_write_pos, ==, 0);
test_destroy(test);


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




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]