[Spice-devel] [PATCH 21/23] server: move SET_ACK to red_channel
Marc-André Lureau
marcandre.lureau at gmail.com
Sat Feb 12 11:55:24 PST 2011
On Fri, Feb 11, 2011 at 6:23 PM, Alon Levy <alevy at redhat.com> wrote:
> ---
> server/red_channel.c | 27 +++++++++++++++++++++++++++
> server/red_channel.h | 11 +++++++++++
> server/red_tunnel_worker.c | 22 ++--------------------
> server/red_worker.c | 35 +++--------------------------------
> 4 files changed, 43 insertions(+), 52 deletions(-)
>
> diff --git a/server/red_channel.c b/server/red_channel.c
> index be1d9f9..dfba8b4 100644
> --- a/server/red_channel.c
> +++ b/server/red_channel.c
> @@ -28,6 +28,7 @@
> #include <errno.h>
> #include "stat.h"
> #include "red_channel.h"
> +#include "generated_marshallers.h"
>
> static PipeItem *red_channel_pipe_get(RedChannel *channel);
> static void red_channel_event(int fd, int event, void *data);
> @@ -249,10 +250,34 @@ static void red_channel_reset_send_data(RedChannel *channel)
> channel->send_data.header->serial = ++channel->send_data.serial;
> }
>
> +void red_channel_push_set_ack(RedChannel *channel)
> +{
> + red_channel_pipe_add_type(channel, PIPE_ITEM_TYPE_SET_ACK);
> +}
> +
I don't really see the need for a function, otherwise, we could have a
function for all the other types.
> +static void red_channel_send_set_ack(RedChannel *channel)
> +{
> + SpiceMsgSetAck ack;
> +
> + ASSERT(channel);
> + red_channel_init_send_data(channel, SPICE_MSG_SET_ACK, NULL);
> + ack.generation = ++channel->ack_data.generation;
> + ack.window = channel->ack_data.client_window;
> + channel->ack_data.messages_window = 0;
> +
> + spice_marshall_msg_set_ack(channel->send_data.marshaller, &ack);
> +
> + red_channel_begin_send_message(channel);
> +}
> +
> static void red_channel_send_item(RedChannel *channel, PipeItem *item)
> {
> red_channel_reset_send_data(channel);
> switch (item->type) {
> + case PIPE_ITEM_TYPE_SET_ACK:
> + red_channel_send_set_ack(channel);
> + free(item);
Why not call free() in release_item() instead?
> + return;
> }
> /* only reached if not handled here */
> channel->send_item(channel, item);
> @@ -261,6 +286,8 @@ static void red_channel_send_item(RedChannel *channel, PipeItem *item)
> static void red_channel_release_item(RedChannel *channel, PipeItem *item, int item_pushed)
> {
> switch (item->type) {
> + case PIPE_ITEM_TYPE_SET_ACK:
> + return;
here?
> }
> /* only reached if not handled here */
> channel->release_item(channel, item, item_pushed);
> diff --git a/server/red_channel.h b/server/red_channel.h
> index c662186..8367ff7 100644
> --- a/server/red_channel.h
> +++ b/server/red_channel.h
> @@ -89,6 +89,16 @@ typedef struct BufDescriptor {
> uint8_t *data;
> } BufDescriptor;
>
> +/* Messages handled by red_channel
> + * SET_ACK - sent to client on channel connection
> + * Note that the numbers don't have to correspond to spice message types,
> + * but we keep the 100 first allocated for base channel approach.
> + * */
> +enum {
> + PIPE_ITEM_TYPE_SET_ACK=1,
> + PIPE_ITEM_TYPE_CHANNEL_BASE=101,
> +};
> +
> typedef struct PipeItem {
> RingItem link;
> int type;
> @@ -226,6 +236,7 @@ void red_channel_pipe_add_type(RedChannel *channel, int pipe_item_type);
>
> void red_channel_ack_zero_messages_window(RedChannel *channel);
> void red_channel_ack_set_client_window(RedChannel *channel, int client_window);
> +void red_channel_push_set_ack(RedChannel *channel);
>
> // TODO: unstaticed for display/cursor channels. they do some specific pushes not through
> // adding elements or on events. but not sure if this is actually required (only result
> diff --git a/server/red_tunnel_worker.c b/server/red_tunnel_worker.c
> index fdbe46e..d481a38 100644
> --- a/server/red_tunnel_worker.c
> +++ b/server/red_tunnel_worker.c
> @@ -67,8 +67,7 @@
> typedef struct TunnelWorker TunnelWorker;
>
> enum {
> - PIPE_ITEM_TYPE_SET_ACK,
> - PIPE_ITEM_TYPE_MIGRATE,
> + PIPE_ITEM_TYPE_MIGRATE = PIPE_ITEM_TYPE_CHANNEL_BASE,
> PIPE_ITEM_TYPE_MIGRATE_DATA,
> PIPE_ITEM_TYPE_TUNNEL_INIT,
> PIPE_ITEM_TYPE_SERVICE_IP_MAP,
> @@ -2334,19 +2333,6 @@ static int tunnel_channel_handle_message(RedChannel *channel, SpiceDataHeader *h
> /* outgoing msgs
> ********************************/
>
> -static void tunnel_channel_send_set_ack(TunnelChannel *channel, PipeItem *item)
> -{
> - ASSERT(channel);
> -
> - channel->base.send_data.u.ack.generation = ++channel->base.ack_data.generation;
> - channel->base.send_data.u.ack.window = CLIENT_ACK_WINDOW;
> -
> - red_channel_init_send_data(&channel->base, SPICE_MSG_SET_ACK, item);
> - red_channel_add_buf(&channel->base, &channel->base.send_data.u.ack, sizeof(SpiceMsgSetAck));
> -
> - red_channel_begin_send_message(&channel->base);
> -}
> -
> static void tunnel_channel_send_migrate(TunnelChannel *channel, PipeItem *item)
> {
> ASSERT(channel);
> @@ -2813,9 +2799,6 @@ static void tunnel_channel_send_item(RedChannel *channel, PipeItem *item)
> TunnelChannel *tunnel_channel = (TunnelChannel *)channel;
>
> switch (item->type) {
> - case PIPE_ITEM_TYPE_SET_ACK:
> - tunnel_channel_send_set_ack(tunnel_channel, item);
> - break;
> case PIPE_ITEM_TYPE_TUNNEL_INIT:
> tunnel_channel_send_init(tunnel_channel, item);
> break;
> @@ -2860,7 +2843,6 @@ static void tunnel_channel_release_pipe_item(RedChannel *channel, PipeItem *item
> return;
> }
> switch (item->type) {
> - case PIPE_ITEM_TYPE_SET_ACK:
> case PIPE_ITEM_TYPE_TUNNEL_INIT:
> free(item);
> break;
> @@ -3409,7 +3391,7 @@ static void tunnel_channel_disconnect(RedChannel *channel)
>
> static void on_new_tunnel_channel(TunnelChannel *channel)
> {
> - red_channel_pipe_add_type(&channel->base, PIPE_ITEM_TYPE_SET_ACK);
> + red_channel_push_set_ack(&channel->base);
>
> if (channel->base.migrate) {
> channel->expect_migrate_data = TRUE;
> diff --git a/server/red_worker.c b/server/red_worker.c
> index 473473c..1659af0 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -231,12 +231,11 @@ enum {
> };
>
> enum {
> - PIPE_ITEM_TYPE_DRAW,
> + PIPE_ITEM_TYPE_DRAW = PIPE_ITEM_TYPE_CHANNEL_BASE,
> PIPE_ITEM_TYPE_INVAL_ONE,
> PIPE_ITEM_TYPE_CURSOR,
> PIPE_ITEM_TYPE_MIGRATE,
> PIPE_ITEM_TYPE_LOCAL_CURSOR,
> - PIPE_ITEM_TYPE_SET_ACK,
> PIPE_ITEM_TYPE_CURSOR_INIT,
> PIPE_ITEM_TYPE_IMAGE,
> PIPE_ITEM_TYPE_STREAM_CREATE,
> @@ -7548,21 +7547,6 @@ static inline void send_qxl_drawable(DisplayChannel *display_channel, Drawable *
> red_lossy_send_qxl_drawable(display_channel->common.worker, display_channel, item);
> }
>
> -static void red_send_set_ack(RedChannel *channel)
> -{
> - SpiceMsgSetAck ack;
> -
> - ASSERT(channel);
> - red_channel_init_send_data(channel, SPICE_MSG_SET_ACK, NULL);
> - ack.generation = ++channel->ack_data.generation;
> - ack.window = channel->ack_data.client_window;
> - channel->ack_data.messages_window = 0;
> -
> - spice_marshall_msg_set_ack(channel->send_data.marshaller, &ack);
> -
> - red_channel_begin_send_message(channel);
> -}
> -
> static inline void red_send_verb(RedChannel *channel, uint16_t verb)
> {
> ASSERT(channel);
> @@ -8130,10 +8114,6 @@ static void display_channel_send_item(RedChannel *base, PipeItem *pipe_item)
> display_channel_send_migrate_data(display_channel);
> free(pipe_item);
> break;
> - case PIPE_ITEM_TYPE_SET_ACK:
> - red_send_set_ack((RedChannel *)display_channel);
> - free(pipe_item);
> - break;
> case PIPE_ITEM_TYPE_IMAGE:
> red_send_image(display_channel, (ImageItem *)pipe_item);
> release_image_item((ImageItem *)pipe_item);
> @@ -8196,10 +8176,6 @@ static void cursor_channel_send_item(RedChannel *channel, PipeItem *pipe_item)
> cursor_channel_send_migrate(cursor_channel);
> free(pipe_item);
> break;
> - case PIPE_ITEM_TYPE_SET_ACK:
> - red_send_set_ack(channel);
> - free(pipe_item);
> - break;
> case PIPE_ITEM_TYPE_CURSOR_INIT:
> red_reset_cursor_cache(cursor_channel);
> red_send_cursor_init(cursor_channel);
> @@ -8674,7 +8650,7 @@ static void on_new_display_channel(RedWorker *worker)
> DisplayChannel *display_channel = worker->display_channel;
> ASSERT(display_channel);
>
> - red_channel_pipe_add_type(&display_channel->common.base, PIPE_ITEM_TYPE_SET_ACK);
> + red_channel_push_set_ack(&display_channel->common.base);
>
> if (display_channel->common.base.migrate) {
> display_channel->expect_migrate_data = TRUE;
> @@ -9160,8 +9136,6 @@ static void display_channel_release_item(RedChannel *channel, PipeItem *item, in
> case PIPE_ITEM_TYPE_IMAGE:
> release_image_item((ImageItem *)item);
> break;
> - case PIPE_ITEM_TYPE_SET_ACK:
> - break;
> default:
> PANIC("invalid item type");
> }
> @@ -9274,7 +9248,7 @@ static void on_new_cursor_channel(RedWorker *worker)
>
> ASSERT(channel);
> red_channel_ack_zero_messages_window(&channel->common.base);
> - red_channel_pipe_add_type(&channel->common.base, PIPE_ITEM_TYPE_SET_ACK);
> + red_channel_push_set_ack(&channel->common.base);
> if (worker->surfaces[0].context.canvas && !channel->common.base.migrate) {
> red_channel_pipe_add_type(&worker->cursor_channel->common.base, PIPE_ITEM_TYPE_CURSOR_INIT);
> }
> @@ -9295,9 +9269,6 @@ static void cursor_channel_release_item(RedChannel *channel, PipeItem *item, int
> case PIPE_ITEM_TYPE_CURSOR:
> red_release_cursor(common->worker, SPICE_CONTAINEROF(item, CursorItem, pipe_data));
> break;
> - case PIPE_ITEM_TYPE_SET_ACK:
> - free(item);
> - break;
> default:
> PANIC("invalid item type");
> }
> --
> 1.7.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