[Spice-commits] 3 commits - server/red-stream-device.c server/tests

Frediano Ziglio fziglio at kemper.freedesktop.org
Wed Apr 11 11:38:02 UTC 2018


 server/red-stream-device.c        |   83 ++++++++++++++++++++++++++++++++------
 server/tests/test-stream-device.c |   71 ++++++++++++++++++++++++++++++++
 2 files changed, 143 insertions(+), 11 deletions(-)

New commits:
commit 2a3d5624382ba49c4eb906e69697b92f79d06cf4
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Wed Mar 21 18:38:40 2018 +0000

    test-stream-device: Test we can read empty capabilities
    
    Code can have problems reading empty messages, check we can
    handle it.
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Christophe de Dinechin <dinechin at redhat.com>

diff --git a/server/tests/test-stream-device.c b/server/tests/test-stream-device.c
index 3eb71628..00fbc90c 100644
--- a/server/tests/test-stream-device.c
+++ b/server/tests/test-stream-device.c
@@ -352,6 +352,51 @@ static void test_stream_device_format_after_data(void)
     basic_event_loop_destroy();
 }
 
+// check empty capabilities
+static void test_stream_device_empty_capabilities(void)
+{
+    uint8_t *p = message;
+    SpiceCoreInterface *core = basic_event_loop_init();
+    Test *test = test_new(core);
+
+    pos = 0;
+    vmc_write_pos = 0;
+    message_sizes_curr = message_sizes;
+    message_sizes_end = message_sizes;
+
+    // add some messages into device buffer
+    p = add_stream_hdr(p, STREAM_TYPE_CAPABILITIES, 0);
+    *message_sizes_end = p - message;
+    ++message_sizes_end;
+    p = add_format(p, 640, 480, SPICE_VIDEO_CODEC_TYPE_MJPEG);
+    *message_sizes_end = p - message;
+    ++message_sizes_end;
+    p = add_format(p, 640, 480, SPICE_VIDEO_CODEC_TYPE_MJPEG);
+    *message_sizes_end = p - message;
+    ++message_sizes_end;
+
+    vmc_instance.base.sif = &vmc_interface.base;
+    spice_server_add_interface(test->server, &vmc_instance.base);
+
+    // we need to open the device and kick the start
+    // the alarm is to avoid program to stuck
+    alarm(5);
+    spice_server_port_event(&vmc_instance, SPICE_PORT_EVENT_OPENED);
+    spice_server_char_device_wakeup(&vmc_instance);
+    alarm(0);
+
+    // we should read all data
+    g_assert(message_sizes_curr - message_sizes == 3);
+
+    // we should have no data from the device
+    discard_server_capabilities();
+    g_assert_cmpint(vmc_write_pos, ==, 0);
+
+    test_destroy(test);
+    basic_event_loop_destroy();
+}
+
+
 int main(int argc, char *argv[])
 {
     g_test_init(&argc, &argv, NULL);
@@ -360,6 +405,7 @@ int main(int argc, char *argv[])
     g_test_add_func("/server/stream-device-unfinished", test_stream_device_unfinished);
     g_test_add_func("/server/stream-device-multiple", test_stream_device_multiple);
     g_test_add_func("/server/stream-device-format-after-data", test_stream_device_format_after_data);
+    g_test_add_func("/server/stream-device-empty-capabilities", test_stream_device_empty_capabilities);
 
     return g_test_run();
 }
commit 262af0a920c233c8e65fa5f01d0c4c2271848430
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Wed Mar 21 17:05:42 2018 +0000

    stream-device: Handle capabilities
    
    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 at redhat.com>
    Acked-by: Christophe de Dinechin <dinechin at redhat.com>

diff --git a/server/red-stream-device.c b/server/red-stream-device.c
index 6bc664ee..df6a366f 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;
@@ -156,7 +159,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;
@@ -261,6 +265,39 @@ handle_msg_format(StreamDevice *dev, SpiceCharDeviceInstance *sin)
 }
 
 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 know about
