[Spice-devel] [PATCH spice-gtk 3/5] Add support for SPICE_COMMON_CAP_MINI_HEADER
Marc-André Lureau
marcandre.lureau at gmail.com
Tue Jan 10 04:07:45 PST 2012
Hi
On Sun, Jan 8, 2012 at 9:53 AM, Yonit Halperin <yhalperi at redhat.com> wrote:
> Don't send/receive serial and sub_list when the server supports the
> above cap.
> ---
> gtk/channel-base.c | 8 ++-
> gtk/spice-channel-priv.h | 12 ++++-
> gtk/spice-channel.c | 121 ++++++++++++++++++++++++++++++++++------------
> 3 files changed, 103 insertions(+), 38 deletions(-)
>
> diff --git a/gtk/channel-base.c b/gtk/channel-base.c
> index a6a2892..4d93093 100644
> --- a/gtk/channel-base.c
> +++ b/gtk/channel-base.c
> @@ -134,8 +134,9 @@ void spice_channel_handle_migrate(SpiceChannel *channel, SpiceMsgIn *in)
> spice_channel_recv_msg(channel, get_msg_handler, &data);
> if (!data) {
> g_warning("expected SPICE_MSG_MIGRATE_DATA, got empty message");
> - } else if (data->header.type != SPICE_MSG_MIGRATE_DATA) {
> - g_warning("expected SPICE_MSG_MIGRATE_DATA, got %d", data->header.type);
> + } else if (spice_header_wrapper_get_msg_type(data->header) != SPICE_MSG_MIGRATE_DATA) {
> + g_warning("expected SPICE_MSG_MIGRATE_DATA, got %d",
> + spice_header_wrapper_get_msg_type(data->header));
> }
> }
>
> @@ -143,7 +144,8 @@ void spice_channel_handle_migrate(SpiceChannel *channel, SpiceMsgIn *in)
>
> if ((mig->flags & SPICE_MIGRATE_NEED_DATA_TRANSFER) && (data != NULL)) {
> out = spice_msg_out_new(SPICE_CHANNEL(channel), SPICE_MSGC_MIGRATE_DATA);
> - spice_marshaller_add(out->marshaller, data->data, data->header.size);
> + spice_marshaller_add(out->marshaller, data->data,
> + spice_header_wrapper_get_msg_size(data->header));
> spice_msg_out_send_internal(out);
> }
> }
> diff --git a/gtk/spice-channel-priv.h b/gtk/spice-channel-priv.h
> index df35e4c..38ca23c 100644
> --- a/gtk/spice-channel-priv.h
> +++ b/gtk/spice-channel-priv.h
> @@ -37,22 +37,25 @@
> #include "demarshallers.h"
>
> #include "ssl_verify.h"
> +#include "spice-header.h"
>
> G_BEGIN_DECLS
>
> +
> struct _SpiceMsgOut {
> int refcount;
> SpiceChannel *channel;
> SpiceMessageMarshallers *marshallers;
> SpiceMarshaller *marshaller;
> - SpiceDataHeader *header;
> + SpiceHeaderWrapper *header;
> gboolean ro_check;
> };
>
> struct _SpiceMsgIn {
> int refcount;
> SpiceChannel *channel;
> - SpiceDataHeader header;
> + uint8_t header_buf[MAX_SPICE_DATA_HEADER_SIZE];
> + SpiceHeaderWrapper *header;
> uint8_t *data;
> int hpos,dpos;
> uint8_t *parsed;
> @@ -86,6 +89,10 @@ struct _SpiceChannelPrivate {
> unsigned int sasl_decoded_offset;
> #endif
>
> + gboolean use_mini_header;
> + uint64_t out_serial;
> + uint64_t in_serial;
> +
> /* not swapped */
> SpiceSession *session;
> struct coroutine coroutine;
> @@ -109,7 +116,6 @@ struct _SpiceChannelPrivate {
> int connection_id;
> int channel_id;
> int channel_type;
> - int serial;
> SpiceLinkHeader link_hdr;
> SpiceLinkMess link_msg;
> SpiceLinkHeader peer_hdr;
> diff --git a/gtk/spice-channel.c b/gtk/spice-channel.c
> index c4abd7b..2e33cda 100644
> --- a/gtk/spice-channel.c
> +++ b/gtk/spice-channel.c
> @@ -41,6 +41,7 @@
> #include <ctype.h>
>
> #include "gio-coroutine.h"
> +#include "spice-header.h"
>
> static void spice_channel_handle_msg(SpiceChannel *channel, SpiceMsgIn *msg);
> static void spice_channel_write_msg(SpiceChannel *channel, SpiceMsgOut *out);
> @@ -98,7 +99,8 @@ static void spice_channel_init(SpiceChannel *channel)
>
> c = channel->priv = SPICE_CHANNEL_GET_PRIVATE(channel);
>
> - c->serial = 1;
> + c->out_serial = 1;
> + c->in_serial = 1;
> c->fd = -1;
> strcpy(c->name, "?");
> c->caps = g_array_new(FALSE, TRUE, sizeof(guint32));
> @@ -106,6 +108,7 @@ static void spice_channel_init(SpiceChannel *channel)
> c->remote_caps = g_array_new(FALSE, TRUE, sizeof(guint32));
> c->remote_common_caps = g_array_new(FALSE, TRUE, sizeof(guint32));
> spice_channel_set_common_capability(channel, SPICE_COMMON_CAP_PROTOCOL_AUTH_SELECTION);
> + spice_channel_set_common_capability(channel, SPICE_COMMON_CAP_MINI_HEADER);
> g_queue_init(&c->xmit_queue);
> g_static_mutex_init(&c->xmit_queue_lock);
> c->main_thread = g_thread_self();
> @@ -359,6 +362,13 @@ SpiceMsgIn *spice_msg_in_new(SpiceChannel *channel)
> in = spice_new0(SpiceMsgIn, 1);
> in->refcount = 1;
> in->channel = channel;
> + if (channel->priv->use_mini_header) {
> + in->header = SPICE_HEADER_WRAPPER(g_object_new(SPICE_TYPE_MINI_HEADER, NULL));
> + } else {
> + in->header = SPICE_HEADER_WRAPPER(g_object_new(SPICE_TYPE_FULL_HEADER, NULL));
> + }
I suppose that object could be recycled, to avoid reallocation for each message.
Then, you could also drop the of "use_mini_header" boolean in spice-channel.c.
> + spice_header_wrapper_set_data(in->header, in->header_buf);
> +
> return in;
> }
>
> @@ -371,8 +381,8 @@ SpiceMsgIn *spice_msg_in_sub_new(SpiceChannel *channel, SpiceMsgIn *parent,
> g_return_val_if_fail(channel != NULL, NULL);
>
> in = spice_msg_in_new(channel);
> - in->header.type = sub->type;
> - in->header.size = sub->size;
> + spice_header_wrapper_set_msg_type(in->header, sub->type);
> + spice_header_wrapper_set_msg_size(in->header, sub->size);
> in->data = (uint8_t*)(sub+1);
> in->dpos = sub->size;
> in->parent = parent;
> @@ -403,6 +413,7 @@ void spice_msg_in_unref(SpiceMsgIn *in)
> } else {
> free(in->data);
> }
> + g_object_unref(in->header);
> free(in);
> }
>
> @@ -411,7 +422,7 @@ int spice_msg_in_type(SpiceMsgIn *in)
> {
> g_return_val_if_fail(in != NULL, -1);
>
> - return in->header.type;
> + return spice_header_wrapper_get_msg_type(in->header);
> }
>
> G_GNUC_INTERNAL
> @@ -453,10 +464,19 @@ G_GNUC_INTERNAL
> void spice_msg_in_hexdump(SpiceMsgIn *in)
> {
> SpiceChannelPrivate *c = in->channel->priv;
> + uint64_t serial = c->in_serial;
> + int sub_list = 0;
> +
> + if (!c->use_mini_header) {
> + SpiceFullHeader *full_header = SPICE_FULL_HEADER(in->header);
> + serial = spice_full_header_get_msg_serial(full_header);
> + sub_list = spice_full_header_get_msg_sub_list(full_header);
> + }
>
> fprintf(stderr, "--\n<< hdr: %s serial %" PRIu64 " type %d size %d sub-list %d\n",
> - c->name, in->header.serial, in->header.type,
> - in->header.size, in->header.sub_list);
> + c->name,
> + serial, spice_header_wrapper_get_msg_type(in->header),
> + spice_header_wrapper_get_msg_size(in->header), sub_list);
> hexdump("<< msg", in->data, in->dpos);
> }
>
> @@ -464,10 +484,17 @@ G_GNUC_INTERNAL
> void spice_msg_out_hexdump(SpiceMsgOut *out, unsigned char *data, int len)
> {
> SpiceChannelPrivate *c = out->channel->priv;
> + uint64_t serial = c->out_serial;
> + int sub_list = 0;
>
> + if (!c->use_mini_header) {
> + SpiceFullHeader *full_header = SPICE_FULL_HEADER(out->header);
> + serial = spice_full_header_get_msg_serial(full_header);
> + sub_list = spice_full_header_get_msg_sub_list(full_header);
> + }
> fprintf(stderr, "--\n>> hdr: %s serial %" PRIu64 " type %d size %d sub-list %d\n",
> - c->name, out->header->serial, out->header->type,
> - out->header->size, out->header->sub_list);
> + c->name, serial, spice_header_wrapper_get_msg_type(out->header),
> + spice_header_wrapper_get_msg_size(out->header), sub_list);
> hexdump(">> msg", data, len);
> }
>
> @@ -500,6 +527,7 @@ SpiceMsgOut *spice_msg_out_new(SpiceChannel *channel, int type)
> {
> SpiceChannelPrivate *c = channel->priv;
> SpiceMsgOut *out;
> + uint8_t *header_buf;
>
> g_return_val_if_fail(c != NULL, NULL);
>
> @@ -510,12 +538,23 @@ SpiceMsgOut *spice_msg_out_new(SpiceChannel *channel, int type)
>
> out->marshallers = c->marshallers;
> out->marshaller = spice_marshaller_new();
> - out->header = (SpiceDataHeader *)
> - spice_marshaller_reserve_space(out->marshaller, sizeof(SpiceDataHeader));
> - spice_marshaller_set_base(out->marshaller, sizeof(SpiceDataHeader));
> - out->header->serial = c->serial++;
> - out->header->type = type;
> - out->header->sub_list = 0;
> + if (c->use_mini_header) {
> + out->header = SPICE_HEADER_WRAPPER(g_object_new(SPICE_TYPE_MINI_HEADER, NULL));
> + } else {
> + out->header = SPICE_HEADER_WRAPPER(g_object_new(SPICE_TYPE_FULL_HEADER, NULL));
> + }
> +
> + header_buf = spice_marshaller_reserve_space(out->marshaller,
> + spice_header_wrapper_get_header_size(out->header));
> + spice_header_wrapper_set_data(out->header, header_buf);
> + spice_marshaller_set_base(out->marshaller, spice_header_wrapper_get_header_size(out->header));
> + spice_header_wrapper_set_msg_type(out->header, type);
> +
> + if (!c->use_mini_header) {
> + spice_full_header_set_msg_serial(SPICE_FULL_HEADER(out->header), c->out_serial);
> + spice_full_header_set_msg_sub_list(SPICE_FULL_HEADER(out->header), 0);
> + }
> + c->out_serial++;
> return out;
> }
>
> @@ -536,6 +575,7 @@ void spice_msg_out_unref(SpiceMsgOut *out)
> if (out->refcount > 0)
> return;
> spice_marshaller_destroy(out->marshaller);
> + g_object_unref(out->header);
> free(out);
> }
>
> @@ -710,6 +750,7 @@ static void spice_channel_write_msg(SpiceChannel *channel, SpiceMsgOut *out)
> uint8_t *data;
> int free_data;
> size_t len;
> + uint32_t msg_size;
>
> g_return_if_fail(channel != NULL);
> g_return_if_fail(out != NULL);
> @@ -721,8 +762,9 @@ static void spice_channel_write_msg(SpiceChannel *channel, SpiceMsgOut *out)
> return;
> }
>
> - out->header->size =
> - spice_marshaller_get_total_size(out->marshaller) - sizeof(SpiceDataHeader);
> + msg_size = spice_marshaller_get_total_size(out->marshaller) -
> + spice_header_wrapper_get_header_size(out->header);
> + spice_header_wrapper_set_msg_size(out->header, msg_size);
> data = spice_marshaller_linearize(out->marshaller, 0, &len, &free_data);
> /* spice_msg_out_hexdump(out, data, len); */
> spice_channel_write(channel, data, len);
> @@ -1569,7 +1611,8 @@ static void spice_channel_recv_link_msg(SpiceChannel *channel)
> goto error;
> }
> }
> -
> + c->use_mini_header = spice_channel_test_common_capability(channel,
> + SPICE_COMMON_CAP_MINI_HEADER);
> return;
>
> error:
> @@ -1600,54 +1643,66 @@ void spice_channel_recv_msg(SpiceChannel *channel,
> {
> SpiceChannelPrivate *c = channel->priv;
> SpiceMsgIn *in;
> + int header_size;
> + int msg_size;
> + int msg_type;
> + int sub_list_offset = 0;
> int rc;
>
> if (!c->msg_in) {
> c->msg_in = spice_msg_in_new(channel);
> }
> in = c->msg_in;
> + header_size = spice_header_wrapper_get_header_size(in->header);
>
> /* receive message */
> - if (in->hpos < sizeof(in->header)) {
> - rc = spice_channel_read(channel, (uint8_t*)&in->header + in->hpos,
> - sizeof(in->header) - in->hpos);
> + if (in->hpos < header_size) {
> + rc = spice_channel_read(channel, in->header_buf + in->hpos,
> + header_size - in->hpos);
> if (rc < 0) {
> g_critical("recv hdr: %s", strerror(errno));
> return;
> }
> in->hpos += rc;
> - if (in->hpos < sizeof(in->header))
> + if (in->hpos < header_size)
> return;
> - in->data = spice_malloc(in->header.size);
> + in->data = spice_malloc(spice_header_wrapper_get_msg_size(in->header));
> }
> - if (in->dpos < in->header.size) {
> + msg_size = spice_header_wrapper_get_msg_size(in->header);
> + if (in->dpos < msg_size) {
> rc = spice_channel_read(channel, in->data + in->dpos,
> - in->header.size - in->dpos);
> + msg_size - in->dpos);
> if (rc < 0) {
> g_critical("recv msg: %s", strerror(errno));
> return;
> }
> in->dpos += rc;
> - if (in->dpos < in->header.size)
> + if (in->dpos < msg_size)
> return;
> }
>
> - if (in->header.type == SPICE_MSG_LIST || in->header.sub_list) {
> + msg_type = spice_header_wrapper_get_msg_type(in->header);
> + if (!c->use_mini_header) {
> + sub_list_offset = spice_full_header_get_msg_sub_list(SPICE_FULL_HEADER(in->header));
> + }
> +
> + if (msg_type == SPICE_MSG_LIST || sub_list_offset) {
> SpiceSubMessageList *sub_list;
> SpiceSubMessage *sub;
> SpiceMsgIn *sub_in;
> int i;
>
> - sub_list = (SpiceSubMessageList *)(in->data + in->header.sub_list);
> + sub_list = (SpiceSubMessageList *)(in->data + sub_list_offset);
> for (i = 0; i < sub_list->size; i++) {
> sub = (SpiceSubMessage *)(in->data + sub_list->sub_messages[i]);
> sub_in = spice_msg_in_sub_new(channel, in, sub);
> sub_in->parsed = c->parser(sub_in->data, sub_in->data + sub_in->dpos,
> - sub_in->header.type, c->peer_hdr.minor_version,
> + spice_header_wrapper_get_msg_type(sub_in->header),
> + c->peer_hdr.minor_version,
> &sub_in->psize, &sub_in->pfree);
> if (sub_in->parsed == NULL) {
> g_critical("failed to parse sub-message: %s type %d",
> - c->name, sub_in->header.type);
> + c->name, spice_header_wrapper_get_msg_type(sub_in->header));
> return;
> }
> msg_handler(channel, sub_in, data);
> @@ -1665,16 +1720,16 @@ void spice_channel_recv_msg(SpiceChannel *channel,
> }
> }
>
> - if (in->header.type == SPICE_MSG_LIST) {
> + if (msg_type == SPICE_MSG_LIST) {
> goto end;
> }
>
> /* parse message */
> - in->parsed = c->parser(in->data, in->data + in->dpos, in->header.type,
> + in->parsed = c->parser(in->data, in->data + in->dpos, msg_type,
> c->peer_hdr.minor_version, &in->psize, &in->pfree);
> if (in->parsed == NULL) {
> g_critical("failed to parse message: %s type %d",
> - c->name, in->header.type);
> + c->name, msg_type);
> goto end;
> }
>
> @@ -1684,6 +1739,7 @@ void spice_channel_recv_msg(SpiceChannel *channel,
> msg_handler(channel, in, data);
>
> end:
> + c->in_serial++;
> /* release message */
> c->msg_in = NULL;
> spice_msg_in_unref(in);
> @@ -2224,6 +2280,7 @@ static void channel_reset(SpiceChannel *channel, gboolean migrating)
> g_array_set_size(c->caps, 0);
> /* Restore our default capabilities in case the channel gets re-used */
> spice_channel_set_common_capability(channel, SPICE_COMMON_CAP_PROTOCOL_AUTH_SELECTION);
> + spice_channel_set_common_capability(channel, SPICE_COMMON_CAP_MINI_HEADER);
> }
>
> /* system or coroutine context */
> --
> 1.7.6.4
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
--
Marc-André Lureau
More information about the Spice-devel
mailing list