[Spice-devel] [RFC PATCH spice-server v4 04/22] stream-device: Start parsing new protocol from guest

[Jonathon Jongsma]

On Fri, 2017-08-25 at 10:53 +0100, Frediano Ziglio wrote:
> Parse the data sent from the guest to the streaming device.

For completeness perhaps add "At the moment, the data is simply
discarded after it is parsed."

> 
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
>  server/stream-device.c | 115
> ++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 110 insertions(+), 5 deletions(-)
> 
> diff --git a/server/stream-device.c b/server/stream-device.c
> index f3a147b8..e343f1f5 100644
> --- a/server/stream-device.c
> +++ b/server/stream-device.c
> @@ -19,6 +19,8 @@
>  #include <config.h>
>  #endif
>  
> +#include <spice/stream-device.h>
> +
>  #include "char-device.h"
>  
>  #define TYPE_STREAM_DEVICE stream_device_get_type()
> @@ -35,6 +37,9 @@ typedef struct StreamDeviceClass StreamDeviceClass;
>  
>  struct StreamDevice {
>      RedCharDevice parent;
> +    StreamDevHeader hdr;
> +    uint8_t hdr_pos;
> +    bool has_error;
>  };
>  
>  struct StreamDeviceClass {
> @@ -46,24 +51,124 @@ static StreamDevice
> *stream_device_new(SpiceCharDeviceInstance *sin, RedsState *
>  
>  G_DEFINE_TYPE(StreamDevice, stream_device, RED_TYPE_CHAR_DEVICE)
>  
> +typedef void StreamMsgHandler(StreamDevice *dev,
> SpiceCharDeviceInstance *sin);
> +
> +static StreamMsgHandler handle_msg_format, handle_msg_data,
> handle_msg_invalid;
> +
>  static RedPipeItem *
>  stream_device_read_msg_from_dev(RedCharDevice *self,
> SpiceCharDeviceInstance *sin)
>  {
> +    StreamDevice *dev = STREAM_DEVICE(self);
>      SpiceCharDeviceInterface *sif;
>      int n;
>  
> +    if (dev->has_error) {
> +        return NULL;
> +    }
> +
>      sif = spice_char_device_get_interface(sin);
>  
> -    do {
> -        uint8_t buf[256];
> -        n = sif->read(sin, buf, sizeof(buf));
> -        spice_debug("read %d bytes from device", n);
> -    } while (n > 0);
> +    /* read header */
> +    while (dev->hdr_pos < sizeof(dev->hdr)) {
> +        n = sif->read(sin, (uint8_t *) &dev->hdr, sizeof(dev->hdr) -
> dev->hdr_pos);
> +        if (n <= 0) {
> +            return NULL;
> +        }
> +        dev->hdr_pos += n;
> +        if (dev->hdr_pos >= sizeof(dev->hdr)) {
> +            dev->hdr.type = GUINT16_FROM_LE(dev->hdr.type);
> +            dev->hdr.size = GUINT32_FROM_LE(dev->hdr.size);
> +        }
> +    }
> +
> +    switch ((StreamMsgType) dev->hdr.type) {
> +    case STREAM_TYPE_FORMAT:
> +        if (dev->hdr.size != sizeof(StreamMsgFormat)) {
> +            handle_msg_invalid(dev, sin);
> +        } else {
> +            handle_msg_format(dev, sin);
> +        }
> +        break;
> +    case STREAM_TYPE_DATA:
> +        handle_msg_data(dev, sin);
> +        break;
> +    case STREAM_TYPE_CAPABILITIES:
> +        /* FIXME */
> +    default:
> +        handle_msg_invalid(dev, sin);
> +        break;
> +    }
>  
>      return NULL;
>  }
>  
>  static void
> +handle_msg_invalid(StreamDevice *dev, SpiceCharDeviceInstance *sin)
> +{
> +    static const char error_msg[] = "Wrong protocol";

Perhaps "Protocol error" would be more accurate than "Wrong protocol"?
Alternately, we could add a third const char* argument so that teh
calling function can customize the error message? (e.g. "Invalid
message type", "Wrong size for StreamMsgFormat", etc.)


> +
> +    int msg_size = sizeof(StreamMsgNotifyError) + strlen(error_msg)
> + 1;
> +    int total_size = sizeof(StreamDevHeader) + msg_size;
> +
> +    RedCharDevice *char_dev = RED_CHAR_DEVICE(dev);
> +    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_NOTIFY_ERROR);
> +    hdr->size = GUINT32_TO_LE(msg_size);
> +
> +    StreamMsgNotifyError *const error = (StreamMsgNotifyError
> *)(hdr+1);
> +    error->error_code = GUINT32_TO_LE(0);
> +    strcpy((char *) error->msg, error_msg);
> +
> +    red_char_device_write_buffer_add(char_dev, buf);
> +
> +    dev->has_error = true;
> +}
> +
> +static void
> +handle_msg_format(StreamDevice *dev, SpiceCharDeviceInstance *sin)
> +{
> +    StreamMsgFormat fmt;
> +    SpiceCharDeviceInterface *sif =
> spice_char_device_get_interface(sin);
> +    int n = sif->read(sin, (uint8_t *) &fmt, sizeof(fmt));
> +    if (n == 0) {
> +        return;
> +    }
> +    if (n != sizeof(fmt)) {
> +        handle_msg_invalid(dev, sin);
> +        return;
> +    }
> +    fmt.width = GUINT32_FROM_LE(fmt.width);
> +    fmt.height = GUINT32_FROM_LE(fmt.height);
> +    dev->hdr_pos = 0;
> +}
> +
> +static void
> +handle_msg_data(StreamDevice *dev, SpiceCharDeviceInstance *sin)
> +{
> +    SpiceCharDeviceInterface *sif =
> spice_char_device_get_interface(sin);
> +    int n;
> +    while (1) {
> +        uint8_t buf[16 * 1024];
> +        n = sif->read(sin, buf, sizeof(buf));
> +        /* TODO */
> +        spice_debug("read %d bytes from device", n);
> +        if (n <= 0) {
> +            break;
> +        }
> +        dev->hdr.size -= n;
> +    }
> +    if (dev->hdr.size == 0) {
> +        dev->hdr_pos = 0;
> +    }
> +}
> +
> +static void
>  stream_device_send_msg_to_client(RedCharDevice *self, RedPipeItem
> *msg, RedClient *client)
>  {
>  }


Acked-by: Jonathon Jongsma <jjongsma at redhat.com>