+    memset(dev->guest_capabilities, 0, sizeof(dev->guest_capabilities));
+    memcpy(dev->guest_capabilities, dev->msg->capabilities.capabilities,
+           MIN(sizeof(dev->guest_capabilities), dev->hdr.size));
+
+    return true;
+}
+
+static bool
 handle_msg_data(StreamDevice *dev, SpiceCharDeviceInstance *sin)
 {
     SpiceCharDeviceInterface *sif = spice_char_device_get_interface(sin);
@@ -590,6 +627,25 @@ 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;
+    fill_dev_hdr(hdr, STREAM_TYPE_CAPABILITIES, 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)
 {
     if (event != SPICE_PORT_EVENT_OPENED && event != SPICE_PORT_EVENT_CLOSED) {
@@ -602,6 +658,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 2fdd0a39..3eb71628 100644
--- a/server/tests/test-stream-device.c
+++ b/server/tests/test-stream-device.c
@@ -135,12 +135,36 @@ static uint8_t *add_format(uint8_t *p, uint32_t w, uint32_t h, SpiceVideoCodecTy
     return p + sizeof(fmt);
 }
 
+/* currently we don't care about possible capabilities sent so discard them
+ * from server reply */
+static void
+discard_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));
+    hdr.type = GUINT16_FROM_LE(hdr.type);
+    hdr.size = GUINT32_FROM_LE(hdr.size);
+    if (hdr.type == STREAM_TYPE_CAPABILITIES) {
+        g_assert_cmpint(hdr.size, <=, vmc_write_pos - sizeof(hdr));
+        vmc_write_pos -= hdr.size + sizeof(hdr);
+        memmove(vmc_write_buf, vmc_write_buf + 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;
 
+    discard_server_capabilities();
+
     g_assert(vmc_write_pos >= sizeof(hdr));
 
     memcpy(&hdr, vmc_write_buf, sizeof(hdr));
@@ -245,6 +269,7 @@ static void test_stream_device_unfinished(void)
     g_assert(message_sizes_curr - message_sizes == 1);
 
     // we should have no data from the device
+    discard_server_capabilities();
     g_assert_cmpint(vmc_write_pos, ==, 0);
 
     test_destroy(test);
commit 4e27551f0670f85d5d1b8f9cd6ccb17cac9f7069
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Mon Mar 5 15:57:31 2018 +0000

    stream-device: Factor out function to fill message headers
    
    This function will be reused to initialise different message
    headers.
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Christophe de Dinechin <dinechin at redhat.com>

diff --git a/server/red-stream-device.c b/server/red-stream-device.c
index d81c3b26..6bc664ee 100644
--- a/server/red-stream-device.c
+++ b/server/red-stream-device.c
@@ -76,6 +76,15 @@ close_timer_func(void *opaque)
     }
 }
 
+static void
+fill_dev_hdr(StreamDevHeader *hdr, StreamMsgType msg_type, uint32_t msg_size)
+{
+    hdr->protocol_version = STREAM_DEVICE_PROTOCOL;
+    hdr->padding = 0;
+    hdr->type = GUINT16_TO_LE(msg_type);
+    hdr->size = GUINT32_TO_LE(msg_size);
+}
+
 static bool
 stream_device_partial_read(StreamDevice *dev, SpiceCharDeviceInstance *sin)
 {
@@ -212,10 +221,7 @@ handle_msg_invalid(StreamDevice *dev, SpiceCharDeviceInstance *sin, const char *
     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_NOTIFY_ERROR);
-    hdr->size = GUINT32_TO_LE(msg_size);
+    fill_dev_hdr(hdr, STREAM_TYPE_NOTIFY_ERROR, msg_size);
 
     StreamMsgNotifyError *const error = (StreamMsgNotifyError *)(hdr+1);
     error->error_code = GUINT32_TO_LE(0);
@@ -453,10 +459,7 @@ stream_device_stream_start(void *opaque, StreamMsgStartStop *start,
     buf->buf_used = total_size;
 
     StreamDevHeader *hdr = (StreamDevHeader *)buf->buf;
-    hdr->protocol_version = STREAM_DEVICE_PROTOCOL;
-    hdr->padding = 0;
-    hdr->type = GUINT16_TO_LE(STREAM_TYPE_START_STOP);
-    hdr->size = GUINT32_TO_LE(msg_size);
+    fill_dev_hdr(hdr, STREAM_TYPE_START_STOP, msg_size);
 
     memcpy(&hdr[1], start, msg_size);
 


More information about the Spice-commits mailing list