Re: [PATCH spice-server v3 1/3] Add CommonGraphicsChannelPrivate struct

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 2016-10-14 at 10:40 -0400, Frediano Ziglio wrote:
> > 
> > From: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
> > 
> > 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(CommonGraphicsCh
> > annel
> > *self, gboolean value)
> > +{
> > +    self->priv->during_target_migrate = value;
> > +}
> > +
> > +gboolean
> > common_graphics_channel_get_during_target_migrate(CommonGraphicsCh
> > annel
> > *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(CommonGraphicsCh
> > annel
> > *self, gboolean value);
> > +gboolean
> > common_graphics_channel_get_during_target_migrate(CommonGraphicsCh
> > annel
> > *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.
> 
I agree with it

Pavel

> >      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_GRAPHIC
> > S_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_ch
> > annel));
> >  
> >      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@xxxxxxxxxx>
> 
> Frediano
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]