[Spice-devel] [PATCH 06/11] server: move some cursor code to cursor-channel.c

Frediano Ziglio fziglio at redhat.com
Wed Oct 28 09:07:01 PDT 2015


> 
> From: Marc-André Lureau <marcandre.lureau at gmail.com>
> 
> Also fix warning due to unexpected pipe item type
> 
> The specific item type that was not being handled was
> PIPE_ITEM_TYPE_INVAL_ONE (#102). This item type is used by the cursor
> channel, but the analogous item for the display channel is
> PIPE_ITEM_TYPE_INVAL_PALETTE_CACHE. Use this value instead.
> 
> The exact warning follows:
> 
>     (/usr/bin/qemu-kvm:24458): Spice-Warning **:
>     ../../server/dcc-send.c:2442:dcc_send_item: should not be reached
>     (/usr/bin/qemu-kvm:24458): Spice-CRITICAL **:
>     ../../server/dcc.c:1595:release_item_before_push: invalid item type
> 
> Author:    Marc-André Lureau <marcandre.lureau at gmail.com>
> Date:      Thu Feb 27 19:38:58 2014 +0200
> ---
>  server/cache_item.tmpl.c |   4 +-
>  server/cursor-channel.c  | 219
>  ++++++++++++++++++++++++++---------------------
>  server/cursor-channel.h  |  27 +++---
>  server/red_dispatcher.c  |   1 +
>  server/red_worker.c      | 200 +++++++++++++++++++------------------------
>  server/red_worker.h      |  65 +++++---------
>  6 files changed, 255 insertions(+), 261 deletions(-)
> 
> diff --git a/server/cache_item.tmpl.c b/server/cache_item.tmpl.c
> index dc314c0..ad2b579 100644
> --- a/server/cache_item.tmpl.c
> +++ b/server/cache_item.tmpl.c
> @@ -21,6 +21,7 @@
>  #define CACHE_HASH_KEY CURSOR_CACHE_HASH_KEY
>  #define CACHE_HASH_SIZE CURSOR_CACHE_HASH_SIZE
>  #define CACHE_INVAL_TYPE SPICE_MSG_CURSOR_INVAL_ONE
> +#define PIPE_ITEM_TYPE PIPE_ITEM_TYPE_INVAL_ONE

I would prefer a PIPE_ITEM_TYPE_INVALID macro here.
Is not clear which part of the patch is fixing the "unexpected pipe item type".

>  #define FUNC_NAME(name) red_cursor_cache_##name
>  #define VAR_NAME(name) cursor_cache_##name
>  #define CHANNEL CursorChannel
> @@ -32,6 +33,7 @@
>  #define CACHE_HASH_KEY PALETTE_CACHE_HASH_KEY
>  #define CACHE_HASH_SIZE PALETTE_CACHE_HASH_SIZE
>  #define CACHE_INVAL_TYPE SPICE_MSG_DISPLAY_INVAL_PALETTE
> +#define PIPE_ITEM_TYPE PIPE_ITEM_TYPE_INVAL_PALETTE_CACHE
>  #define FUNC_NAME(name) red_palette_cache_##name
>  #define VAR_NAME(name) palette_cache_##name
>  #define CHANNEL DisplayChannel
> @@ -78,7 +80,7 @@ static void FUNC_NAME(remove)(CHANNELCLIENT
> *channel_client, CacheItem *item)
>      channel_client->VAR_NAME(items)--;
>      channel_client->VAR_NAME(available) += item->size;
>  
> -    red_channel_pipe_item_init(&channel->common.base, &item->u.pipe_data,
> PIPE_ITEM_TYPE_INVAL_ONE);
> +    red_channel_pipe_item_init(&channel->common.base, &item->u.pipe_data,
> PIPE_ITEM_TYPE);
>      red_channel_client_pipe_add_tail(&channel_client->common.base,
>      &item->u.pipe_data); // for now
>  }
>  
> diff --git a/server/cursor-channel.c b/server/cursor-channel.c
> index 7ba4a7d..adf7d78 100644
> --- a/server/cursor-channel.c
> +++ b/server/cursor-channel.c
> @@ -25,57 +25,64 @@
>  #include "cache_item.tmpl.c"
>  #undef CLIENT_CURSOR_CACHE
>  
> -static inline CursorItem *alloc_cursor_item(void)
> +CursorItem *cursor_item_new(QXLInstance *qxl, RedCursorCmd *cmd, uint32_t
> group_id)
>  {
>      CursorItem *cursor_item;
>  
> +    spice_return_val_if_fail(cmd != NULL, NULL);
> +
>      cursor_item = g_slice_new0(CursorItem);
> +    cursor_item->qxl = qxl;
>      cursor_item->refs = 1;
> +    cursor_item->group_id = group_id;
> +    cursor_item->red_cursor = cmd;
>  
>      return cursor_item;
>  }
>  
> -CursorItem *cursor_item_new(RedCursorCmd *cmd, uint32_t group_id)
> +CursorItem *cursor_item_ref(CursorItem *item)
>  {
> -    CursorItem *cursor_item;
> +    spice_return_val_if_fail(item != NULL, NULL);
> +    spice_return_val_if_fail(item->refs > 0, NULL);

This is detecting a memory error, perhaps a spice_error or a spice_critical
should be better.

>  
> -    spice_return_val_if_fail(cmd != NULL, NULL);
> -    spice_warn_if(!(cursor_item = alloc_cursor_item()));
> -
> -    cursor_item->group_id = group_id;
> -    cursor_item->red_cursor = cmd;
> +    item->refs++;
>  
> -    return cursor_item;
> +    return item;
>  }
>  
> -void cursor_item_unref(QXLInstance *qxl, CursorItem *cursor)
> +void cursor_item_unref(CursorItem *item)
>  {
> -    if (!--cursor->refs) {
> -        QXLReleaseInfoExt release_info_ext;
> -        RedCursorCmd *cursor_cmd;
> -
> -        cursor_cmd = cursor->red_cursor;
> -        release_info_ext.group_id = cursor->group_id;
> -        release_info_ext.info = cursor_cmd->release_info;
> -        qxl->st->qif->release_resource(qxl, release_info_ext);
> -        red_put_cursor_cmd(cursor_cmd);
> -        free(cursor_cmd);
> -
> -        g_slice_free(CursorItem, cursor);
> -    }
> +    QXLReleaseInfoExt release_info_ext;
> +    RedCursorCmd *cursor_cmd;
> +
> +    spice_return_if_fail(item != NULL);
> +
> +    if (--item->refs)
> +        return;
> +
> +    cursor_cmd = item->red_cursor;
> +    release_info_ext.group_id = item->group_id;
> +    release_info_ext.info = cursor_cmd->release_info;
> +    item->qxl->st->qif->release_resource(item->qxl, release_info_ext);
> +    red_put_cursor_cmd(cursor_cmd);
> +    free(cursor_cmd);
> +
> +    g_slice_free(CursorItem, item);
> +
>  }
>  

