On 03/20/2018 01:28 PM, Frediano Ziglio wrote:
Looks good, with minor nits.
On 19 Mar 2018, at 17:46, Frediano Ziglio <fzig...@redhat.com> 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 <fzig...@redhat.com>
---
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)
Currently no capability is defined, so:
STREAM_CAP_END = MAX_GUEST_CAPABILITIES_BYTES = 0
+
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;
...
@@ -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) {
Premature optimization.
Extra is not expensive and code is coherent with other part.
+ 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) {
Reading the various sif->read, the convention on return values is a bit
unclear. Most other places seem to check for <= 0. Only handle_msg_format
uses < 0. Someone could teach me why?
Is it possible for sif->read to return 0 on error?
No, 0 is 0 byte in the current buffer which does not mean end of file,
there's no EAGAIN behaviour.
Basically with <=0 you handle either 0 bytes or error while with <0 only
errors.
Is it possible for sif->read to return less than the requested size (e.g.
EINTR)?
There's no EINTR but what can happen is that guest did a partial write
or the buffer was full so the write was truncated. The interface is not
blocking so partial read are normal.
Since, currently, there are no capabilities
dev->hdr.size - dev->msg_pos = 0 - 0 = 0
The call sif->read(sin,buffer,0) returns 0.
Uri
_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
