[Spice-devel] [PATCH spice-server 2/2] stream-device: Implements properly device reset on open/close

[Frediano Ziglio]

Due to the way Qemu handle the device we must consume all pending
data inside the callback to read data from the device.
We need to consume all data from previous device dialog to avoid
that next device usage is still seeing old data.
This as Qemu returns 0 if you call SpiceCharDeviceInterface::read
outside this callback (which is called by Qemu using
spice_server_char_device_wakeup).

Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
---
 server/stream-device.c            | 15 ++++++-
 server/tests/test-stream-device.c | 93 ++++++++++++++++++++-------------------
 2 files changed, 61 insertions(+), 47 deletions(-)

Note that patch is much smaller without space changes.

diff --git a/server/stream-device.c b/server/stream-device.c
index 18fee2e8..6c71c7ba 100644
--- a/server/stream-device.c
+++ b/server/stream-device.c
@@ -79,11 +79,22 @@ stream_device_read_msg_from_dev(RedCharDevice *self, SpiceCharDeviceInstance *si
     int n;
     bool handled = false;
 
-    if (dev->has_error || dev->flow_stopped || !dev->stream_channel) {
+    sif = spice_char_device_get_interface(sin);
+
+    // in order to get in sync every time we open the device we need
+    // to discard data here. This as Qemu keeps a buffer of data which
+    // is used only during spice_server_char_device_wakeup from Qemu
+    if (G_UNLIKELY(dev->has_error)) {
+        uint8_t buf[16 * 1024];
+        while (sif->read(sin, buf, sizeof(buf)) > 0) {
+            continue;
+        }
         return NULL;
     }
 
-    sif = spice_char_device_get_interface(sin);
+    if (dev->flow_stopped || !dev->stream_channel) {
+        return NULL;
+    }
 
     /* read header */
     while (dev->hdr_pos < sizeof(dev->hdr)) {
diff --git a/server/tests/test-stream-device.c b/server/tests/test-stream-device.c
index f937e30b..d6143774 100644
--- a/server/tests/test-stream-device.c
+++ b/server/tests/test-stream-device.c
@@ -131,51 +131,54 @@ static void test_stream_device(void)
     SpiceCoreInterface *core = basic_event_loop_init();
     Test *test = test_new(core);
 
-    message_sizes_curr = message_sizes;
-    message_sizes_end = message_sizes;
-
-    // add some messages into device buffer
-    // here we are testing the device is reading at least two
-    // consecutive format messages
-    // first message part has 2 extra bytes to check for header split
-    p = add_format(p, 640, 480, SPICE_VIDEO_CODEC_TYPE_MJPEG);
-    *message_sizes_end = p - message + 2;
-    ++message_sizes_end;
-
-    p = add_format(p, 640, 480, SPICE_VIDEO_CODEC_TYPE_VP9);
-
-    // this split the second format in half
-    *message_sizes_end = p - message - 4;
-    ++message_sizes_end;
-
-    *message_sizes_end = p - message;
-    ++message_sizes_end;
-
-    // add a message to stop data to be read
-    p = add_stream_hdr(p, STREAM_TYPE_INVALID, 0);
-    *message_sizes_end = p - message;
-    ++message_sizes_end;
-
-    // this message should not be read
-    p = add_stream_hdr(p, STREAM_TYPE_INVALID, 0);
-    *message_sizes_end = p - message;
-    ++message_sizes_end;
-
-    vmc_instance.base.sif = &vmc_interface.base;
-    spice_server_add_interface(test->server, &vmc_instance.base);
-
-    // device should not have read data before we open it
-    spice_server_char_device_wakeup(&vmc_instance);
-    g_assert_cmpint(pos, ==, 0);
-
-    // we need to open the device and kick the start
-    spice_server_port_event(&vmc_instance, SPICE_PORT_EVENT_OPENED);
-    spice_server_char_device_wakeup(&vmc_instance);
-
-    // make sure first 3 parts are read completely
-    g_assert(message_sizes_curr - message_sizes >= 3);
-    // make sure last part is not read at all
-    g_assert(message_sizes_curr - message_sizes < 5);
+    for (int test_num=0; test_num < 2; ++test_num) {
+        pos = 0;
+        message_sizes_curr = message_sizes;
+        message_sizes_end = message_sizes;
+
+        // add some messages into device buffer
+        // here we are testing the device is reading at least two
+        // consecutive format messages
+        // first message part has 2 extra bytes to check for header split
+        p = add_format(p, 640, 480, SPICE_VIDEO_CODEC_TYPE_MJPEG);
+        *message_sizes_end = p - message + 2;
+        ++message_sizes_end;
+
+        p = add_format(p, 640, 480, SPICE_VIDEO_CODEC_TYPE_VP9);
+
+        // this split the second format in half
+        *message_sizes_end = p - message - 4;
+        ++message_sizes_end;
+
+        *message_sizes_end = p - message;
+        ++message_sizes_end;
+
+        // add a message to stop data to be read
+        p = add_stream_hdr(p, STREAM_TYPE_INVALID, 0);
+        *message_sizes_end = p - message;
+        ++message_sizes_end;
+
+        // this message should not be read
+        p = add_stream_hdr(p, STREAM_TYPE_INVALID, 0);
+        *message_sizes_end = p - message;
+        ++message_sizes_end;
+
+        vmc_instance.base.sif = &vmc_interface.base;
+        spice_server_add_interface(test->server, &vmc_instance.base);
+
+        // device should not have read data before we open it
+        spice_server_char_device_wakeup(&vmc_instance);
+        g_assert_cmpint(pos, ==, 0);
+
+        // we need to open the device and kick the start
+        spice_server_port_event(&vmc_instance, SPICE_PORT_EVENT_OPENED);
+        spice_server_char_device_wakeup(&vmc_instance);
+        spice_server_port_event(&vmc_instance, SPICE_PORT_EVENT_CLOSED);
+
+        // make sure the device readed all, we need this to make sure that
+        // device will be in sync when opened again
+        g_assert(message_sizes_curr - message_sizes == 5);
+    }
 
     test_destroy(test);
     basic_event_loop_destroy();
-- 
2.14.3