Re: [PATCH 08/17] CommonChannel -> CommonWorkerChannel

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

 



On Thu, 2016-02-11 at 14:44 -0500, Frediano Ziglio wrote:
> > 
> > From: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
> > 
> > Rename this struct to make it clear that it's only the base class for
> > worker channels (e.g. display and cursor), not all channels.
> > 
> > Also renamed CommonChannelClient to CommonWorkerChannelClient.
> 
> I quite disagree with this change.
> 
> I would like to remove the dependency from Cursor/DisplayChannel to
> the Worker. The dependency is weak and not necessary (I already posted
> a patch for this and is quite small).
> 
> I agree however that the Common is not clear between who, just the Worker
> name (unless by Worker you mean something different than RedWorker). 
> As they represent graphic card channels I could suggest CommonGraphicClient,
> but actually I'm not that comfortable with this either.


I agree that the name CommonWorkerChannel is not ideal. But I do feel that it
needs some name change due to the reason mentioned in the commit log.
CommonGraphicsChannel is not too bad, although this class may not necessarily be
a base class for future graphics-related channels (e.g. virgl-related stuff).
Perhaps CommonQxlChannel?  Not really sure.


> 
> Frediano
> 
> > ---
> >  server/cursor-channel.c  | 26 +++++++++++++-------------
> >  server/dcc.c             |  8 ++++----
> >  server/dcc.h             |  4 ++--
> >  server/display-channel.c |  6 +++---
> >  server/display-channel.h |  2 +-
> >  server/red-worker.c      | 42 +++++++++++++++++++++---------------------
> >  server/red-worker.h      | 44 ++++++++++++++++++++++----------------------
> >  7 files changed, 66 insertions(+), 66 deletions(-)
> > 
> > diff --git a/server/cursor-channel.c b/server/cursor-channel.c
> > index 4c15582..45b7e38 100644
> > --- a/server/cursor-channel.c
> > +++ b/server/cursor-channel.c
> > @@ -59,7 +59,7 @@ typedef struct CursorPipeItem {
> >  } CursorPipeItem;
> >  
> >  struct CursorChannel {
> > -    CommonChannel common; // Must be the first thing
> > +    CommonWorkerChannel common; // Must be the first thing
> >  
> >      CursorItem *item;
> >      int cursor_visible;
> > @@ -74,7 +74,7 @@ struct CursorChannel {
> >  };
> >  
> >  struct CursorChannelClient {
> > -    CommonChannelClient common;
> > +    CommonWorkerChannelClient common;
> >  
> >      CacheItem *cursor_cache[CURSOR_CACHE_HASH_SIZE];
> >      Ring cursor_cache_lru;
> > @@ -427,7 +427,7 @@ static void cursor_channel_release_item(RedChannelClient
> > *rcc, PipeItem *item, i
> >  CursorChannel* cursor_channel_new(RedWorker *worker)
> >  {
> >      CursorChannel *cursor_channel;
> > -    CommonChannel *channel = NULL;
> > +    CommonWorkerChannel *channel = NULL;
> >      ChannelCbs cbs = {
> >          .on_disconnect =  cursor_channel_client_on_disconnect,
> >          .send_item = cursor_channel_send_item,
> > @@ -470,15 +470,15 @@ CursorChannelClient*
> > cursor_channel_client_new(CursorChannel *cursor, RedClient
> >      spice_return_val_if_fail(!num_caps || caps, NULL);
> >  
> >      CursorChannelClient *ccc =
> > -        (CursorChannelClient*)common_channel_new_client(&cursor->common,
> > -
> > sizeof(CursorChannelClient),
> > -                                                        client, stream,
> > -                                                        mig_target,
> > -                                                        FALSE,
> > -                                                        common_caps,
> > -                                                        num_common_caps,
> > -                                                        caps,
> > -                                                        num_caps);
> > +
> > (CursorChannelClient*)common_worker_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);
> > @@ -561,7 +561,7 @@ void cursor_channel_init(CursorChannel *cursor,
> > CursorChannelClient *client)
> >      spice_return_if_fail(cursor);
> >  
> >      if (!red_channel_is_connected(&cursor->common.base)
> > -        || COMMON_CHANNEL(cursor)->during_target_migrate) {
> > +        || COMMON_WORKER_CHANNEL(cursor)->during_target_migrate) {
> >          spice_debug("during_target_migrate: skip init");
> >          return;
> >      }
> > diff --git a/server/dcc.c b/server/dcc.c
> > index 5ccbda1..684fef1 100644
> > --- a/server/dcc.c
> > +++ b/server/dcc.c
> > @@ -368,8 +368,8 @@ DisplayChannelClient *dcc_new(DisplayChannel *display,
> >  {
> >      DisplayChannelClient *dcc;
> >  
> > -    dcc = (DisplayChannelClient*)common_channel_new_client(
> > -        COMMON_CHANNEL(display), sizeof(DisplayChannelClient),
> > +    dcc = (DisplayChannelClient*)common_worker_channel_new_client(
> > +        COMMON_WORKER_CHANNEL(display), sizeof(DisplayChannelClient),
> >          client, stream, mig_target, TRUE,
> >          common_caps, num_common_caps,
> >          caps, num_caps);
> > @@ -445,7 +445,7 @@ void dcc_start(DisplayChannelClient *dcc)
> >  {
> >      DisplayChannel *display = DCC_TO_DC(dcc);
> >      RedChannelClient *rcc = RED_CHANNEL_CLIENT(dcc);
> > -    QXLInstance *qxl = red_worker_get_qxl(COMMON_CHANNEL(display)->worker);
> > +    QXLInstance *qxl =
> > red_worker_get_qxl(COMMON_WORKER_CHANNEL(display)->worker);
> >  
> >      red_channel_client_push_set_ack(RED_CHANNEL_CLIENT(dcc));
> >  
> > @@ -626,7 +626,7 @@ void dcc_destroy_surface(DisplayChannelClient *dcc,
> > uint32_t surface_id)
> >      display = DCC_TO_DC(dcc);
> >      channel = RED_CHANNEL(display);
> >  
> > -    if (COMMON_CHANNEL(display)->during_target_migrate ||
> > +    if (COMMON_WORKER_CHANNEL(display)->during_target_migrate ||
> >          !dcc->surface_client_created[surface_id]) {
> >          return;
> >      }
> > diff --git a/server/dcc.h b/server/dcc.h
> > index a482938..c78f3b8 100644
> > --- a/server/dcc.h
> > +++ b/server/dcc.h
> > @@ -55,7 +55,7 @@ typedef struct FreeList {
> >  } FreeList;
> >  
> >  struct DisplayChannelClient {
> > -    CommonChannelClient common;
> > +    CommonWorkerChannelClient common;
> >      SpiceImageCompression image_compression;
> >      spice_wan_compression_t jpeg_state;
> >      spice_wan_compression_t zlib_glz_state;
> > @@ -115,7 +115,7 @@ struct DisplayChannelClient {
> >  };
> >  
> >  #define DCC_TO_WORKER(dcc)                                              \
> > -    (SPICE_CONTAINEROF((dcc)->common.base.channel, CommonChannel,
> > base)->worker)
> > +    (SPICE_CONTAINEROF((dcc)->common.base.channel, CommonWorkerChannel,
> > base)->worker)
> >  #define DCC_TO_DC(dcc)                                                  \
> >       SPICE_CONTAINEROF((dcc)->common.base.channel, DisplayChannel,
> >       common.base)
> >  #define RCC_TO_DCC(rcc) SPICE_CONTAINEROF((rcc), DisplayChannelClient,
> >  common.base)
> > diff --git a/server/display-channel.c b/server/display-channel.c
> > index 5b496ae..c46676d 100644
> > --- a/server/display-channel.c
> > +++ b/server/display-channel.c
> > @@ -241,7 +241,7 @@ static void stop_streams(DisplayChannel *display)
> >  void display_channel_surface_unref(DisplayChannel *display, uint32_t
> >  surface_id)
> >  {
> >      RedSurface *surface = &display->surfaces[surface_id];
> > -    RedWorker *worker = COMMON_CHANNEL(display)->worker;
> > +    RedWorker *worker = COMMON_WORKER_CHANNEL(display)->worker;
> >      QXLInstance *qxl = red_worker_get_qxl(worker);
> >      DisplayChannelClient *dcc;
> >      RingItem *link, *next;
> > @@ -1445,7 +1445,7 @@ void display_channel_drawable_unref(DisplayChannel
> > *display, Drawable *drawable)
> >          ring_remove(item);
> >      }
> >      if (drawable->red_drawable) {
> > -        red_drawable_unref(COMMON_CHANNEL(display)->worker,
> > drawable->red_drawable, drawable->group_id);
> > +        red_drawable_unref(COMMON_WORKER_CHANNEL(display)->worker,
> > drawable->red_drawable, drawable->group_id);
> >      }
> >      drawable_free(display, drawable);
> >      display->drawable_count--;
> > @@ -2155,7 +2155,7 @@ void display_channel_gl_scanout(DisplayChannel
> > *display)
> >  
> >  static void set_gl_draw_async_count(DisplayChannel *display, int num)
> >  {
> > -    RedWorker *worker = COMMON_CHANNEL(display)->worker;
> > +    RedWorker *worker = COMMON_WORKER_CHANNEL(display)->worker;
> >      QXLInstance *qxl = red_worker_get_qxl(worker);
> >  
> >      display->gl_draw_async_count = num;
> > diff --git a/server/display-channel.h b/server/display-channel.h
> > index e0b18ca..62aaaf6 100644
> > --- a/server/display-channel.h
> > +++ b/server/display-channel.h
> > @@ -165,7 +165,7 @@ struct _Drawable {
> >  };
> >  
> >  struct DisplayChannel {
> > -    CommonChannel common; // Must be the first thing
> > +    CommonWorkerChannel common; // Must be the first thing
> >      uint32_t bits_unique;
> >  
> >      MonitorsConfig *monitors_config;
> > diff --git a/server/red-worker.c b/server/red-worker.c
> > index d4e10d7..03d05dd 100644
> > --- a/server/red-worker.c
> > +++ b/server/red-worker.c
> > @@ -114,7 +114,7 @@ static int display_is_connected(RedWorker *worker)
> >  
> >  static uint8_t *common_alloc_recv_buf(RedChannelClient *rcc, uint16_t type,
> >  uint32_t size)
> >  {
> > -    CommonChannel *common = SPICE_CONTAINEROF(rcc->channel, CommonChannel,
> > base);
> > +    CommonWorkerChannel *common = SPICE_CONTAINEROF(rcc->channel,
> > CommonWorkerChannel, base);
> >  
> >      /* SPICE_MSGC_MIGRATE_DATA is the only client message whose size is
> >      dynamic */
> >      if (type == SPICE_MSGC_MIGRATE_DATA) {
> > @@ -432,7 +432,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 = COMMON_CHANNEL_CLIENT(rcc);
> > +    CommonWorkerChannelClient *ccc = COMMON_WORKER_CHANNEL_CLIENT(rcc);
> >      int flags;
> >      int delay_val;
> >  
> > @@ -464,16 +464,16 @@ static int
> > common_channel_config_socket(RedChannelClient *rcc)
> >      return TRUE;
> >  }
> >  
> > -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)
> > +CommonWorkerChannelClient
> > *common_worker_channel_new_client(CommonWorkerChannel *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,
> > @@ -481,7 +481,7 @@ CommonChannelClient
> > *common_channel_new_client(CommonChannel *common,
> >      if (!rcc) {
> >          return NULL;
> >      }
> > -    CommonChannelClient *common_cc = (CommonChannelClient*)rcc;
> > +    CommonWorkerChannelClient *common_cc = (CommonWorkerChannelClient*)rcc;
> >      common_cc->worker = common->worker;
> >      common_cc->id = common->worker->qxl->id;
> >      common->during_target_migrate = mig_target;
> > @@ -494,14 +494,14 @@ CommonChannelClient
> > *common_channel_new_client(CommonChannel *common,
> >  }
> >  
> >  
> > -CommonChannel *red_worker_new_channel(RedWorker *worker, int size,
> > -                                   const char *name,
> > -                                   uint32_t channel_type, int
> > migration_flags,
> > -                                   ChannelCbs *channel_cbs,
> > -                                   channel_handle_parsed_proc
> > handle_parsed)
> > +CommonWorkerChannel *red_worker_new_channel(RedWorker *worker, int size,
> > +                                            const char *name,
> > +                                            uint32_t channel_type, int
> > migration_flags,
> > +                                            ChannelCbs *channel_cbs,
> > +                                            channel_handle_parsed_proc
> > handle_parsed)
> >  {
> >      RedChannel *channel = NULL;
> > -    CommonChannel *common;
> > +    CommonWorkerChannel *common;
> >  
> >      spice_return_val_if_fail(worker, NULL);
> >      spice_return_val_if_fail(channel_cbs, NULL);
> > @@ -523,7 +523,7 @@ CommonChannel *red_worker_new_channel(RedWorker *worker,
> > int size,
> >      spice_return_val_if_fail(channel, NULL);
> >      red_channel_set_stat_node(channel, reds_stat_add_node(reds,
> >      worker->stat, name, TRUE));
> >  
> > -    common = (CommonChannel *)channel;
> > +    common = (CommonWorkerChannel *)channel;
> >      common->worker = worker;
> >      return common;
> >  }
> > @@ -842,7 +842,7 @@ static void handle_dev_start(void *opaque, void
> > *payload)
> >  
> >      spice_assert(!worker->running);
> >      if (worker->cursor_channel) {
> > -        COMMON_CHANNEL(worker->cursor_channel)->during_target_migrate =
> > FALSE;
> > +        COMMON_WORKER_CHANNEL(worker->cursor_channel)
> > ->during_target_migrate
> > = FALSE;
> >      }
> >      if (worker->display_channel) {
> >          worker->display_channel->common.during_target_migrate = FALSE;
> > diff --git a/server/red-worker.h b/server/red-worker.h
> > index 91533e1..8e499a1 100644
> > --- a/server/red-worker.h
> > +++ b/server/red-worker.h
> > @@ -24,19 +24,19 @@
> >  
> >  typedef struct RedWorker RedWorker;
> >  
> > -typedef struct CommonChannelClient {
> > +typedef struct CommonWorkerChannelClient {
> >      RedChannelClient base;
> >  
> >      uint32_t id;
> >      RedWorker *worker;
> >      int is_low_bandwidth;
> > -} CommonChannelClient;
> > +} CommonWorkerChannelClient;
> >  
> > -#define COMMON_CHANNEL_CLIENT(Client) ((CommonChannelClient*)(Client))
> > +#define COMMON_WORKER_CHANNEL_CLIENT(Client)
> > ((CommonWorkerChannelClient*)(Client))
> >  #define COMMON_CLIENT_TIMEOUT (NSEC_PER_SEC * 30)
> >  
> >  #define CHANNEL_RECEIVE_BUF_SIZE 1024
> > -typedef struct CommonChannel {
> > +typedef struct CommonWorkerChannel {
> >      RedChannel base; // Must be the first thing
> >  
> >      struct RedWorker *worker;
> > @@ -47,9 +47,9 @@ typedef struct CommonChannel {
> >                                    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) */
> > -} CommonChannel;
> > +} CommonWorkerChannel;
> >  
> > -#define COMMON_CHANNEL(Channel) ((CommonChannel*)(Channel))
> > +#define COMMON_WORKER_CHANNEL(Channel) ((CommonWorkerChannel*)(Channel))
> >  
> >  enum {
> >      PIPE_ITEM_TYPE_VERB = PIPE_ITEM_TYPE_CHANNEL_BASE,
> > @@ -102,21 +102,21 @@ RedMemSlotInfo* red_worker_get_memslot(RedWorker
> > *worker);
> >  void red_drawable_unref(RedWorker *worker, RedDrawable *red_drawable,
> >                          uint32_t group_id);
> >  
> > -CommonChannel *red_worker_new_channel(RedWorker *worker, int size,
> > -                                   const char *name,
> > -                                   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);
> > +CommonWorkerChannel *red_worker_new_channel(RedWorker *worker, int size,
> > +                                            const char *name,
> > +                                            uint32_t channel_type, int
> > migration_flags,
> > +                                            ChannelCbs *channel_cbs,
> > +                                            channel_handle_parsed_proc
> > handle_parsed);
> > +
> > +CommonWorkerChannelClient
> > *common_worker_channel_new_client(CommonWorkerChannel *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
> _______________________________________________
> 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]