[Spice-devel] [RFC v4 54/62] server/red_worker: split cursor pipe item from cursor item

Marc-André Lureau marcandre.lureau at gmail.com
Mon May 2 16:54:05 PDT 2011


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)

>     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);

> +    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