[Spice-devel] [PATCH 06/11] server: make more of cursor private

Christophe Fergeau cfergeau at redhat.com
Fri Oct 30 09:46:46 PDT 2015


This one looks good to me.
Some 
#define COMMON_CHANNEL_CLIENT(Client) ((CommonChannelClient*)(Client))
macro which we might want to change for now to
#define COMMON_CHANNEL_CLIENT(Client)  ((Client)->base))

and ditto for
#define CURSOR_CHANNEL_CLIENT(Client) ((CursorChannelClient*)(Client))


Christophe

On Thu, Oct 29, 2015 at 11:09:33AM +0000, Frediano Ziglio wrote:
> From: Marc-André Lureau <marcandre.lureau at gmail.com>
> 
> ---
>  server/cursor-channel.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++--
>  server/cursor-channel.h | 51 +++-----------------------------
>  server/red_channel.h    |  2 ++
>  server/red_worker.c     | 20 +++++--------
>  server/red_worker.h     |  1 +
>  5 files changed, 89 insertions(+), 63 deletions(-)
> 
> diff --git a/server/cursor-channel.c b/server/cursor-channel.c
> index bb4c57a..d03bc1b 100644
> --- a/server/cursor-channel.c
> +++ b/server/cursor-channel.c
> @@ -19,6 +19,41 @@
>  #include "common/generated_server_marshallers.h"
>  #include "cursor-channel.h"
>  
> +#define CLIENT_CURSOR_CACHE_SIZE 256
> +
> +#define CURSOR_CACHE_HASH_SHIFT 8
> +#define CURSOR_CACHE_HASH_SIZE (1 << CURSOR_CACHE_HASH_SHIFT)
> +#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;
> +} CursorItem;
> +
> +G_STATIC_ASSERT(sizeof(CursorItem) <= QXL_CURSUR_DEVICE_DATA_SIZE);
> +
> +typedef struct LocalCursor {
> +    CursorItem base;
> +    SpicePoint16 position;
> +    uint32_t data_size;
> +    SpiceCursor red_cursor;
> +} LocalCursor;
> +
> +typedef struct CursorPipeItem {
> +    PipeItem base;
> +    CursorItem *cursor_item;
> +    int refs;
> +} CursorPipeItem;
> +
>  struct CursorChannel {
>      CommonChannel common; // Must be the first thing
>  
> @@ -34,13 +69,23 @@ struct CursorChannel {
>  #endif
>  };
>  
> +struct CursorChannelClient {
> +    CommonChannelClient common;
> +
> +    CacheItem *cursor_cache[CURSOR_CACHE_HASH_SIZE];
> +    Ring cursor_cache_lru;
> +    long cursor_cache_available;
> +    uint32_t cursor_cache_items;
> +};
> +
> +
>  #define RCC_TO_CCC(rcc) SPICE_CONTAINEROF((rcc), CursorChannelClient, common.base)
>  
>  #define CLIENT_CURSOR_CACHE
>  #include "cache_item.tmpl.c"
>  #undef CLIENT_CURSOR_CACHE
>  
> -CursorItem *cursor_item_new(QXLInstance *qxl, RedCursorCmd *cmd, uint32_t group_id)
> +static CursorItem *cursor_item_new(QXLInstance *qxl, RedCursorCmd *cmd, uint32_t group_id)
>  {
>      CursorItem *cursor_item;
>  
> @@ -55,7 +100,7 @@ CursorItem *cursor_item_new(QXLInstance *qxl, RedCursorCmd *cmd, uint32_t group_
>      return cursor_item;
>  }
>  
> -CursorItem *cursor_item_ref(CursorItem *item)
> +static CursorItem *cursor_item_ref(CursorItem *item)
>  {
>      spice_return_val_if_fail(item != NULL, NULL);
>      spice_return_val_if_fail(item->refs > 0, NULL);
> @@ -65,7 +110,7 @@ CursorItem *cursor_item_ref(CursorItem *item)
>      return item;
>  }
>  
> -void cursor_item_unref(CursorItem *item)
> +static void cursor_item_unref(CursorItem *item)
>  {
>      QXLReleaseInfoExt release_info_ext;
>      RedCursorCmd *cursor_cmd;
> @@ -400,6 +445,17 @@ CursorChannel* cursor_channel_new(RedWorker *worker)
>      return cursor;
>  }
>  
> +void cursor_channel_client_migrate(CursorChannelClient* client)
> +{
> +    RedChannelClient *rcc;
> +
> +    spice_return_if_fail(client);
> +    rcc = RED_CHANNEL_CLIENT(client);
> +
> +    red_channel_client_pipe_add_type(rcc, PIPE_ITEM_TYPE_INVAL_CURSOR_CACHE);
> +    red_channel_client_default_migrate(rcc);
> +}
> +
>  CursorChannelClient* cursor_channel_client_new(CursorChannel *cursor, RedClient *client, RedsStream *stream,
>                                                 int mig_target,
>                                                 uint32_t *common_caps, int num_common_caps,
> @@ -497,6 +553,22 @@ void cursor_channel_reset(CursorChannel *cursor)
>      }
>  }
>  
> +void cursor_channel_init(CursorChannel *cursor, CursorChannelClient *client)
> +{
> +    spice_return_if_fail(cursor);
> +
> +    if (COMMON_CHANNEL(cursor)->during_target_migrate) {
> +        spice_debug("during_target_migrate: skip init");
> +        return;
> +    }
> +
> +    if (client)
> +        red_channel_client_pipe_add_type(RED_CHANNEL_CLIENT(client),
> +                                         PIPE_ITEM_TYPE_CURSOR_INIT);
> +    else
> +        red_channel_pipes_add_type(RED_CHANNEL(cursor), PIPE_ITEM_TYPE_CURSOR_INIT);
> +}
> +
>  void cursor_channel_set_mouse_mode(CursorChannel *cursor, uint32_t mode)
>  {
>      spice_return_if_fail(cursor);
> diff --git a/server/cursor-channel.h b/server/cursor-channel.h
> index 980c9ef..f295f42 100644
> --- a/server/cursor-channel.h
> +++ b/server/cursor-channel.h
> @@ -25,55 +25,15 @@
>  #include "cache-item.h"
>  #include "stat.h"
>  
> -#define CLIENT_CURSOR_CACHE_SIZE 256
> -
> -#define CURSOR_CACHE_HASH_SHIFT 8
> -#define CURSOR_CACHE_HASH_SIZE (1 << CURSOR_CACHE_HASH_SHIFT)
> -#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 CursorChannel CursorChannel;
> +typedef struct CursorChannelClient CursorChannelClient;
>  
> -typedef struct CursorItem {
> -    QXLInstance *qxl;
> -    uint32_t group_id;
> -    int refs;
> -    RedCursorCmd *red_cursor;
> -} CursorItem;
> -
> -typedef struct CursorPipeItem {
> -    PipeItem base;
> -    CursorItem *cursor_item;
> -    int refs;
> -} CursorPipeItem;
> -
> -typedef struct LocalCursor {
> -    CursorItem base;
> -    SpicePoint16 position;
> -    uint32_t data_size;
> -    SpiceCursor red_cursor;
> -} LocalCursor;
> -
> -typedef struct CursorChannelClient {
> -    CommonChannelClient common;
> -
> -    CacheItem *cursor_cache[CURSOR_CACHE_HASH_SIZE];
> -    Ring cursor_cache_lru;
> -    long cursor_cache_available;
> -    uint32_t cursor_cache_items;
> -} CursorChannelClient;
> -
> -G_STATIC_ASSERT(sizeof(CursorItem) <= QXL_CURSUR_DEVICE_DATA_SIZE);
> +#define CURSOR_CHANNEL_CLIENT(Client) ((CursorChannelClient*)(Client))
>  
>  CursorChannel*       cursor_channel_new         (RedWorker *worker);
>  void                 cursor_channel_disconnect  (CursorChannel *cursor);
>  void                 cursor_channel_reset       (CursorChannel *cursor);
> +void                 cursor_channel_init        (CursorChannel *cursor, CursorChannelClient* client);
>  void                 cursor_channel_process_cmd (CursorChannel *cursor, RedCursorCmd *cursor_cmd,
>                                                   uint32_t group_id);
>  void                 cursor_channel_set_mouse_mode(CursorChannel *cursor, uint32_t mode);
> @@ -83,9 +43,6 @@ CursorChannelClient* cursor_channel_client_new  (CursorChannel *cursor,
>                                                   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);
> +void                 cursor_channel_client_migrate(CursorChannelClient* client);
>  
>  #endif /* CURSOR_CHANNEL_H_ */
> diff --git a/server/red_channel.h b/server/red_channel.h
> index 201a4d2..eda4436 100644
> --- a/server/red_channel.h
> +++ b/server/red_channel.h
> @@ -302,6 +302,8 @@ struct RedChannelClient {
>      RedChannelClientConnectivityMonitor connectivity_monitor;
>  };
>  
> +#define RED_CHANNEL_CLIENT(Client) ((RedChannelClient *)(Client))
> +
>  struct RedChannel {
>      uint32_t type;
>      uint32_t id;
> diff --git a/server/red_worker.c b/server/red_worker.c
> index 153f661..4d797c8 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -9414,7 +9414,7 @@ static int common_channel_config_socket(RedChannelClient *rcc)
>      RedClient *client = red_channel_client_get_client(rcc);
>      MainChannelClient *mcc = red_client_get_main(client);
>      RedsStream *stream = red_channel_client_get_stream(rcc);
> -    CommonChannelClient *ccc = SPICE_CONTAINEROF(rcc, CommonChannelClient, base);
> +    CommonChannelClient *ccc = COMMON_CHANNEL_CLIENT(rcc);
>      int flags;
>      int delay_val;
>  
> @@ -9918,14 +9918,14 @@ static void red_connect_cursor(RedWorker *worker, RedClient *client, RedsStream
>                                      caps, num_caps);
>      spice_return_if_fail(ccc != NULL);
>  
> -    RedChannelClient *rcc = &ccc->common.base;
> +    RedChannelClient *rcc = RED_CHANNEL_CLIENT(ccc);
>      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 && !COMMON_CHANNEL(channel)->during_target_migrate) {
> -        red_channel_client_pipe_add_type(rcc, PIPE_ITEM_TYPE_CURSOR_INIT);
> -    }
> +    if (worker->surfaces[0].context.canvas)
> +        cursor_channel_init(channel, ccc);
>  }
>  
>  static void surface_dirty_region_to_rects(RedSurface *surface,
> @@ -10267,9 +10267,7 @@ static void dev_create_primary_surface(RedWorker *worker, uint32_t surface_id,
>          red_channel_push(&worker->display_channel->common.base);
>      }
>  
> -    if (!COMMON_CHANNEL(worker->cursor_channel)->during_target_migrate)
> -        red_channel_pipes_add_type(RED_CHANNEL(worker->cursor_channel),
> -                                   PIPE_ITEM_TYPE_CURSOR_INIT);
> +    cursor_channel_init(worker->cursor_channel, NULL);
>  }
>  
>  void handle_dev_create_primary_surface(void *opaque, void *payload)
> @@ -10655,11 +10653,7 @@ void handle_dev_cursor_migrate(void *opaque, void *payload)
>      RedChannelClient *rcc = msg->rcc;
>  
>      spice_info("migrate cursor client");
> -    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);
> +    cursor_channel_client_migrate(CURSOR_CHANNEL_CLIENT(rcc));
>  }
>  
>  void handle_dev_set_compression(void *opaque, void *payload)
> diff --git a/server/red_worker.h b/server/red_worker.h
> index 0a3d7c6..33dd974 100644
> --- a/server/red_worker.h
> +++ b/server/red_worker.h
> @@ -33,6 +33,7 @@ typedef struct CommonChannelClient {
>      int is_low_bandwidth;
>  } CommonChannelClient;
>  
> +#define COMMON_CHANNEL_CLIENT(Client) ((CommonChannelClient*)(Client))
>  #define DISPLAY_CLIENT_TIMEOUT 30000000000ULL //nano
>  
>  #define CHANNEL_RECEIVE_BUF_SIZE 1024
> -- 
> 2.4.3
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20151030/132b21b2/attachment.sig>


More information about the Spice-devel mailing list