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

Frediano Ziglio fziglio at redhat.com
Tue Oct 27 12:19:10 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
 #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);
 
-    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);
+
 }
 
 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;
 }
 
+#ifdef DEBUG_CURSORS
+static int _cursor_count = 0;
+#endif
+
 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)
 {
+    RedChannel *channel = (RedChannel *)cursor;
+
     if (!channel || !red_channel_is_connected(channel)) {
         return;
     }
+
+    /* TODO: use a class vdispose */
     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);
 
     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);
 
-    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);
-}
-
 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);
+
     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);
+
     cursor_pipe_item = SPICE_CONTAINEROF(item, CursorPipeItem, base);
+    /* TODO: refcnt at PipeItem instead ? */
     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)
 {
-    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 (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) {
+        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;
+
+    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);
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");
         }
         n++;
     }
@@ -4471,7 +4485,6 @@ static void marshaller_add_compressed(SpiceMarshaller *m,
     } while (max);
 }
 
-
 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)
+
 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,
+                                               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);
-    }
-}
-
 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)
         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
-- 
2.4.3



More information about the Spice-devel mailing list