[Spice-devel] [PATCH spice-server v3 1/3] Add CommonGraphicsChannelPrivate struct

Frediano Ziglio fziglio at redhat.com
Fri Oct 14 14:40:12 UTC 2016


> 
> From: Jonathon Jongsma <jjongsma at redhat.com>
> 
> Encapsulate private data for CommonGraphicsChannel and prepare for
> GObject conversion.
> ---
>  server/common-graphics-channel.c | 33 +++++++++++++++++++++++++++++++--
>  server/common-graphics-channel.h | 14 ++++++--------
>  server/cursor-channel-client.c   |  2 +-
>  server/cursor-channel.c          |  8 +++++---
>  server/dcc-send.c                |  2 +-
>  server/dcc.c                     |  9 +++++----
>  server/display-channel.c         |  6 +++---
>  server/red-worker.c              | 10 +++++++---
>  8 files changed, 59 insertions(+), 25 deletions(-)
> 
> diff --git a/server/common-graphics-channel.c
> b/server/common-graphics-channel.c
> index bcf7279..b02f3d7 100644
> --- a/server/common-graphics-channel.c
> +++ b/server/common-graphics-channel.c
> @@ -27,6 +27,19 @@
>  #include "dcc.h"
>  #include "main-channel-client.h"
>  
> +#define CHANNEL_RECEIVE_BUF_SIZE 1024
> +
> +struct CommonGraphicsChannelPrivate
> +{
> +    QXLInstance *qxl;
> +    uint8_t recv_buf[CHANNEL_RECEIVE_BUF_SIZE];
> +    int during_target_migrate; /* TRUE when the client that is associated
> with the channel
> +                                  is during migration. Turned off when the
> vm is started.
> +                                  The flag is used to avoid sending messages
> that are artifacts
> +                                  of the transition from stopped vm to
> loaded vm (e.g., recreation
> +                                  of the primary surface) */
> +};
> +
>  static uint8_t *common_alloc_recv_buf(RedChannelClient *rcc, uint16_t type,
>  uint32_t size)
>  {
>      RedChannel *channel = red_channel_client_get_channel(rcc);
> @@ -41,7 +54,7 @@ static uint8_t *common_alloc_recv_buf(RedChannelClient
> *rcc, uint16_t type, uint
>          spice_critical("unexpected message size %u (max is %d)", size,
>          CHANNEL_RECEIVE_BUF_SIZE);
>          return NULL;
>      }
> -    return common->recv_buf;
> +    return common->priv->recv_buf;
>  }
>  
>  static void common_release_recv_buf(RedChannelClient *rcc, uint16_t type,
>  uint32_t size,
> @@ -123,7 +136,23 @@ CommonGraphicsChannel*
> common_graphics_channel_new(RedsState *server,
>      spice_return_val_if_fail(channel, NULL);
>  
>      common = COMMON_GRAPHICS_CHANNEL(channel);
> -    common->qxl = qxl;
> +    common->priv = g_new0(CommonGraphicsChannelPrivate, 1);
> +    common->priv->qxl = qxl;
>      return common;
>  }
>  
> +void common_graphics_channel_set_during_target_migrate(CommonGraphicsChannel
> *self, gboolean value)
> +{
> +    self->priv->during_target_migrate = value;
> +}
> +
> +gboolean
> common_graphics_channel_get_during_target_migrate(CommonGraphicsChannel
> *self)
> +{
> +    return self->priv->during_target_migrate;
> +}
> +
> +QXLInstance* common_graphics_channel_get_qxl(CommonGraphicsChannel *self)
> +{
> +    return self->priv->qxl;
> +}
> +
> diff --git a/server/common-graphics-channel.h
> b/server/common-graphics-channel.h
> index b9473d8..97cd63b 100644
> --- a/server/common-graphics-channel.h
> +++ b/server/common-graphics-channel.h
> @@ -25,21 +25,19 @@ int common_channel_config_socket(RedChannelClient *rcc);
>  
>  #define COMMON_CLIENT_TIMEOUT (NSEC_PER_SEC * 30)
>  
> -#define CHANNEL_RECEIVE_BUF_SIZE 1024
> +typedef struct CommonGraphicsChannelPrivate CommonGraphicsChannelPrivate;
>  typedef struct CommonGraphicsChannel {
>      RedChannel base; // Must be the first thing
>  
> -    QXLInstance *qxl;
> -    uint8_t recv_buf[CHANNEL_RECEIVE_BUF_SIZE];
> -    int during_target_migrate; /* TRUE when the client that is associated
> with the channel
> -                                  is during migration. Turned off when the
> vm is started.
> -                                  The flag is used to avoid sending messages
> that are artifacts
> -                                  of the transition from stopped vm to
> loaded vm (e.g., recreation
> -                                  of the primary surface) */
> +    CommonGraphicsChannelPrivate *priv;
>  } CommonGraphicsChannel;
>  
>  #define COMMON_GRAPHICS_CHANNEL(Channel) ((CommonGraphicsChannel*)(Channel))
>  
> +void common_graphics_channel_set_during_target_migrate(CommonGraphicsChannel
> *self, gboolean value);
> +gboolean
> common_graphics_channel_get_during_target_migrate(CommonGraphicsChannel
> *self);
> +QXLInstance* common_graphics_channel_get_qxl(CommonGraphicsChannel *self);
> +
>  enum {
>      RED_PIPE_ITEM_TYPE_VERB = RED_PIPE_ITEM_TYPE_CHANNEL_BASE,
>      RED_PIPE_ITEM_TYPE_INVAL_ONE,
> diff --git a/server/cursor-channel-client.c b/server/cursor-channel-client.c
> index 90778ed..b7ab2e5 100644
> --- a/server/cursor-channel-client.c
> +++ b/server/cursor-channel-client.c
> @@ -124,7 +124,7 @@ CursorChannelClient*
> cursor_channel_client_new(CursorChannel *cursor, RedClient
>                           "common-caps", common_caps_array,
>                           "caps", caps_array,
>                           NULL);
> -    COMMON_GRAPHICS_CHANNEL(cursor)->during_target_migrate = mig_target;
> +
> common_graphics_channel_set_during_target_migrate(COMMON_GRAPHICS_CHANNEL(cursor),
> mig_target);
>  
>      if (caps_array)
>          g_array_unref(caps_array);
> diff --git a/server/cursor-channel.c b/server/cursor-channel.c
> index a4d5b58..8fdd074 100644
> --- a/server/cursor-channel.c
> +++ b/server/cursor-channel.c
> @@ -337,11 +337,13 @@ void cursor_channel_process_cmd(CursorChannel *cursor,
> RedCursorCmd *cursor_cmd)
>  {
>      CursorItem *cursor_item;
>      int cursor_show = FALSE;
> +    QXLInstance *qxl = NULL;
>  

I would remove the initialization, it's initialized just below
so compiler can issue warning if the line below is removed
and variable is used uninitialized.

>      spice_return_if_fail(cursor);
>      spice_return_if_fail(cursor_cmd);
>  
> -    cursor_item = cursor_item_new(cursor->common.qxl, cursor_cmd);
> +    qxl = common_graphics_channel_get_qxl(COMMON_GRAPHICS_CHANNEL(cursor));
> +    cursor_item = cursor_item_new(qxl, cursor_cmd);
>  
>      switch (cursor_cmd->type) {
>      case QXL_CURSOR_SET:
> @@ -389,7 +391,7 @@ void cursor_channel_reset(CursorChannel *cursor)
>  
>      if (red_channel_is_connected(channel)) {
>          red_channel_pipes_add_type(channel,
>          RED_PIPE_ITEM_TYPE_INVAL_CURSOR_CACHE);
> -        if (!cursor->common.during_target_migrate) {
> +        if
> (!common_graphics_channel_get_during_target_migrate(COMMON_GRAPHICS_CHANNEL(cursor)))
> {
>              red_pipes_add_verb(channel, SPICE_MSG_CURSOR_RESET);
>          }
>          if (!red_channel_wait_all_sent(&cursor->common.base,
> @@ -405,7 +407,7 @@ static void cursor_channel_init_client(CursorChannel
> *cursor, CursorChannelClien
>      spice_return_if_fail(cursor);
>  
>      if (!red_channel_is_connected(&cursor->common.base)
> -        || COMMON_GRAPHICS_CHANNEL(cursor)->during_target_migrate) {
> +        ||
> common_graphics_channel_get_during_target_migrate(COMMON_GRAPHICS_CHANNEL(cursor)))
> {
>          spice_debug("during_target_migrate: skip init");
>          return;
>      }
> diff --git a/server/dcc-send.c b/server/dcc-send.c
> index c49adab..e33f428 100644
> --- a/server/dcc-send.c
> +++ b/server/dcc-send.c
> @@ -2311,7 +2311,7 @@ static void marshall_gl_scanout(RedChannelClient *rcc,
>  {
>      DisplayChannelClient *dcc = DISPLAY_CHANNEL_CLIENT(rcc);
>      DisplayChannel *display_channel = DCC_TO_DC(dcc);
> -    QXLInstance* qxl = display_channel->common.qxl;
> +    QXLInstance* qxl =
> common_graphics_channel_get_qxl(COMMON_GRAPHICS_CHANNEL(display_channel));
>  
>      SpiceMsgDisplayGlScanoutUnix *scanout = red_qxl_get_gl_scanout(qxl);
>      if (scanout != NULL) {
> diff --git a/server/dcc.c b/server/dcc.c
> index 3ded1b9..3519d2e 100644
> --- a/server/dcc.c
> +++ b/server/dcc.c
> @@ -284,7 +284,8 @@ void dcc_create_surface(DisplayChannelClient *dcc, int
> surface_id)
>      flags = is_primary_surface(DCC_TO_DC(dcc), surface_id) ?
>      SPICE_SURFACE_FLAGS_PRIMARY : 0;
>  
>      /* don't send redundant create surface commands to client */
> -    if (!dcc || display->common.during_target_migrate ||
> +    if (!dcc ||
> +
> common_graphics_channel_get_during_target_migrate(COMMON_GRAPHICS_CHANNEL(display))
> ||
>          dcc->priv->surface_client_created[surface_id]) {
>          return;
>      }
> @@ -514,8 +515,8 @@ DisplayChannelClient *dcc_new(DisplayChannel *display,
>                           "zlib-glz-state", zlib_glz_state,
>                           NULL);
>      spice_info("New display (client %p) dcc %p stream %p", client, dcc,
>      stream);
> -    display->common.during_target_migrate = mig_target;
> -    dcc->priv->id = display->common.qxl->id;
> +
> common_graphics_channel_set_during_target_migrate(COMMON_GRAPHICS_CHANNEL(display),
> mig_target);
> +    dcc->priv->id =
> common_graphics_channel_get_qxl(COMMON_GRAPHICS_CHANNEL(display))->id;
>  
>      if (common_caps_array)
>          g_array_unref(common_caps_array);
> @@ -749,7 +750,7 @@ void dcc_destroy_surface(DisplayChannelClient *dcc,
> uint32_t surface_id)
>      display = DCC_TO_DC(dcc);
>      channel = RED_CHANNEL(display);
>  
> -    if (COMMON_GRAPHICS_CHANNEL(display)->during_target_migrate ||
> +    if
> (common_graphics_channel_get_during_target_migrate(COMMON_GRAPHICS_CHANNEL(display))
> ||
>          !dcc->priv->surface_client_created[surface_id]) {
>          return;
>      }
> diff --git a/server/display-channel.c b/server/display-channel.c
> index b9e2b93..69edd35 100644
> --- a/server/display-channel.c
> +++ b/server/display-channel.c
> @@ -172,7 +172,7 @@ static void stop_streams(DisplayChannel *display)
>  void display_channel_surface_unref(DisplayChannel *display, uint32_t
>  surface_id)
>  {
>      RedSurface *surface = &display->priv->surfaces[surface_id];
> -    QXLInstance *qxl = display->common.qxl;
> +    QXLInstance *qxl =
> common_graphics_channel_get_qxl(COMMON_GRAPHICS_CHANNEL(display));
>      DisplayChannelClient *dcc;
>      GListIter iter;
>  
> @@ -1831,7 +1831,7 @@ void display_channel_create_surface(DisplayChannel
> *display, uint32_t surface_id
>  
>      if (display->priv->renderer == RED_RENDERER_INVALID) {
>          int i;
> -        QXLInstance *qxl = display->common.qxl;
> +        QXLInstance *qxl =
> common_graphics_channel_get_qxl(COMMON_GRAPHICS_CHANNEL(display));
>          RedsState *reds = red_qxl_get_server(qxl->st);
>          GArray *renderers = reds_get_renderers(reds);
>          for (i = 0; i < renderers->len; i++) {
> @@ -2037,7 +2037,7 @@ void display_channel_gl_scanout(DisplayChannel
> *display)
>  
>  static void set_gl_draw_async_count(DisplayChannel *display, int num)
>  {
> -    QXLInstance *qxl = display->common.qxl;
> +    QXLInstance *qxl =
> common_graphics_channel_get_qxl(COMMON_GRAPHICS_CHANNEL(display));
>  
>      display->priv->gl_draw_async_count = num;
>  
> diff --git a/server/red-worker.c b/server/red-worker.c
> index 2dfa8e4..678856b 100644
> --- a/server/red-worker.c
> +++ b/server/red-worker.c
> @@ -533,7 +533,9 @@ static void dev_create_primary_surface(RedWorker *worker,
> uint32_t surface_id,
>                                     line_0, surface.flags &
>                                     QXL_SURF_FLAG_KEEP_DATA, TRUE);
>      display_channel_set_monitors_config_to_primary(display);
>  
> -    if (display_is_connected(worker) &&
> !worker->display_channel->common.during_target_migrate) {
> +    CommonGraphicsChannel *common =
> COMMON_GRAPHICS_CHANNEL(worker->display_channel);
> +    if (display_is_connected(worker) &&
> +        !common_graphics_channel_get_during_target_migrate(common)) {
>          /* guest created primary, so it will (hopefully) send a
>          monitors_config
>           * now, don't send our own temporary one */
>          if (!worker->driver_cap_monitors_config) {
> @@ -638,10 +640,12 @@ static void handle_dev_start(void *opaque, void
> *payload)
>  
>      spice_assert(!worker->running);
>      if (worker->cursor_channel) {
> -
> COMMON_GRAPHICS_CHANNEL(worker->cursor_channel)->during_target_migrate
> = FALSE;
> +        CommonGraphicsChannel *common =
> COMMON_GRAPHICS_CHANNEL(worker->cursor_channel);
> +        common_graphics_channel_set_during_target_migrate(common, FALSE);
>      }
>      if (worker->display_channel) {
> -        worker->display_channel->common.during_target_migrate = FALSE;
> +        CommonGraphicsChannel *common =
> COMMON_GRAPHICS_CHANNEL(worker->display_channel);
> +        common_graphics_channel_set_during_target_migrate(common, FALSE);
>          display_channel_wait_for_migrate_data(worker->display_channel);
>      }
>      worker->running = TRUE;

Beside that

Acked-by: Frediano Ziglio <fziglio at redhat.com>

Frediano


More information about the Spice-devel mailing list