There are a lot of renames cursor_channel -> cursor and cursor -> item (here
and in following hunks). Should we remove them ?
Which naming should we use ?

>  static void cursor_set_item(CursorChannel *cursor, CursorItem *item)
>  {
>      if (cursor->item)
> -        cursor_item_unref(red_worker_get_qxl(cursor->common.worker),
> cursor->item);
> +        cursor_item_unref(cursor->item);
>  
> -    if (item)
> -        item->refs++;
> -
> -    cursor->item = item;
> +    cursor->item = item ? cursor_item_ref(item) : NULL;
>  }
>  

qxl field in CursorItem is added mainly to be able to free it without passing a
QXLInstance object which can be computed quite easily from CursorChannel.
Is it worth adding the qxl field ?

> +#ifdef DEBUG_CURSORS
> +static int _cursor_count = 0;
> +#endif
> +

This variable is unused, I'll remove it in the next patch.

>  static PipeItem *new_cursor_pipe_item(RedChannelClient *rcc, void *data, int
>  num)
>  {
>      CursorPipeItem *item = spice_malloc0(sizeof(CursorPipeItem));
> @@ -135,11 +142,15 @@ static void red_reset_cursor_cache(RedChannelClient
> *rcc)
>      red_cursor_cache_reset(RCC_TO_CCC(rcc), CLIENT_CURSOR_CACHE_SIZE);
>  }
>  
> -void cursor_channel_disconnect(RedChannel *channel)
> +void cursor_channel_disconnect(CursorChannel *cursor)
>  {

I agree for coherence with this type change.

> +    RedChannel *channel = (RedChannel *)cursor;
> +
>      if (!channel || !red_channel_is_connected(channel)) {
>          return;
>      }
> +
> +    /* TODO: use a class vdispose */

I don't understand this comment.

>      red_channel_apply_clients(channel, red_reset_cursor_cache);
>      red_channel_disconnect(channel);
>  }
> @@ -147,7 +158,8 @@ void cursor_channel_disconnect(RedChannel *channel)
>  
>  static void put_cursor_pipe_item(CursorChannelClient *ccc, CursorPipeItem
>  *pipe_item)
>  {
> -    spice_assert(pipe_item);
> +    spice_return_if_fail(pipe_item);
> +    spice_return_if_fail(pipe_item->refs > 0);
>  

Second check is checking a memory corruption, I suggest spice_error/spice_critical.

>      if (--pipe_item->refs) {
>          return;
> @@ -155,7 +167,7 @@ static void put_cursor_pipe_item(CursorChannelClient
> *ccc, CursorPipeItem *pipe_
>  
>      spice_assert(!pipe_item_is_linked(&pipe_item->base));
>  
> -    cursor_item_unref(red_worker_get_qxl(ccc->common.worker),
> pipe_item->cursor_item);
> +    cursor_item_unref(pipe_item->cursor_item);
>      free(pipe_item);
>  }
>  
> @@ -206,37 +218,37 @@ static void
> cursor_channel_client_release_item_after_push(CursorChannelClient *c
>  static void red_marshall_cursor_init(RedChannelClient *rcc, SpiceMarshaller
>  *base_marshaller,
>                                       PipeItem *pipe_item)
>  {
> -    CursorChannel *cursor_channel;
> +    CursorChannel *cursor;
>      CursorChannelClient *ccc = RCC_TO_CCC(rcc);
>      SpiceMsgCursorInit msg;
>      AddBufInfo info;
>  
>      spice_assert(rcc);
> -    cursor_channel = SPICE_CONTAINEROF(rcc->channel, CursorChannel,
> common.base);
> +    cursor = SPICE_CONTAINEROF(rcc->channel, CursorChannel, common.base);
>  
>      red_channel_client_init_send_data(rcc, SPICE_MSG_CURSOR_INIT, NULL);
> -    msg.visible = cursor_channel->cursor_visible;
> -    msg.position = cursor_channel->cursor_position;
> -    msg.trail_length = cursor_channel->cursor_trail_length;
> -    msg.trail_frequency = cursor_channel->cursor_trail_frequency;
> +    msg.visible = cursor->cursor_visible;
> +    msg.position = cursor->cursor_position;
> +    msg.trail_length = cursor->cursor_trail_length;
> +    msg.trail_frequency = cursor->cursor_trail_frequency;
>  
> -    cursor_fill(ccc, &msg.cursor, cursor_channel->item, &info);
> +    cursor_fill(ccc, &msg.cursor, cursor->item, &info);
>      spice_marshall_msg_cursor_init(base_marshaller, &msg);
>      add_buf_from_info(base_marshaller, &info);
>  }
>  
>  static void cursor_marshall(RedChannelClient *rcc,
> -                   SpiceMarshaller *m, CursorPipeItem *cursor_pipe_item)
> +                            SpiceMarshaller *m, CursorPipeItem
> *cursor_pipe_item)
>  {
> -    CursorChannel *cursor_channel = SPICE_CONTAINEROF(rcc->channel,
> CursorChannel, common.base);
> +    CursorChannel *cursor = SPICE_CONTAINEROF(rcc->channel, CursorChannel,
> common.base);
>      CursorChannelClient *ccc = RCC_TO_CCC(rcc);
> -    CursorItem *cursor = cursor_pipe_item->cursor_item;
> +    CursorItem *item = cursor_pipe_item->cursor_item;
>      PipeItem *pipe_item = &cursor_pipe_item->base;
>      RedCursorCmd *cmd;
>  
> -    spice_assert(cursor_channel);
> +    spice_return_if_fail(cursor);
>  

Quite a big change in behavior.

> -    cmd = cursor->red_cursor;
> +    cmd = item->red_cursor;
>      switch (cmd->type) {
>      case QXL_CURSOR_MOVE:
>          {
> @@ -253,9 +265,9 @@ static void cursor_marshall(RedChannelClient *rcc,
>  
>              red_channel_client_init_send_data(rcc, SPICE_MSG_CURSOR_SET,
>              pipe_item);
>              cursor_set.position = cmd->u.set.position;
> -            cursor_set.visible = cursor_channel->cursor_visible;
> +            cursor_set.visible = cursor->cursor_visible;
>  
> -            cursor_fill(ccc, &cursor_set.cursor, cursor, &info);
> +            cursor_fill(ccc, &cursor_set.cursor, item, &info);
>              spice_marshall_msg_cursor_set(m, &cursor_set);
>              add_buf_from_info(m, &info);
>              break;
> @@ -279,7 +291,7 @@ static void cursor_marshall(RedChannelClient *rcc,
>  }
>  
>  static inline void red_marshall_inval(RedChannelClient *rcc,
> -        SpiceMarshaller *base_marshaller, CacheItem *cach_item)
> +                                      SpiceMarshaller *base_marshaller,
> CacheItem *cach_item)
>  {
>      SpiceMsgDisplayInvalOne inval_one;
>  
> @@ -289,13 +301,6 @@ static inline void red_marshall_inval(RedChannelClient
> *rcc,
>      spice_marshall_msg_cursor_inval_one(base_marshaller, &inval_one);
>  }
>  
> -static void red_cursor_marshall_inval(RedChannelClient *rcc,
> -                SpiceMarshaller *m, CacheItem *cach_item)
> -{
> -    spice_assert(rcc);
> -    red_marshall_inval(rcc, m, cach_item);
> -}
> -

Inlined, I agree with the change.

>  static void cursor_channel_send_item(RedChannelClient *rcc, PipeItem
>  *pipe_item)
>  {
>      SpiceMarshaller *m = red_channel_client_get_marshaller(rcc);
> @@ -306,10 +311,10 @@ static void cursor_channel_send_item(RedChannelClient
> *rcc, PipeItem *pipe_item)
>          cursor_marshall(rcc, m, SPICE_CONTAINEROF(pipe_item, CursorPipeItem,
>          base));
>          break;
>      case PIPE_ITEM_TYPE_INVAL_ONE:
> -        red_cursor_marshall_inval(rcc, m, (CacheItem *)pipe_item);
> +        red_marshall_inval(rcc, m, (CacheItem *)pipe_item);
>          break;
>      case PIPE_ITEM_TYPE_VERB:
> -        red_marshall_verb(rcc, ((VerbItem*)pipe_item)->verb);
> +        red_marshall_verb(rcc, (VerbItem*)pipe_item);
>          break;
>      case PIPE_ITEM_TYPE_CURSOR_INIT:
>          red_reset_cursor_cache(rcc);
> @@ -317,7 +322,7 @@ static void cursor_channel_send_item(RedChannelClient
> *rcc, PipeItem *pipe_item)
>          break;
>      case PIPE_ITEM_TYPE_INVAL_CURSOR_CACHE:
>          red_reset_cursor_cache(rcc);
> -        red_marshall_verb(rcc, SPICE_MSG_CURSOR_INVAL_ALL);
> +        red_channel_client_init_send_data(rcc, SPICE_MSG_CURSOR_INVAL_ALL,
> NULL);
>          break;
>      default:
>          spice_error("invalid pipe item type");
> @@ -329,7 +334,9 @@ static void cursor_channel_send_item(RedChannelClient
> *rcc, PipeItem *pipe_item)
>  
>  static CursorPipeItem *cursor_pipe_item_ref(CursorPipeItem *item)
>  {
> -    spice_assert(item);
> +    spice_return_val_if_fail(item, NULL);
> +    spice_return_val_if_fail(item->refs > 0, NULL);
> +

Same as above, more restrict check.

>      item->refs++;
>      return item;
>  }
> @@ -338,8 +345,11 @@ static CursorPipeItem
> *cursor_pipe_item_ref(CursorPipeItem *item)
>  static void cursor_channel_hold_pipe_item(RedChannelClient *rcc, PipeItem
>  *item)
>  {
>      CursorPipeItem *cursor_pipe_item;
> -    spice_assert(item);
> +
> +    spice_return_if_fail(item);
> +

Why this change?

>      cursor_pipe_item = SPICE_CONTAINEROF(item, CursorPipeItem, base);
> +    /* TODO: refcnt at PipeItem instead ? */

I don't understand this comment.

>      cursor_pipe_item_ref(cursor_pipe_item);
>  }
>  
> @@ -357,51 +367,56 @@ static void
> cursor_channel_release_item(RedChannelClient *rcc, PipeItem *item, i
>      }
>  }
>  
> -CursorChannel* cursor_channel_new(RedWorker *worker, int migrate)
> +CursorChannel* cursor_channel_new(RedWorker *worker)
>  {

The migrate field was not used, perhaps should be split and merged in another patch?
I'm ok with it.

> -    CursorChannel* cursor;
> +    CursorChannel *cursor;
> +    RedChannel *channel = NULL;
> +    ChannelCbs cbs = {
> +        .on_disconnect =  cursor_channel_client_on_disconnect,
> +        .send_item = cursor_channel_send_item,
> +        .hold_item = cursor_channel_hold_pipe_item,
> +        .release_item = cursor_channel_release_item
> +    };
>  
>      spice_info("create cursor channel");
> -    cursor = (CursorChannel *)__new_channel(
> -        worker, sizeof(CursorChannel),
> -        SPICE_CHANNEL_CURSOR,
> -        0,
> -        cursor_channel_client_on_disconnect,
> -        cursor_channel_send_item,
> -        cursor_channel_hold_pipe_item,
> -        cursor_channel_release_item,
> -        red_channel_client_handle_message,
> -        NULL,
> -        NULL,
> -        NULL);
> +    channel = red_worker_new_channel(worker, sizeof(CursorChannel),
> +                                     SPICE_CHANNEL_CURSOR, 0,
> +                                     &cbs,
> red_channel_client_handle_message);
>  
> +    cursor = (CursorChannel *)channel;
>      cursor->cursor_visible = TRUE;
>      cursor->mouse_mode = SPICE_MOUSE_MODE_SERVER;
>  
>      return cursor;
>  }
>  
> -CursorChannelClient *cursor_channel_client_new(CommonChannel *common,
> -                                               RedClient *client, RedsStream
> *stream,
> +CursorChannelClient* cursor_channel_client_new(CursorChannel *cursor,
> RedClient *client, RedsStream *stream,
>                                                 int mig_target,
>                                                 uint32_t *common_caps, int
>                                                 num_common_caps,
>                                                 uint32_t *caps, int num_caps)
>  {
> +    spice_return_val_if_fail(cursor, NULL);
> +    spice_return_val_if_fail(client, NULL);
> +    spice_return_val_if_fail(stream, NULL);
> +    spice_return_val_if_fail(!num_common_caps || common_caps, NULL);
> +    spice_return_val_if_fail(!num_caps || caps, NULL);
> +
>      CursorChannelClient *ccc =
> -        (CursorChannelClient*)common_channel_client_create(
> -            sizeof(CursorChannelClient), common, client, stream,
> -            mig_target,
> -            FALSE,
> -            common_caps,
> -            num_common_caps,
> -            caps,
> -            num_caps);
> -
> -    if (!ccc) {
> -        return NULL;
> -    }
> +        (CursorChannelClient*)common_channel_new_client(&cursor->common,
> +
> sizeof(CursorChannelClient),
> +                                                        client, stream,
> +                                                        mig_target,
> +                                                        FALSE,
> +                                                        common_caps,
> +                                                        num_common_caps,
> +                                                        caps,
> +                                                        num_caps);
> +    spice_return_val_if_fail(ccc != NULL, NULL);
> +
>      ring_init(&ccc->cursor_cache_lru);
>      ccc->cursor_cache_available = CLIENT_CURSOR_CACHE_SIZE;
> +
> +
>      return ccc;
>  }
>  
> @@ -411,7 +426,11 @@ void cursor_channel_process_cmd(CursorChannel *cursor,
> RedCursorCmd *cursor_cmd,
>      CursorItem *cursor_item;
>      int cursor_show = FALSE;
>  
> -    cursor_item = cursor_item_new(cursor_cmd, group_id);
> +    spice_return_if_fail(cursor);
> +    spice_return_if_fail(cursor_cmd);
> +
> +    cursor_item = cursor_item_new(red_worker_get_qxl(cursor->common.worker),
> +                                  cursor_cmd, group_id);
>  
>      switch (cursor_cmd->type) {
>      case QXL_CURSOR_SET:
> @@ -431,34 +450,40 @@ void cursor_channel_process_cmd(CursorChannel *cursor,
> RedCursorCmd *cursor_cmd,
>          cursor->cursor_trail_frequency = cursor_cmd->u.trail.frequency;
>          break;
>      default:
> -        spice_error("invalid cursor command %u", cursor_cmd->type);
> +        spice_warning("invalid cursor command %u", cursor_cmd->type);
> +        return;

If command came from VM I agree to ignore the message.
If not I would prefer the error.

>      }
>  
> -    if (red_channel_is_connected(&cursor->common.base) &&
> (cursor->mouse_mode == SPICE_MOUSE_MODE_SERVER ||
> -                                   cursor_cmd->type != QXL_CURSOR_MOVE ||
> cursor_show)) {
> -        red_channel_pipes_new_add(&cursor->common.base,
> new_cursor_pipe_item,
> -                                  (void*)cursor_item);
> +    if (cursor->mouse_mode == SPICE_MOUSE_MODE_SERVER
> +        || cursor_cmd->type != QXL_CURSOR_MOVE
> +        || cursor_show) {

Why the connection check was removed?

> +        red_channel_pipes_new_add(&cursor->common.base,
> +                                  new_cursor_pipe_item, cursor_item);
>      }
> -    cursor_item_unref(red_worker_get_qxl(cursor->common.worker),
> cursor_item);
> +
> +    cursor_item_unref(cursor_item);
>  }
>  
>  void cursor_channel_reset(CursorChannel *cursor)
>  {
> +    RedChannel *channel = (RedChannel *)cursor;
> +

RedChannel *channel = &cursor->common.base;

is more type safe. Actually just style change.

> +    spice_return_if_fail(cursor);
> +
>      cursor_set_item(cursor, NULL);
>      cursor->cursor_visible = TRUE;
>      cursor->cursor_position.x = cursor->cursor_position.y = 0;
>      cursor->cursor_trail_length = cursor->cursor_trail_frequency = 0;
>  
> -    if (red_channel_is_connected(&cursor->common.base)) {
> -        red_channel_pipes_add_type(&cursor->common.base,
> -                                   PIPE_ITEM_TYPE_INVAL_CURSOR_CACHE);
> +    if (red_channel_is_connected(channel)) {
> +        red_channel_pipes_add_type(&cursor->common.base,
> PIPE_ITEM_TYPE_INVAL_CURSOR_CACHE);
>          if (!cursor->common.during_target_migrate) {
>              red_pipes_add_verb(&cursor->common.base,
>              SPICE_MSG_CURSOR_RESET);
>          }
>          if (!red_channel_wait_all_sent(&cursor->common.base,
>                                         DISPLAY_CLIENT_TIMEOUT)) {
>              red_channel_apply_clients(&cursor->common.base,
> -
> red_channel_client_disconnect_if_pending_send);
> +
> red_channel_client_disconnect_if_pending_send);
>          }
>      }
>  }
> diff --git a/server/cursor-channel.h b/server/cursor-channel.h
> index 2c19859..e1ef4b6 100644
> --- a/server/cursor-channel.h
> +++ b/server/cursor-channel.h
> @@ -32,7 +32,14 @@
>  #define CURSOR_CACHE_HASH_MASK (CURSOR_CACHE_HASH_SIZE - 1)
>  #define CURSOR_CACHE_HASH_KEY(id) ((id) & CURSOR_CACHE_HASH_MASK)
>  
> +enum {
> +    PIPE_ITEM_TYPE_CURSOR = PIPE_ITEM_TYPE_COMMON_LAST,
> +    PIPE_ITEM_TYPE_CURSOR_INIT,
> +    PIPE_ITEM_TYPE_INVAL_CURSOR_CACHE,
> +};
> +
>  typedef struct CursorItem {
> +    QXLInstance *qxl;
>      uint32_t group_id;
>      int refs;
>      RedCursorCmd *red_cursor;
> @@ -77,20 +84,20 @@ typedef struct CursorChannel {
>  
>  G_STATIC_ASSERT(sizeof(CursorItem) <= QXL_CURSUR_DEVICE_DATA_SIZE);
>  
> -CursorChannel*       cursor_channel_new         (RedWorker *worker, int
> migrate);
> -void                 cursor_channel_disconnect  (RedChannel *channel);
> +CursorChannel*       cursor_channel_new         (RedWorker *worker);
> +void                 cursor_channel_disconnect  (CursorChannel *cursor);
>  void                 cursor_channel_reset       (CursorChannel *cursor);
>  void                 cursor_channel_process_cmd (CursorChannel *cursor,
>  RedCursorCmd *cursor_cmd,
>                                                   uint32_t group_id);
>  
> -CursorItem*          cursor_item_new            (RedCursorCmd *cmd, uint32_t
> group_id);
> -void                 cursor_item_unref          (QXLInstance *qxl,
> CursorItem *cursor);
> -
> +CursorChannelClient* cursor_channel_client_new  (CursorChannel *cursor,
> +                                                 RedClient *client,
> RedsStream *stream,
> +                                                 int mig_target,
> +                                                 uint32_t *common_caps, int
> num_common_caps,
> +                                                 uint32_t *caps, int
> num_caps);
>  
> -CursorChannelClient *cursor_channel_client_new(CommonChannel *common,
> -                                               RedClient *client, RedsStream
> *stream,
> -                                               int mig_target,
> -                                               uint32_t *common_caps, int
> num_common_caps,
> -                                               uint32_t *caps, int
> num_caps);
> +CursorItem*          cursor_item_new            (QXLInstance *qxl,
> RedCursorCmd *cmd, uint32_t group_id);
> +CursorItem*          cursor_item_ref            (CursorItem *cursor);
> +void                 cursor_item_unref          (CursorItem *cursor);
>  
>  #endif /* CURSOR_CHANNEL_H_ */
> diff --git a/server/red_dispatcher.c b/server/red_dispatcher.c
> index 43f061d..3fc2c1c 100644
> --- a/server/red_dispatcher.c
> +++ b/server/red_dispatcher.c
> @@ -160,6 +160,7 @@ static void red_dispatcher_set_cursor_peer(RedChannel
> *channel, RedClient *clien
>      memcpy(payload.common_caps, common_caps,
>      sizeof(uint32_t)*num_common_caps);
>      memcpy(payload.caps, caps, sizeof(uint32_t)*num_caps);
>  
> +    /* TODO serialize it all, no dangling pointers */
>      dispatcher_send_message(&dispatcher->dispatcher,
>                              RED_WORKER_MESSAGE_CURSOR_CONNECT,
>                              &payload);

This comment is not related to the commit message, I would remove it.

> diff --git a/server/red_worker.c b/server/red_worker.c
> index 8ae0878..817ff34 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -265,6 +265,23 @@ struct SpiceWatch {
>      void *watch_func_opaque;
>  };
>  
> +enum {
> +    PIPE_ITEM_TYPE_DRAW = PIPE_ITEM_TYPE_COMMON_LAST,
> +    PIPE_ITEM_TYPE_IMAGE,
> +    PIPE_ITEM_TYPE_STREAM_CREATE,
> +    PIPE_ITEM_TYPE_STREAM_CLIP,
> +    PIPE_ITEM_TYPE_STREAM_DESTROY,
> +    PIPE_ITEM_TYPE_UPGRADE,
> +    PIPE_ITEM_TYPE_MIGRATE_DATA,
> +    PIPE_ITEM_TYPE_PIXMAP_SYNC,
> +    PIPE_ITEM_TYPE_PIXMAP_RESET,
> +    PIPE_ITEM_TYPE_INVAL_PALETTE_CACHE,
> +    PIPE_ITEM_TYPE_CREATE_SURFACE,
> +    PIPE_ITEM_TYPE_DESTROY_SURFACE,
> +    PIPE_ITEM_TYPE_MONITORS_CONFIG,
> +    PIPE_ITEM_TYPE_STREAM_ACTIVATE_REPORT,
> +};
> +
>  #define MAX_LZ_ENCODERS MAX_CACHE_CLIENTS
>  
>  typedef struct SurfaceCreateItem {
> @@ -720,7 +737,6 @@ static void
> display_channel_client_release_item_before_push(DisplayChannelClient
>                                                              PipeItem *item);
>  static void
>  display_channel_client_release_item_after_push(DisplayChannelClient *dcc,
>                                                             PipeItem *item);
> -
>  static void red_push_monitors_config(DisplayChannelClient *dcc);
>  
>  /*
> @@ -764,7 +780,6 @@ static void red_push_monitors_config(DisplayChannelClient
> *dcc);
>                                           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)
>  
>  
>  
> @@ -4192,12 +4207,11 @@ static int red_process_cursor(RedWorker *worker,
> uint32_t max_pipe_size, int *ri
>                  free(cursor);
>                  break;
>              }
> -
>              cursor_channel_process_cmd(worker->cursor_channel, cursor,
>              ext_cmd.group_id);
>              break;
>          }
>          default:
> -            spice_error("bad command type");
> +            spice_warning("bad command type");
>          }

Where does the command came? If from VM I'm ok, if from internal I would prefer an error.

>          n++;
>      }
> @@ -4471,7 +4485,6 @@ static void marshaller_add_compressed(SpiceMarshaller
> *m,
>      } while (max);
>  }
>  
> -

I would remove these spurious space changes.

>  static void fill_base(SpiceMarshaller *base_marshaller, Drawable *drawable)
>  {
>      SpiceMsgDisplayBase base;
> @@ -8293,6 +8306,8 @@ static void
> red_display_marshall_stream_end(RedChannelClient *rcc,
>      spice_marshall_msg_display_stream_destroy(base_marshaller, &destroy);
>  }
>  
> +#define RCC_TO_CCC(rcc) SPICE_CONTAINEROF((rcc), CursorChannelClient,
> common.base)
> +

This macro is just moved, I'll remove this change.

>  static void red_marshall_surface_create(RedChannelClient *rcc,
>      SpiceMarshaller *base_marshaller, SpiceMsgSurfaceCreate *surface_create)
>  {
> @@ -8361,17 +8376,6 @@ static void
> red_marshall_stream_activate_report(RedChannelClient *rcc,
>      spice_marshall_msg_display_stream_activate_report(base_marshaller,
>      &msg);
>  }
>  
> -static inline void red_marshall_inval(RedChannelClient *rcc,
> -                                      SpiceMarshaller *base_marshaller,
> CacheItem *cach_item)
> -{
> -    SpiceMsgDisplayInvalOne inval_one;
> -
> -    red_channel_client_init_send_data(rcc, cach_item->inval_type, NULL);
> -    inval_one.id = *(uint64_t *)&cach_item->id;
> -
> -    spice_marshall_msg_cursor_inval_one(base_marshaller, &inval_one);
> -}
> -
>  static void display_channel_send_item(RedChannelClient *rcc, PipeItem
>  *pipe_item)
>  {
>      SpiceMarshaller *m = red_channel_client_get_marshaller(rcc);
> @@ -8384,9 +8388,6 @@ static void display_channel_send_item(RedChannelClient
> *rcc, PipeItem *pipe_item
>          marshall_qxl_drawable(rcc, m, dpi);
>          break;
>      }
> -    case PIPE_ITEM_TYPE_INVAL_ONE:
> -        red_marshall_inval(rcc, m, (CacheItem *)pipe_item);
> -        break;
>      case PIPE_ITEM_TYPE_STREAM_CREATE: {
>          StreamAgent *agent = SPICE_CONTAINEROF(pipe_item, StreamAgent,
>          create_item);
>          red_display_marshall_stream_start(rcc, m, agent);
> @@ -8406,7 +8407,7 @@ static void display_channel_send_item(RedChannelClient
> *rcc, PipeItem *pipe_item
>          red_display_marshall_upgrade(rcc, m, (UpgradeItem *)pipe_item);
>          break;
>      case PIPE_ITEM_TYPE_VERB:
> -        red_marshall_verb(rcc, ((VerbItem*)pipe_item)->verb);
> +        red_marshall_verb(rcc, (VerbItem*)pipe_item);
>          break;
>      case PIPE_ITEM_TYPE_MIGRATE_DATA:
>          display_channel_marshall_migrate_data(rcc, m);
> @@ -8422,7 +8423,7 @@ static void display_channel_send_item(RedChannelClient
> *rcc, PipeItem *pipe_item
>          break;
>      case PIPE_ITEM_TYPE_INVAL_PALETTE_CACHE:
>          red_reset_palette_cache(dcc);
> -        red_marshall_verb(rcc, SPICE_MSG_DISPLAY_INVAL_ALL_PALETTES);
> +        red_channel_client_init_send_data(rcc,
> SPICE_MSG_DISPLAY_INVAL_ALL_PALETTES, NULL);
>          break;
>      case PIPE_ITEM_TYPE_CREATE_SURFACE: {
>          SurfaceCreateItem *surface_create = SPICE_CONTAINEROF(pipe_item,
>          SurfaceCreateItem,
> @@ -8908,7 +8909,7 @@ static inline void flush_cursor_commands(RedWorker
> *worker)
>              red_channel_send(channel);
>              if (red_now() >= end_time) {
>                  spice_warning("flush cursor timeout");
> -                cursor_channel_disconnect(channel);
> +                cursor_channel_disconnect(worker->cursor_channel);
>                  worker->cursor_channel = NULL;
>              } else {
>                  sleep_count++;
> @@ -9520,16 +9521,16 @@ SpiceCoreInterface worker_core = {
>      .watch_remove = worker_watch_remove,
>  };
>  
> -CommonChannelClient *common_channel_client_create(int size,
> -                                                  CommonChannel *common,
> -                                                  RedClient *client,
> -                                                  RedsStream *stream,
> -                                                  int mig_target,
> -                                                  int monitor_latency,
> -                                                  uint32_t *common_caps,
> -                                                  int num_common_caps,
> -                                                  uint32_t *caps,
> -                                                  int num_caps)
> +CommonChannelClient *common_channel_new_client(CommonChannel *common,
> +                                               int size,

Why changing argument order?

> +                                               RedClient *client,
> +                                               RedsStream *stream,
> +                                               int mig_target,
> +                                               int monitor_latency,
> +                                               uint32_t *common_caps,
> +                                               int num_common_caps,
> +                                               uint32_t *caps,
> +                                               int num_caps)
>  {
>      RedChannelClient *rcc =
>          red_channel_client_create(size, &common->base, client, stream,
>          monitor_latency,
> @@ -9557,8 +9558,8 @@ DisplayChannelClient
> *display_channel_client_create(CommonChannel *common,
>                                                      uint32_t *caps, int
>                                                      num_caps)
>  {
>      DisplayChannelClient *dcc =
> -        (DisplayChannelClient*)common_channel_client_create(
> -            sizeof(DisplayChannelClient), common, client, stream,
> +        (DisplayChannelClient*)common_channel_new_client(
> +            common, sizeof(DisplayChannelClient), client, stream,
>              mig_target,
>              TRUE,
>              common_caps, num_common_caps,
> @@ -9572,38 +9573,30 @@ DisplayChannelClient
> *display_channel_client_create(CommonChannel *common,
>      return dcc;
>  }
>  
> -RedChannel *__new_channel(RedWorker *worker, int size, uint32_t
> channel_type,
> -                          int migration_flags,
> -                          channel_disconnect_proc on_disconnect,
> -                          channel_send_pipe_item_proc send_item,
> -                          channel_hold_pipe_item_proc hold_item,
> -                          channel_release_pipe_item_proc release_item,
> -                          channel_handle_parsed_proc handle_parsed,
> -                          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
> migrate_get_serial)
> +RedChannel *red_worker_new_channel(RedWorker *worker, int size,
> +                                   uint32_t channel_type, int
> migration_flags,
> +                                   ChannelCbs *channel_cbs,
> +                                   channel_handle_parsed_proc handle_parsed)
>  {
>      RedChannel *channel = NULL;
>      CommonChannel *common;
> -    ChannelCbs channel_cbs = { NULL, };
> -
> -    channel_cbs.config_socket = common_channel_config_socket;
> -    channel_cbs.on_disconnect = on_disconnect;
> -    channel_cbs.send_item = send_item;
> -    channel_cbs.hold_item = hold_item;
> -    channel_cbs.release_item = release_item;
> -    channel_cbs.alloc_recv_buf = common_alloc_recv_buf;
> -    channel_cbs.release_recv_buf = common_release_recv_buf;
> -    channel_cbs.handle_migrate_flush_mark = handle_migrate_flush_mark;
> -    channel_cbs.handle_migrate_data = handle_migrate_data;
> -    channel_cbs.handle_migrate_data_get_serial = migrate_get_serial;
> +
> +    spice_return_val_if_fail(worker, NULL);
> +    spice_return_val_if_fail(channel_cbs, NULL);
> +    spice_return_val_if_fail(!channel_cbs->config_socket, NULL);
> +    spice_return_val_if_fail(!channel_cbs->alloc_recv_buf, NULL);
> +    spice_return_val_if_fail(!channel_cbs->release_recv_buf, NULL);
> +
> +    channel_cbs->config_socket = common_channel_config_socket;
> +    channel_cbs->alloc_recv_buf = common_alloc_recv_buf;
> +    channel_cbs->release_recv_buf = common_release_recv_buf;
>  
>      channel = red_channel_create_parser(size, &worker_core,
>                                          channel_type, worker->qxl->id,
>                                          TRUE /* handle_acks */,
>                                          spice_get_client_channel_parser(channel_type,
>                                          NULL),
>                                          handle_parsed,
> -                                        &channel_cbs,
> +                                        channel_cbs,
>                                          migration_flags);
>      common = (CommonChannel *)channel;
>      if (!channel) {
> @@ -9723,7 +9716,6 @@ static void
> display_channel_client_release_item_before_push(DisplayChannelClient
>          free(item);
>          break;
>      }
> -    case PIPE_ITEM_TYPE_INVAL_ONE:
>      case PIPE_ITEM_TYPE_VERB:
>      case PIPE_ITEM_TYPE_MIGRATE_DATA:
>      case PIPE_ITEM_TYPE_PIXMAP_SYNC:
> @@ -9753,25 +9745,26 @@ static void
> display_channel_release_item(RedChannelClient *rcc, PipeItem *item,
>  static void display_channel_create(RedWorker *worker, int migrate)
>  {
>      DisplayChannel *display_channel;
> +    ChannelCbs cbs = {
> +        .on_disconnect = display_channel_client_on_disconnect,
> +        .send_item = display_channel_send_item,
> +        .hold_item = display_channel_hold_pipe_item,
> +        .release_item = display_channel_release_item,
> +        .handle_migrate_flush_mark = display_channel_handle_migrate_mark,
> +        .handle_migrate_data = display_channel_handle_migrate_data,
> +        .handle_migrate_data_get_serial =
> display_channel_handle_migrate_data_get_serial
> +    };
>  
>      if (worker->display_channel) {
>          return;
>      }
>  
>      spice_info("create display channel");
> -    if (!(worker->display_channel = (DisplayChannel *)__new_channel(
> +    if (!(worker->display_channel = (DisplayChannel
> *)red_worker_new_channel(
>              worker, sizeof(*display_channel),
>              SPICE_CHANNEL_DISPLAY,
>              SPICE_MIGRATE_NEED_FLUSH | SPICE_MIGRATE_NEED_DATA_TRANSFER,
> -            display_channel_client_on_disconnect,
> -            display_channel_send_item,
> -            display_channel_hold_pipe_item,
> -            display_channel_release_item,
> -            display_channel_handle_message,
> -            display_channel_handle_migrate_mark,
> -            display_channel_handle_migrate_data,
> -            display_channel_handle_migrate_data_get_serial
> -            ))) {
> +            &cbs, display_channel_handle_message))) {
>          spice_warning("failed to create display channel");
>          return;
>      }
> @@ -9906,29 +9899,6 @@ static void handle_new_display_channel(RedWorker
> *worker, RedClient *client, Red
>      on_new_display_channel_client(dcc);
>  }
>  
> -static void red_migrate_cursor(RedWorker *worker, RedChannelClient *rcc)
> -{
> -    if (red_channel_client_is_connected(rcc)) {
> -        red_channel_client_pipe_add_type(rcc,
> -                                         PIPE_ITEM_TYPE_INVAL_CURSOR_CACHE);
> -        red_channel_client_default_migrate(rcc);
> -    }
> -}
> -
> -static void on_new_cursor_channel(RedWorker *worker, RedChannelClient *rcc)
> -{
> -    CursorChannel *channel = worker->cursor_channel;
> -
> -    spice_assert(channel);
> -    red_channel_client_ack_zero_messages_window(rcc);
> -    red_channel_client_push_set_ack(rcc);
> -    // TODO: why do we check for context.canvas? defer this to after display
> cc is connected
> -    // and test it's canvas? this is just a test to see if there is an
> active renderer?
> -    if (worker->surfaces[0].context.canvas &&
> !channel->common.during_target_migrate) {
> -        red_channel_client_pipe_add_type(rcc, PIPE_ITEM_TYPE_CURSOR_INIT);
> -    }
> -}
> -

These function was inlined directly. Just style, I'm fine with this
change. Perhaps not really related.

>  static void red_connect_cursor(RedWorker *worker, RedClient *client,
>  RedsStream *stream,
>                                 int migrate,
>                                 uint32_t *common_caps, int num_common_caps,
> @@ -9937,24 +9907,28 @@ static void red_connect_cursor(RedWorker *worker,
> RedClient *client, RedsStream
>      CursorChannel *channel;
>      CursorChannelClient *ccc;
>  
> -    if (worker->cursor_channel == NULL) {
> -        spice_warning("cursor channel was not created");
> -        return;
> -    }
> +    spice_return_if_fail(worker->cursor_channel != NULL);
> +
>      channel = worker->cursor_channel;
>      spice_info("add cursor channel client");
> -    ccc = cursor_channel_client_new(&channel->common, client, stream,
> +    ccc = cursor_channel_client_new(channel, client, stream,
>                                      migrate,
>                                      common_caps, num_common_caps,
>                                      caps, num_caps);
> -    if (!ccc) {
> -        return;
> -    }
> +    spice_return_if_fail(ccc != NULL);
>  #ifdef RED_STATISTICS
>      channel->stat = stat_add_node(worker->stat, "cursor_channel", TRUE);
>      channel->common.base.out_bytes_counter = stat_add_counter(channel->stat,
>      "out_bytes", TRUE);
>  #endif
> -    on_new_cursor_channel(worker, &ccc->common.base);
> +
> +    RedChannelClient *rcc = &ccc->common.base;
> +    red_channel_client_ack_zero_messages_window(rcc);
> +    red_channel_client_push_set_ack(rcc);
> +    // TODO: why do we check for context.canvas? defer this to after display
> cc is connected
> +    // and test it's canvas? this is just a test to see if there is an
> active renderer?
> +    if (worker->surfaces[0].context.canvas &&
> !channel->common.during_target_migrate) {
> +        red_channel_client_pipe_add_type(rcc, PIPE_ITEM_TYPE_CURSOR_INIT);
> +    }
>  }
>  
>  static void surface_dirty_region_to_rects(RedSurface *surface,
> @@ -10296,10 +10270,9 @@ static void dev_create_primary_surface(RedWorker
> *worker, uint32_t surface_id,
>          red_channel_push(&worker->display_channel->common.base);
>      }
>  
> -    if (cursor_is_connected(worker) &&
> !worker->cursor_channel->common.during_target_migrate) {
> +    if (!worker->cursor_channel->common.during_target_migrate)

The connection check was removed without explanation

>          red_channel_pipes_add_type(&worker->cursor_channel->common.base,
>                                     PIPE_ITEM_TYPE_CURSOR_INIT);
> -    }
>  }
>  
>  void handle_dev_create_primary_surface(void *opaque, void *payload)
> @@ -10498,7 +10471,9 @@ void handle_dev_oom(void *opaque, void *payload)
>  
>  void handle_dev_reset_cursor(void *opaque, void *payload)
>  {
> -    cursor_channel_reset(((RedWorker*)opaque)->cursor_channel);
> +    RedWorker *worker = opaque;
> +
> +    cursor_channel_reset(worker->cursor_channel);
>  }
>  
>  void handle_dev_reset_image_cache(void *opaque, void *payload)
> @@ -10642,10 +10617,12 @@ void handle_dev_cursor_channel_create(void *opaque,
> void *payload)
>      RedWorker *worker = opaque;
>      RedChannel *red_channel;
>  
> -    // TODO: handle seemless migration. Temp, setting migrate to FALSE
>      if (!worker->cursor_channel) {
> -        worker->cursor_channel = cursor_channel_new(worker, FALSE);
> +        worker->cursor_channel = cursor_channel_new(worker);
> +    } else {
> +        spice_warning("cursor channel already created");
>      }
> +
>      red_channel = &worker->cursor_channel->common.base;
>      send_data(worker->channel, &red_channel, sizeof(RedChannel *));
>  }
> @@ -10672,19 +10649,20 @@ void handle_dev_cursor_disconnect(void *opaque,
> void *payload)
>      RedChannelClient *rcc = msg->rcc;
>  
>      spice_info("disconnect cursor client");
> -    spice_assert(rcc);
>      red_channel_client_disconnect(rcc);
>  }
>  
>  void handle_dev_cursor_migrate(void *opaque, void *payload)
>  {
>      RedWorkerMessageCursorMigrate *msg = payload;
> -    RedWorker *worker = opaque;
>      RedChannelClient *rcc = msg->rcc;
>  
>      spice_info("migrate cursor client");
> -    spice_assert(rcc);
> -    red_migrate_cursor(worker, rcc);
> +    if (!red_channel_client_is_connected(rcc))
> +        return;
> +
> +    red_channel_client_pipe_add_type(rcc,
> PIPE_ITEM_TYPE_INVAL_CURSOR_CACHE);
> +    red_channel_client_default_migrate(rcc);
>  }
>  
>  void handle_dev_set_compression(void *opaque, void *payload)
> @@ -10761,8 +10739,10 @@ void handle_dev_set_mouse_mode(void *opaque, void
> *payload)
>      RedWorkerMessageSetMouseMode *msg = payload;
>      RedWorker *worker = opaque;
>  
> +    spice_info("mouse mode %u", msg->mode);
> +    spice_return_if_fail(worker->cursor_channel);
> +
>      worker->cursor_channel->mouse_mode = msg->mode;
> -    spice_info("mouse mode %u", worker->cursor_channel->mouse_mode);
>  }
>  
>  void handle_dev_add_memslot_async(void *opaque, void *payload)
> diff --git a/server/red_worker.h b/server/red_worker.h
> index 502283e..60c326a 100644
> --- a/server/red_worker.h
> +++ b/server/red_worker.h
> @@ -32,6 +32,7 @@ typedef struct CommonChannelClient {
>      int is_low_bandwidth;
>  } CommonChannelClient;
>  
> +
>  #define DISPLAY_CLIENT_TIMEOUT 30000000000ULL //nano
>  
>  #define CHANNEL_RECEIVE_BUF_SIZE 1024
> @@ -48,25 +49,10 @@ typedef struct CommonChannel {
>  } CommonChannel;
>  
>  enum {
> -    PIPE_ITEM_TYPE_DRAW = PIPE_ITEM_TYPE_CHANNEL_BASE,
> +    PIPE_ITEM_TYPE_VERB = PIPE_ITEM_TYPE_CHANNEL_BASE,
>      PIPE_ITEM_TYPE_INVAL_ONE,
> -    PIPE_ITEM_TYPE_CURSOR,
> -    PIPE_ITEM_TYPE_CURSOR_INIT,
> -    PIPE_ITEM_TYPE_IMAGE,
> -    PIPE_ITEM_TYPE_STREAM_CREATE,
> -    PIPE_ITEM_TYPE_STREAM_CLIP,
> -    PIPE_ITEM_TYPE_STREAM_DESTROY,
> -    PIPE_ITEM_TYPE_UPGRADE,
> -    PIPE_ITEM_TYPE_VERB,
> -    PIPE_ITEM_TYPE_MIGRATE_DATA,
> -    PIPE_ITEM_TYPE_PIXMAP_SYNC,
> -    PIPE_ITEM_TYPE_PIXMAP_RESET,
> -    PIPE_ITEM_TYPE_INVAL_CURSOR_CACHE,
> -    PIPE_ITEM_TYPE_INVAL_PALETTE_CACHE,
> -    PIPE_ITEM_TYPE_CREATE_SURFACE,
> -    PIPE_ITEM_TYPE_DESTROY_SURFACE,
> -    PIPE_ITEM_TYPE_MONITORS_CONFIG,
> -    PIPE_ITEM_TYPE_STREAM_ACTIVATE_REPORT,
> +
> +    PIPE_ITEM_TYPE_COMMON_LAST
>  };
>  
>  typedef struct VerbItem {
> @@ -74,10 +60,9 @@ typedef struct VerbItem {
>      uint16_t verb;
>  } VerbItem;
>  
> -static inline void red_marshall_verb(RedChannelClient *rcc, uint16_t verb)
> +static inline void red_marshall_verb(RedChannelClient *rcc, VerbItem *item)
>  {
> -    spice_assert(rcc);
> -    red_channel_client_init_send_data(rcc, verb, NULL);
> +    red_channel_client_init_send_data(rcc, item->verb, NULL);
>  }
>  
>  static inline void red_pipe_add_verb(RedChannelClient* rcc, uint16_t verb)
> @@ -103,7 +88,6 @@ static inline void red_pipe_add_verb(RedChannelClient*
> rcc, uint16_t verb)
>  #define RCC_FOREACH_SAFE(link, next, rcc, channel) \
>      SAFE_FOREACH(link, next, channel,  &(channel)->clients, rcc,
>      LINK_TO_RCC(link))
>  
> -
>  static inline void red_pipes_add_verb(RedChannel *channel, uint16_t verb)
>  {
>      RedChannelClient *rcc;
> @@ -114,30 +98,25 @@ static inline void red_pipes_add_verb(RedChannel
> *channel, uint16_t verb)
>      }
>  }
>  
> +
>  RedWorker* red_worker_new(QXLInstance *qxl, RedDispatcher *red_dispatcher);
>  bool       red_worker_run(RedWorker *worker);
>  QXLInstance* red_worker_get_qxl(RedWorker *worker);
>  
> -CommonChannelClient *common_channel_client_create(int size,
> -                                                  CommonChannel *common,
> -                                                  RedClient *client,
> -                                                  RedsStream *stream,
> -                                                  int mig_target,
> -                                                  int monitor_latency,
> -                                                  uint32_t *common_caps,
> -                                                  int num_common_caps,
> -                                                  uint32_t *caps,
> -                                                  int num_caps);
> -
> -RedChannel *__new_channel(RedWorker *worker, int size, uint32_t
> channel_type,
> -                          int migration_flags,
> -                          channel_disconnect_proc on_disconnect,
> -                          channel_send_pipe_item_proc send_item,
> -                          channel_hold_pipe_item_proc hold_item,
> -                          channel_release_pipe_item_proc release_item,
> -                          channel_handle_parsed_proc handle_parsed,
> -                          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
> migrate_get_serial);
> +RedChannel *red_worker_new_channel(RedWorker *worker, int size,
> +                                   uint32_t channel_type, int
> migration_flags,
> +                                   ChannelCbs *channel_cbs,
> +                                   channel_handle_parsed_proc
> handle_parsed);
> +
> +CommonChannelClient *common_channel_new_client(CommonChannel *common,
> +                                               int size,
> +                                               RedClient *client,
> +                                               RedsStream *stream,
> +                                               int mig_target,
> +                                               int monitor_latency,
> +                                               uint32_t *common_caps,
> +                                               int num_common_caps,
> +                                               uint32_t *caps,
> +                                               int num_caps);
>  
>  #endif

This patch require more attention and more review.

Frediano


More information about the Spice-devel mailing list