[Spice-devel] [RFC v4 54/62] server/red_worker: split cursor pipe item from cursor item
Alon Levy
alevy at redhat.com
Mon May 2 23:46:02 PDT 2011
On Tue, May 03, 2011 at 01:54:05AM +0200, Marc-André Lureau wrote:
> On Tue, Apr 26, 2011 at 12:55 PM, Alon Levy <alevy at redhat.com> wrote:
> > Required to support multiple clients.
> > Also changes somewhat the way we produce PIPE_ITEM_TYPE_LOCAL_CURSOR. Btw,
> > I haven't managed to see when we actually produce such an item during my
> > tests.
> >
> > Previously we had a single pipe item per CursorItem, this is impossible
> > with two pipes, which happens when we have two clients.
> > ---
> > server/red_worker.c | 88 +++++++++++++++++++++++++++++---------------------
> > 1 files changed, 51 insertions(+), 37 deletions(-)
> >
> > diff --git a/server/red_worker.c b/server/red_worker.c
> > index 6ab3d7c..19594fe 100644
> > --- a/server/red_worker.c
> > +++ b/server/red_worker.c
> > @@ -315,13 +315,17 @@ enum {
> > };
> >
> > typedef struct CursorItem {
> > - PipeItem pipe_data;
> > uint32_t group_id;
> > int refs;
> > int type;
> > RedCursorCmd *red_cursor;
> > } CursorItem;
> >
> > +typedef struct CursorPipeItem {
> > + PipeItem base;
> > + CursorItem *cursor_item;
> > +} CursorPipeItem;
> > +
> > typedef struct LocalCursor {
> > CursorItem base;
> > SpicePoint16 position;
> > @@ -4422,8 +4426,6 @@ static CursorItem *get_cursor_item(RedWorker *worker, RedCursorCmd *cmd, uint32_
> > PANIC_ON(!(cursor_item = alloc_cursor_item(worker)));
>
> Should we have a "free list" of CursorPipeItem as well as one of
> CursorItem? it looks like we lost that. (note: I am not convinced
> about the previous free list implementation, which is not maintaining
> a contiguous region and is probably unneeded in the first place - ah..
> the glib slice allocator would be welcome here)
I think we should (as I wrote in the answer to the summary patch). We have
to have a separate list. I'm not sure if having a generic PipeItemData with a void*
is good, or it's better to continue to have a channel specific item (like CursorPipeItem)
and have separate free lists.
>
> > cursor_item->refs = 1;
> > - red_channel_pipe_item_init(&worker->cursor_channel->common.base,
> > - &cursor_item->pipe_data, PIPE_ITEM_TYPE_CURSOR);
> > cursor_item->type = CURSOR_TYPE_INVALID;
> > cursor_item->group_id = group_id;
> > cursor_item->red_cursor = cmd;
> > @@ -4431,17 +4433,24 @@ static CursorItem *get_cursor_item(RedWorker *worker, RedCursorCmd *cmd, uint32_
> > return cursor_item;
> > }
> >
> > -static PipeItem *ref_cursor_pipe_item(RedChannelClient *rcc, void *data, int num)
> > +static PipeItem *new_cursor_pipe_item(RedChannelClient *rcc, void *data, int num)
> > {
> > - CursorItem *cursor_item = data;
> > + CursorPipeItem *item = spice_malloc0(sizeof(CursorPipeItem));
> > + RedWorker *worker = SPICE_CONTAINEROF(rcc->channel, CommonChannel, base)->worker;
> >
> > - cursor_item->refs += (num != 0); /* we already reference the first use */
> > - return &cursor_item->pipe_data;
> > + if (worker->cursor && worker->cursor->type == CURSOR_TYPE_LOCAL) {
> > + red_channel_pipe_item_init(rcc->channel, &item->base, PIPE_ITEM_TYPE_LOCAL_CURSOR);
> > + } else {
> > + red_channel_pipe_item_init(rcc->channel, &item->base, PIPE_ITEM_TYPE_CURSOR);
> > + }
> > + item->cursor_item = data;
> > + item->cursor_item->refs++;
>
> Some day it should be something like:
>
> item->cursor_item = spice_object_ref(cursor_item);
>
It can just be cursor_item_ref today?
> > + return &item->base;
> > }
> >
> > void qxl_process_cursor(RedWorker *worker, RedCursorCmd *cursor_cmd, uint32_t group_id)
> > {
> > - CursorItem *item;
> > + CursorItem *cursor_item;
> > int cursor_show = FALSE;
> >
> > if (!cursor_connected(worker)) {
> > @@ -4449,13 +4458,13 @@ void qxl_process_cursor(RedWorker *worker, RedCursorCmd *cursor_cmd, uint32_t gr
> > free(cursor_cmd);
> > return;
> > }
> > - item = get_cursor_item(worker, cursor_cmd, group_id);
> > + cursor_item = get_cursor_item(worker, cursor_cmd, group_id);
> >
> > switch (cursor_cmd->type) {
> > case QXL_CURSOR_SET:
> > worker->cursor_visible = cursor_cmd->u.set.visible;
> > - item->type = CURSOR_TYPE_DEV;
> > - red_set_cursor(worker, item);
> > + cursor_item->type = CURSOR_TYPE_DEV;
> > + red_set_cursor(worker, cursor_item);
> > break;
> > case QXL_CURSOR_MOVE:
> > cursor_show = !worker->cursor_visible;
> > @@ -4475,11 +4484,10 @@ void qxl_process_cursor(RedWorker *worker, RedCursorCmd *cursor_cmd, uint32_t gr
> >
> > if (cursor_connected(worker) && (worker->mouse_mode == SPICE_MOUSE_MODE_SERVER ||
> > cursor_cmd->type != QXL_CURSOR_MOVE || cursor_show)) {
> > - red_channel_pipes_new_add(&worker->cursor_channel->common.base, ref_cursor_pipe_item,
> > - (void*)item);
> > - } else {
> > - red_release_cursor(worker, item);
> > + red_channel_pipes_new_add(&worker->cursor_channel->common.base, new_cursor_pipe_item,
> > + (void*)cursor_item);
> > }
> > + red_release_cursor(worker, cursor_item);
> > }
> >
> > static inline uint64_t red_now()
> > @@ -8264,7 +8272,8 @@ static void red_cursor_marshall_inval(RedChannelClient *rcc,
> > red_marshall_inval(rcc, m, cach_item);
> > }
> >
> > -static void red_marshall_cursor_init(RedChannelClient *rcc, SpiceMarshaller *base_marshaller)
> > +static void red_marshall_cursor_init(RedChannelClient *rcc, SpiceMarshaller *base_marshaller,
> > + PipeItem *pipe_item)
> > {
> > CursorChannel *cursor_channel;
> > CursorChannelClient *ccc = RCC_TO_CCC(rcc);
> > @@ -8276,7 +8285,7 @@ static void red_marshall_cursor_init(RedChannelClient *rcc, SpiceMarshaller *bas
> > cursor_channel = SPICE_CONTAINEROF(rcc->channel, CursorChannel, common.base);
> > worker = cursor_channel->common.worker;
> >
> > - red_channel_client_init_send_data(rcc, SPICE_MSG_CURSOR_INIT, &worker->cursor->pipe_data);
> > + red_channel_client_init_send_data(rcc, SPICE_MSG_CURSOR_INIT, pipe_item);
> > msg.visible = worker->cursor_visible;
> > msg.position = worker->cursor_position;
> > msg.trail_length = worker->cursor_trail_length;
> > @@ -8288,17 +8297,19 @@ static void red_marshall_cursor_init(RedChannelClient *rcc, SpiceMarshaller *bas
> > }
> >
> > static void red_marshall_local_cursor(RedChannelClient *rcc,
> > - SpiceMarshaller *base_marshaller, LocalCursor *cursor)
> > + SpiceMarshaller *base_marshaller, CursorPipeItem *pipe_item)
> > {
> > CursorChannel *cursor_channel = SPICE_CONTAINEROF(rcc->channel, CursorChannel, common.base);
> > CursorChannelClient *ccc = RCC_TO_CCC(rcc);
> > + LocalCursor *cursor = SPICE_CONTAINEROF(pipe_item->cursor_item, LocalCursor, base);
> > +
> > SpiceMsgCursorSet cursor_set;
> > AddBufInfo info;
> > RedWorker *worker;
> >
> > ASSERT(cursor_channel);
> > worker = cursor_channel->common.worker;
> > - red_channel_client_init_send_data(rcc, SPICE_MSG_CURSOR_SET, &cursor->base.pipe_data);
> > + red_channel_client_init_send_data(rcc, SPICE_MSG_CURSOR_SET, &pipe_item->base);
> > cursor_set.position = cursor->position;
> > cursor_set.visible = worker->cursor_visible;
> >
> > @@ -8319,10 +8330,12 @@ static void cursor_channel_marshall_migrate(RedChannelClient *rcc, SpiceMarshall
> > }
> >
> > static void red_marshall_cursor(RedChannelClient *rcc,
> > - SpiceMarshaller *m, CursorItem *cursor)
> > + SpiceMarshaller *m, CursorPipeItem *cursor_pipe_item)
> > {
> > CursorChannel *cursor_channel = SPICE_CONTAINEROF(rcc->channel, CursorChannel, common.base);
> > CursorChannelClient *ccc = RCC_TO_CCC(rcc);
> > + CursorItem *cursor = cursor_pipe_item->cursor_item;
> > + PipeItem *pipe_item = &cursor_pipe_item->base;
> > RedCursorCmd *cmd;
> > RedWorker *worker;
> >
> > @@ -8335,7 +8348,7 @@ static void red_marshall_cursor(RedChannelClient *rcc,
> > case QXL_CURSOR_MOVE:
> > {
> > SpiceMsgCursorMove cursor_move;
> > - red_channel_client_init_send_data(rcc, SPICE_MSG_CURSOR_MOVE, &cursor->pipe_data);
> > + red_channel_client_init_send_data(rcc, SPICE_MSG_CURSOR_MOVE, pipe_item);
> > cursor_move.position = cmd->u.position;
> > spice_marshall_msg_cursor_move(m, &cursor_move);
> > break;
> > @@ -8345,7 +8358,7 @@ static void red_marshall_cursor(RedChannelClient *rcc,
> > SpiceMsgCursorSet cursor_set;
> > AddBufInfo info;
> >
> > - red_channel_client_init_send_data(rcc, SPICE_MSG_CURSOR_SET, &cursor->pipe_data);
> > + red_channel_client_init_send_data(rcc, SPICE_MSG_CURSOR_SET, pipe_item);
> > cursor_set.position = cmd->u.set.position;
> > cursor_set.visible = worker->cursor_visible;
> >
> > @@ -8355,13 +8368,13 @@ static void red_marshall_cursor(RedChannelClient *rcc,
> > break;
> > }
> > case QXL_CURSOR_HIDE:
> > - red_channel_client_init_send_data(rcc, SPICE_MSG_CURSOR_HIDE, &cursor->pipe_data);
> > + red_channel_client_init_send_data(rcc, SPICE_MSG_CURSOR_HIDE, pipe_item);
> > break;
> > case QXL_CURSOR_TRAIL:
> > {
> > SpiceMsgCursorTrail cursor_trail;
> >
> > - red_channel_client_init_send_data(rcc, SPICE_MSG_CURSOR_TRAIL, &cursor->pipe_data);
> > + red_channel_client_init_send_data(rcc, SPICE_MSG_CURSOR_TRAIL, pipe_item);
> > cursor_trail.length = cmd->u.trail.length;
> > cursor_trail.frequency = cmd->u.trail.frequency;
> > spice_marshall_msg_cursor_trail(m, &cursor_trail);
> > @@ -8370,7 +8383,6 @@ static void red_marshall_cursor(RedChannelClient *rcc,
> > default:
> > red_error("bad cursor command %d", cmd->type);
> > }
> > - red_release_cursor(worker, cursor);
> > }
> >
> > static void red_marshall_surface_create(RedChannelClient *rcc,
> > @@ -8483,10 +8495,10 @@ static void cursor_channel_send_item(RedChannelClient *rcc, PipeItem *pipe_item)
> >
> > switch (pipe_item->type) {
> > case PIPE_ITEM_TYPE_CURSOR:
> > - red_marshall_cursor(rcc, m, (CursorItem *)pipe_item);
> > + red_marshall_cursor(rcc, m, SPICE_CONTAINEROF(pipe_item, CursorPipeItem, base));
> > break;
> > case PIPE_ITEM_TYPE_LOCAL_CURSOR:
> > - red_marshall_local_cursor(rcc, m, (LocalCursor *)pipe_item);
> > + red_marshall_local_cursor(rcc, m, SPICE_CONTAINEROF(pipe_item, CursorPipeItem, base));
> > break;
> > case PIPE_ITEM_TYPE_INVAL_ONE:
> > red_cursor_marshall_inval(rcc, m, (CacheItem *)pipe_item);
> > @@ -8503,8 +8515,7 @@ static void cursor_channel_send_item(RedChannelClient *rcc, PipeItem *pipe_item)
> > break;
> > case PIPE_ITEM_TYPE_CURSOR_INIT:
> > red_reset_cursor_cache(rcc);
> > - red_marshall_cursor_init(rcc, m);
> > - free(pipe_item);
> > + red_marshall_cursor_init(rcc, m, pipe_item);
> > break;
> > case PIPE_ITEM_TYPE_INVAL_CURSOR_CACHE:
> > red_reset_cursor_cache(rcc);
> > @@ -9962,11 +9973,17 @@ static void cursor_channel_release_item(RedChannelClient *rcc, PipeItem *item, i
> >
> > ASSERT(item);
> > switch (item->type) {
> > - case PIPE_ITEM_TYPE_CURSOR:
> > - red_release_cursor(common->worker, SPICE_CONTAINEROF(item, CursorItem, pipe_data));
> > - break;
> > - default:
> > - PANIC("invalid item type");
> > + case PIPE_ITEM_TYPE_CURSOR: {
> > + CursorPipeItem *cursor_pipe_item = SPICE_CONTAINEROF(item, CursorPipeItem, base);
> > + red_release_cursor(common->worker, cursor_pipe_item->cursor_item);
> > + free(cursor_pipe_item);
> > + break;
> > + }
> > + case PIPE_ITEM_TYPE_CURSOR_INIT:
> > + free(item);
> > + break;
> > + default:
> > + PANIC("invalid item type");
> > }
> > }
> >
> > @@ -10034,9 +10051,6 @@ static LocalCursor *_new_local_cursor(RedChannel *channel,
> > LocalCursor *local;
> >
> > local = (LocalCursor *)spice_malloc0(sizeof(LocalCursor) + data_size);
> > -
> > - red_channel_pipe_item_init(channel, &local->base.pipe_data,
> > - PIPE_ITEM_TYPE_LOCAL_CURSOR);
> > local->base.refs = 1;
> > local->base.type = CURSOR_TYPE_LOCAL;
> >
> > --
> > 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