[Spice-devel] [RFC v4 38/62] server/red_channel: introduce client ring in RedChannel
Marc-André Lureau
marcandre.lureau at gmail.com
Mon May 2 16:55:32 PDT 2011
On Tue, Apr 26, 2011 at 12:55 PM, Alon Levy <alevy at redhat.com> wrote:
> main_channel and red_worker had several locations that still accessed rcc
> directly, so they had to be touched too, but the changes are minimal.
>
> Most changes are in red_channel: drop the single client reference in RedChannel
> and add a ring of channels.
>
> We still are missing:
> * surfaces copying / deleting in red_worker when a new client comes/goes
> * remove the disconnect calls on new connections
> ---
> server/main_channel.c | 8 +-
> server/red_channel.c | 229 +++++++++++++++++++++++++++++++++++++++---------
> server/red_channel.h | 11 ++-
> server/red_worker.c | 75 +++++++++++++----
> 4 files changed, 254 insertions(+), 69 deletions(-)
>
> diff --git a/server/main_channel.c b/server/main_channel.c
> index 74c7f40..c878478 100644
> --- a/server/main_channel.c
> +++ b/server/main_channel.c
> @@ -162,15 +162,13 @@ static void main_disconnect(MainChannel *main_chan)
> red_channel_destroy(&main_chan->base);
> }
>
> -#define MAIN_FOREACH(_link, _main, _mcc) \
> - if ((_main) && ((_mcc) = \
> - SPICE_CONTAINEROF((_main)->base.rcc, MainChannelClient, base)))
> -
> RedClient *main_channel_get_client_by_link_id(MainChannel *main_chan, uint32_t link_id)
> {
> + RingItem *link;
> MainChannelClient *mcc;
>
> - MAIN_FOREACH(link, main_chan, mcc) {
> + RING_FOREACH(link, &main_chan->base.clients) {
> + mcc = SPICE_CONTAINEROF(link, MainChannelClient, base.channel_link);
> if (mcc->link_id == link_id) {
> return mcc->base.client;
> }
> diff --git a/server/red_channel.c b/server/red_channel.c
> index 34916fd..a1422b5 100644
> --- a/server/red_channel.c
> +++ b/server/red_channel.c
> @@ -26,6 +26,7 @@
> #include <fcntl.h>
> #include <unistd.h>
> #include <errno.h>
> +#include "ring.h"
> #include "stat.h"
> #include "red_channel.h"
> #include "generated_marshallers.h"
> @@ -156,7 +157,14 @@ void red_channel_client_receive(RedChannelClient *rcc)
>
> void red_channel_receive(RedChannel *channel)
> {
> - red_channel_client_receive(channel->rcc);
> + RingItem *link;
> + RingItem *next;
> + RedChannelClient *rcc;
> +
> + RING_FOREACH_SAFE(link, next, &channel->clients) {
> + rcc = SPICE_CONTAINEROF(link, RedChannelClient, channel_link);
> + red_channel_client_receive(rcc);
> + }
> }
>
> static void red_peer_handle_outgoing(RedsStream *stream, OutgoingHandler *handler)
> @@ -273,9 +281,11 @@ void red_channel_client_push_set_ack(RedChannelClient *rcc)
>
> #define ALL_CLIENTS(func_name, visitor) \
> void func_name(RedChannel *channel) { \
> - if (channel->rcc) { \
> - visitor(channel->rcc); \
> - } \
> + RingItem *link; \
> + RingItem *next; \
> + RING_FOREACH_SAFE(link, next, &channel->clients) { \
> + visitor(SPICE_CONTAINEROF(link, RedChannelClient, channel_link)); \
> + } \
> }
>
> void red_channel_push_set_ack(RedChannel *channel)
> @@ -362,8 +372,8 @@ static void red_channel_client_pipe_remove(RedChannelClient *rcc, PipeItem *item
>
> static void red_channel_add_client(RedChannel *channel, RedChannelClient *rcc)
> {
> - ASSERT(rcc && !channel->rcc);
> - channel->rcc = rcc;
> + ASSERT(rcc);
> + ring_add(&channel->clients, &rcc->channel_link);
> channel->clients_num++;
> }
>
> @@ -446,6 +456,7 @@ RedChannel *red_channel_create(int size,
>
> channel->core = core;
> channel->migrate = migrate;
> + ring_init(&channel->clients);
>
> channel->incoming_cb.alloc_msg_buf = (alloc_msg_recv_buf_proc)alloc_recv_buf;
> channel->incoming_cb.release_msg_buf = (release_msg_recv_buf_proc)release_recv_buf;
> @@ -515,12 +526,16 @@ void red_channel_client_destroy(RedChannelClient *rcc)
>
> void red_channel_destroy(RedChannel *channel)
> {
> + RingItem *link;
> + RingItem *next;
> +
> if (!channel) {
> return;
> }
> red_channel_pipes_clear(channel);
> - if (channel->rcc) {
> - red_channel_client_destroy(channel->rcc);
> + RING_FOREACH_SAFE(link, next, &channel->clients) {
> + red_channel_client_destroy(
> + SPICE_CONTAINEROF(link, RedChannelClient, channel_link));
> }
> free(channel);
> }
> @@ -539,8 +554,12 @@ void red_channel_client_shutdown(RedChannelClient *rcc)
>
> void red_channel_shutdown(RedChannel *channel)
> {
> - if (channel->rcc) {
> - red_channel_client_shutdown(channel->rcc);
> + RingItem *link;
> + RingItem *next;
> +
> + red_printf("%d", channel->clients_num);
> + RING_FOREACH_SAFE(link, next, &channel->clients) {
> + red_channel_client_shutdown(SPICE_CONTAINEROF(link, RedChannelClient, channel_link));
> }
> red_channel_pipes_clear(channel);
> }
> @@ -598,10 +617,21 @@ void red_channel_client_push(RedChannelClient *rcc)
>
> void red_channel_push(RedChannel *channel)
> {
> - if (!channel || !channel->rcc) {
> + RingItem *link;
> + RingItem *next;
> + RedChannelClient *rcc;
> +
> + if (!channel) {
> return;
> }
> - red_channel_client_push(channel->rcc);
> + RING_FOREACH_SAFE(link, next, &channel->clients) {
> + rcc = SPICE_CONTAINEROF(link, RedChannelClient, channel_link);
> + if (rcc->stream == NULL) {
> + rcc->channel->disconnect(rcc);
> + } else {
> + red_channel_client_push(rcc);
> + }
> + }
> }
>
> static void red_channel_client_init_outgoing_messages_window(RedChannelClient *rcc)
> @@ -785,8 +815,12 @@ void red_channel_client_pipe_add_type(RedChannelClient *rcc, int pipe_item_type)
>
> void red_channel_pipes_add_type(RedChannel *channel, int pipe_item_type)
> {
> - if (channel->rcc) {
> - red_channel_client_pipe_add_type(channel->rcc, pipe_item_type);
> + RingItem *link;
> +
> + RING_FOREACH(link, &channel->clients) {
> + red_channel_client_pipe_add_type(
> + SPICE_CONTAINEROF(link, RedChannelClient, channel_link),
> + pipe_item_type);
> }
> }
>
> @@ -802,7 +836,18 @@ void red_channel_pipe_item_remove(RedChannel *channel, PipeItem *item)
>
> int red_channel_is_connected(RedChannel *channel)
> {
> - return (channel->rcc != NULL) && red_channel_client_is_connected(channel->rcc);
> + RingItem *link;
> +
> + if (!channel || channel->clients_num == 0) {
> + return FALSE;
> + }
> + RING_FOREACH(link, &channel->clients) {
> + if (red_channel_client_is_connected(
> + SPICE_CONTAINEROF(link, RedChannelClient, channel_link))) {
> + return TRUE;
> + }
> + }
> + return FALSE;
> }
>
> void red_channel_client_clear_sent_item(RedChannelClient *rcc)
> @@ -829,10 +874,17 @@ void red_channel_client_pipe_clear(RedChannelClient *rcc)
>
> void red_channel_pipes_clear(RedChannel *channel)
> {
> - if (!channel || !channel->rcc) {
> + RingItem *link;
> + RingItem *next;
> + RedChannelClient *rcc;
> +
> + if (!channel) {
> return;
> }
> - red_channel_client_pipe_clear(channel->rcc);
> + RING_FOREACH_SAFE(link, next, &channel->clients) {
> + rcc = SPICE_CONTAINEROF(link, RedChannelClient, channel_link);
> + red_channel_client_pipe_clear(rcc);
> + }
> }
>
> void red_channel_client_ack_zero_messages_window(RedChannelClient *rcc)
> @@ -849,8 +901,7 @@ static void red_channel_client_unlink(RedChannelClient *rcc)
> {
> ring_remove(&rcc->client_link);
> rcc->client->channels_num--;
> - ASSERT(rcc->channel->rcc == rcc);
> - rcc->channel->rcc = NULL;
> + ring_remove(&rcc->channel_link);
> rcc->channel->clients_num--;
> }
>
> @@ -871,46 +922,94 @@ void red_channel_client_disconnect(RedChannelClient *rcc)
>
> void red_channel_disconnect(RedChannel *channel)
> {
> + RingItem *link;
> + RingItem *next;
> +
> red_channel_pipes_clear(channel);
> - if (channel->rcc) {
> - red_channel_client_disconnect(channel->rcc);
> + RING_FOREACH_SAFE(link, next, &channel->clients) {
> + red_channel_client_disconnect(
> + SPICE_CONTAINEROF(link, RedChannelClient, channel_link));
> }
> }
>
> int red_channel_all_clients_serials_are_zero(RedChannel *channel)
> {
> - return (!channel->rcc || channel->rcc->send_data.serial == 0);
> + RingItem *link;
> + RedChannelClient *rcc;
> +
> + RING_FOREACH(link, &channel->clients) {
> + rcc = SPICE_CONTAINEROF(link, RedChannelClient, channel_link);
> + if (rcc->send_data.serial != 0) {
> + return FALSE;
> + }
> + }
> + return TRUE;
> }
>
> void red_channel_apply_clients(RedChannel *channel, channel_client_visitor v)
> {
> - if (channel->rcc) {
> - v(channel->rcc);
> + RingItem *link;
> + RingItem *next;
> + RedChannelClient *rcc;
> +
> + RING_FOREACH_SAFE(link, next, &channel->clients) {
> + rcc = SPICE_CONTAINEROF(link, RedChannelClient, channel_link);
> + v(rcc);
> }
> }
>
> void red_channel_apply_clients_data(RedChannel *channel, channel_client_visitor_data v, void *data)
> {
> - if (channel->rcc) {
> - v(channel->rcc, data);
> + RingItem *link;
> + RingItem *next;
> + RedChannelClient *rcc;
> +
> + RING_FOREACH_SAFE(link, next, &channel->clients) {
> + rcc = SPICE_CONTAINEROF(link, RedChannelClient, channel_link);
> + v(rcc, data);
> }
> }
>
> void red_channel_set_shut(RedChannel *channel)
> {
> - if (channel->rcc) {
> - channel->rcc->incoming.shut = TRUE;
> + RingItem *link;
> + RedChannelClient *rcc;
> +
> + RING_FOREACH(link, &channel->clients) {
> + rcc = SPICE_CONTAINEROF(link, RedChannelClient, channel_link);
> + rcc->incoming.shut = TRUE;
> }
> }
>
> int red_channel_all_blocked(RedChannel *channel)
> {
> - return !channel || !channel->rcc || channel->rcc->send_data.blocked;
> + RingItem *link;
> + RedChannelClient *rcc;
> +
> + if (!channel || channel->clients_num == 0) {
> + return FALSE;
> + }
> + RING_FOREACH(link, &channel->clients) {
> + rcc = SPICE_CONTAINEROF(link, RedChannelClient, channel_link);
> + if (!rcc->send_data.blocked) {
> + return FALSE;
> + }
> + }
> + return TRUE;
> }
>
> int red_channel_any_blocked(RedChannel *channel)
> {
> - return !channel || !channel->rcc || channel->rcc->send_data.blocked;
> + RingItem *link;
> + RedChannelClient *rcc;
> +
> + RING_FOREACH(link, &channel->clients) {
> + rcc = SPICE_CONTAINEROF(link, RedChannelClient, channel_link);
> + if (rcc->send_data.blocked) {
> + return TRUE;
> + }
> + }
> + return FALSE;
> }
>
> int red_channel_client_blocked(RedChannelClient *rcc)
> @@ -947,10 +1046,11 @@ SpiceDataHeader *red_channel_client_get_header(RedChannelClient *rcc)
>
> int red_channel_get_first_socket(RedChannel *channel)
> {
> - if (!channel->rcc || !channel->rcc->stream) {
> + if (!channel || channel->clients_num == 0) {
> return -1;
> }
> - return channel->rcc->stream->socket;
> + return SPICE_CONTAINEROF(ring_get_head(&channel->clients),
> + RedChannelClient, channel_link)->stream->socket;
> }
>
> int red_channel_client_item_being_sent(RedChannelClient *rcc, PipeItem *item)
> @@ -960,12 +1060,30 @@ int red_channel_client_item_being_sent(RedChannelClient *rcc, PipeItem *item)
>
> int red_channel_item_being_sent(RedChannel *channel, PipeItem *item)
> {
> - return channel->rcc && red_channel_client_item_being_sent(channel->rcc, item);
> + RingItem *link;
> + RedChannelClient *rcc;
> +
> + RING_FOREACH(link, &channel->clients) {
> + rcc = SPICE_CONTAINEROF(link, RedChannelClient, channel_link);
> + if (rcc->send_data.item == item) {
> + return TRUE;
> + }
> + }
> + return FALSE;
> }
>
> int red_channel_no_item_being_sent(RedChannel *channel)
> {
> - return !channel->rcc || channel->rcc->send_data.item == NULL;
> + RingItem *link;
> + RedChannelClient *rcc;
> +
> + RING_FOREACH(link, &channel->clients) {
> + rcc = SPICE_CONTAINEROF(link, RedChannelClient, channel_link);
> + if (rcc->send_data.item != NULL) {
> + return FALSE;
> + }
> + }
> + return TRUE;
> }
>
> int red_channel_client_no_item_being_sent(RedChannelClient *rcc)
> @@ -1066,16 +1184,17 @@ void __red_channel_pipes_create_batch(RedChannel *channel,
> new_pipe_item_t creator, void *data,
> rcc_item_t visitor)
> {
> + RingItem *link;
> RedChannelClient *rcc;
> PipeItem *item;
> int num = 0;
>
> - if (!(rcc = channel->rcc)) {
> - return;
> - }
> - item = (*creator)(rcc, data, num++);
> - if (visitor) {
> - (*visitor)(rcc, item);
> + RING_FOREACH(link, &channel->clients) {
> + rcc = SPICE_CONTAINEROF(link, RedChannelClient, channel_link);
> + item = (*creator)(rcc, data, num++);
> + if (visitor) {
> + (*visitor)(rcc, item);
> + }
> }
> }
>
> @@ -1103,15 +1222,39 @@ void red_channel_pipes_new_add_tail(RedChannel *channel, new_pipe_item_t creator
>
> uint32_t red_channel_max_pipe_size(RedChannel *channel)
> {
> - return channel->rcc ? channel->rcc->pipe_size : 0;
> + RingItem *link;
> + RedChannelClient *rcc;
> + uint32_t pipe_size = 0;
> +
> + RING_FOREACH(link, &channel->clients) {
> + rcc = SPICE_CONTAINEROF(link, RedChannelClient, channel_link);
> + pipe_size = pipe_size > rcc->pipe_size ? pipe_size : rcc->pipe_size;
> + }
> + return pipe_size;
> }
>
> uint32_t red_channel_min_pipe_size(RedChannel *channel)
> {
> - return channel->rcc ? channel->rcc->pipe_size : 0;
> + RingItem *link;
> + RedChannelClient *rcc;
> + uint32_t pipe_size = ~0;
> +
> + RING_FOREACH(link, &channel->clients) {
> + rcc = SPICE_CONTAINEROF(link, RedChannelClient, channel_link);
> + pipe_size = pipe_size < rcc->pipe_size ? pipe_size : rcc->pipe_size;
> + }
> + return pipe_size == ~0 ? 0 : pipe_size;
> }
>
> uint32_t red_channel_sum_pipes_size(RedChannel *channel)
> {
> - return channel->rcc ? channel->rcc->pipe_size : 0;
> + RingItem *link;
> + RedChannelClient *rcc;
> + uint32_t sum = 0;
> +
> + RING_FOREACH(link, &channel->clients) {
> + rcc = SPICE_CONTAINEROF(link, RedChannelClient, channel_link);
> + sum += rcc->pipe_size;
> + }
> + return sum;
> }
Although very unlikely, shouldn't we check for overflow? or return a uint64_t?
> diff --git a/server/red_channel.h b/server/red_channel.h
> index 38043f1..bf12843 100644
> --- a/server/red_channel.h
> +++ b/server/red_channel.h
> @@ -175,7 +175,7 @@ struct RedChannel {
> int migrate;
> int handle_acks;
>
> - RedChannelClient *rcc;
> + Ring clients;
> uint32_t clients_num;
>
> OutgoingHandlerInterface outgoing_cb;
> @@ -202,7 +202,7 @@ struct RedChannel {
> };
>
> /* if one of the callbacks should cause disconnect, use red_channel_shutdown and don't
> - explicitly destroy the channel */
> + * explicitly destroy the channel */
> RedChannel *red_channel_create(int size,
> SpiceCoreInterface *core,
> int migrate, int handle_acks,
> @@ -237,6 +237,7 @@ RedChannel *red_channel_create_parser(int size,
> channel_handle_migrate_flush_mark_proc handle_migrate_flush_mark,
> channel_handle_migrate_data_proc handle_migrate_data,
> channel_handle_migrate_data_get_serial_proc handle_migrate_data_get_serial);
> +
> RedChannelClient *red_channel_client_create(int size, RedChannel *channel, RedClient *client,
> RedsStream *stream);
>
> @@ -386,8 +387,10 @@ struct RedClient {
> };
>
> RedClient *red_client_new();
> -void red_client_destroy(RedClient *client);
> -void red_client_set_main(RedClient *client, MainChannelClient *mcc);
> MainChannelClient *red_client_get_main(RedClient *client);
> +void red_client_set_main(RedClient *client, MainChannelClient *mcc);
> +void red_client_destroy(RedClient *client);
> +void red_client_disconnect(RedClient *client);
> +void red_client_remove_channel(RedClient *client, RedChannelClient *rcc);
>
> #endif
> diff --git a/server/red_worker.c b/server/red_worker.c
> index 3599b05..388509a 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -990,6 +990,53 @@ static void red_wait_pipe_item_sent(RedChannelClient *rcc, PipeItem *item);
> static void dump_bitmap(RedWorker *worker, SpiceBitmap *bitmap, uint32_t group_id);
> #endif
>
> +/*
> + * Macros to make iterating over stuff easier
> + * The two collections we iterate over:
> + * given a channel, iterate over it's clients
> + */
> +
> +#define RCC_FOREACH(link, rcc, channel) \
> + for (link = ring_get_head(&(channel)->clients),\
> + rcc = SPICE_CONTAINEROF(link, RedChannelClient, channel_link);\
> + (link); \
> + (link) = ring_next(&(channel)->clients, link),\
> + rcc = SPICE_CONTAINEROF(link, RedChannelClient, channel_link))
> +
> +#define RCC_FOREACH_SAFE(link, next, rcc, channel) \
> + for (link = ring_get_head(&(channel)->clients), \
> + rcc = SPICE_CONTAINEROF(link, RedChannelClient, channel_link), \
> + (next) = (link) ? ring_next(&(channel)->clients, (link)) : NULL; \
> + (link); \
> + (link) = (next), \
> + (next) = (link) ? ring_next(&(channel)->clients, (link)) : NULL, \
> + rcc = SPICE_CONTAINEROF(link, RedChannelClient, channel_link))
> +
> +#define DCC_FOREACH(link, dcc, channel) \
> + for (link = ring_get_head(&(channel)->clients),\
> + dcc = SPICE_CONTAINEROF(link, DisplayChannelClient, common.base.channel_link);\
> + (link); \
> + (link) = ring_next(&(channel)->clients, link),\
> + dcc = SPICE_CONTAINEROF(link, DisplayChannelClient, common.base.channel_link))
> +
> +#define CCC_FOREACH(link, ccc, channel) \
> + for (link = ring_get_head(&(channel)->clients),\
> + ccc = SPICE_CONTAINEROF(link, CommonChannelClient, base.channel_link);\
> + (link); \
> + (link) = ring_next(&(channel)->clients, link),\
> + ccc = SPICE_CONTAINEROF(link, CommonChannelClient, base.channel_link))
> +
> +#define DCC_TO_WORKER(dcc) \
> + (SPICE_CONTAINEROF((dcc)->common.base.channel, CommonChannel, base)->worker)
> +
> +#define DCC_TO_DC(dcc) SPICE_CONTAINEROF((dcc)->common.base.channel,\
> + DisplayChannel, common.base)
> +
> +#define RCC_TO_DCC(rcc) SPICE_CONTAINEROF((rcc), DisplayChannelClient, common.base)
> +#define RCC_TO_CCC(rcc) SPICE_CONTAINEROF((rcc), CursorChannelClient, common.base)
> +
> +
> +
> #ifdef COMPRESS_STAT
> static void print_compress_stats(DisplayChannel *display_channel)
> {
> @@ -1191,25 +1238,17 @@ static void red_pipe_add_verb(RedChannelClient* rcc, uint16_t verb)
> static inline void red_create_surface_item(RedWorker *worker,
> DisplayChannelClient *dcc, int surface_id);
> static void red_push_surface_image(DisplayChannelClient *dcc, int surface_id);
> +
> static void red_pipes_add_verb(RedChannel *channel, uint16_t verb)
> {
> - RedChannelClient *rcc = channel->rcc;
> + RedChannelClient *rcc;
> + RingItem *link;
>
> - if (!rcc) {
> - return;
> + RCC_FOREACH(link, rcc, channel) {
> + red_pipe_add_verb(rcc, verb);
> }
> - red_pipe_add_verb(rcc, verb);
> }
>
> -#define DCC_TO_WORKER(dcc) \
> - (SPICE_CONTAINEROF((dcc)->common.base.channel, CommonChannel, base)->worker)
> -
> -#define DCC_TO_DC(dcc) SPICE_CONTAINEROF((dcc)->common.base.channel,\
> - DisplayChannel, common.base)
> -
> -#define RCC_TO_DCC(rcc) SPICE_CONTAINEROF((rcc), DisplayChannelClient, common.base)
> -#define RCC_TO_CCC(rcc) SPICE_CONTAINEROF((rcc), CursorChannelClient, common.base)
> -
> static inline void red_handle_drawable_surfaces_client_synced(
> DisplayChannelClient *dcc, Drawable *drawable)
> {
> @@ -4870,13 +4909,15 @@ static void red_display_client_clear_glz_drawables(DisplayChannelClient *dcc)
>
> static void red_display_clear_glz_drawables(DisplayChannel *display_channel)
> {
> - DisplayChannelClient *dcc = display_channel ?
> - RCC_TO_DCC(display_channel->common.base.rcc) : NULL;
> + RingItem *link;
> + DisplayChannelClient *dcc;
>
> - if (!dcc) {
> + if (!display_channel) {
> return;
> }
> - red_display_client_clear_glz_drawables(dcc);
> + DCC_FOREACH(link, dcc, &display_channel->common.base) {
> + red_display_client_clear_glz_drawables(dcc);
> + }
> }
>
> /*
> --
> 1.7.4.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