[Spice-devel] [PATCH spice-server 06/10] Move thread/dispatching handling to RedChannel

Jonathon Jongsma jjongsma at redhat.com
Wed Mar 20 20:14:51 UTC 2019


On Wed, 2019-03-20 at 09:59 +0000, Frediano Ziglio wrote:
> Currently channel threading/handling is spread between RedQxl,
> RedWorker and RedChannel.
> Move more to RedChannel simplify RedQxl and RedWorker.
> 
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
>  server/cursor-channel.c    |   4 +-
>  server/cursor-channel.h    |   4 +-
>  server/display-channel.c   |   2 +
>  server/display-channel.h   |   1 +
>  server/red-channel.c       | 111 +++++++++++++++++++++++++++++--
>  server/red-qxl.c           | 110 +-----------------------------
>  server/red-replay-qxl.c    |   3 -
>  server/red-stream-device.c |   2 +-
>  server/red-worker.c        | 133 ++++++++++-------------------------
> --
>  server/red-worker.h        |  46 ++-----------
>  10 files changed, 160 insertions(+), 256 deletions(-)
> 
> diff --git a/server/cursor-channel.c b/server/cursor-channel.c
> index 4220084f..e8af01b0 100644
> --- a/server/cursor-channel.c
> +++ b/server/cursor-channel.c
> @@ -228,7 +228,8 @@ static void
> cursor_channel_send_item(RedChannelClient *rcc, RedPipeItem *pipe_it
>  }
>  
>  CursorChannel* cursor_channel_new(RedsState *server, int id,
> -                                  const SpiceCoreInterfaceInternal
> *core)
> +                                  const SpiceCoreInterfaceInternal
> *core,
> +                                  Dispatcher *dispatcher)
>  {
>      spice_debug("create cursor channel");
>      return g_object_new(TYPE_CURSOR_CHANNEL,
> @@ -238,6 +239,7 @@ CursorChannel* cursor_channel_new(RedsState
> *server, int id,
>                          "id", id,
>                          "migration-flags", 0,
>                          "handle-acks", TRUE,
> +                        "dispatcher", dispatcher,
>                          NULL);
>  }
>  
> diff --git a/server/cursor-channel.h b/server/cursor-channel.h
> index 603c2c0a..767325ea 100644
> --- a/server/cursor-channel.h
> +++ b/server/cursor-channel.h
> @@ -21,6 +21,7 @@
>  
>  #include "common-graphics-channel.h"
>  #include "red-parse-qxl.h"
> +#include "dispatcher.h"
>  
>  G_BEGIN_DECLS
>  
> @@ -57,7 +58,8 @@ GType cursor_channel_get_type(void) G_GNUC_CONST;
>   * CursorChannel thread.
>   */
>  CursorChannel* cursor_channel_new(RedsState *server, int id,
> -                                  const SpiceCoreInterfaceInternal
> *core);
> +                                  const SpiceCoreInterfaceInternal
> *core,
> +                                  Dispatcher *dispatcher);
>  
>  void                 cursor_channel_reset       (CursorChannel
> *cursor);
>  void                 cursor_channel_do_init     (CursorChannel
> *cursor);
> diff --git a/server/display-channel.c b/server/display-channel.c
> index e179abfd..cb052bfc 100644
> --- a/server/display-channel.c
> +++ b/server/display-channel.c
> @@ -2227,6 +2227,7 @@ static SpiceCanvas
> *image_surfaces_get(SpiceImageSurfaces *surfaces, uint32_t su
>  DisplayChannel* display_channel_new(RedsState *reds,
>                                      QXLInstance *qxl,
>                                      const SpiceCoreInterfaceInternal
> *core,
> +                                    Dispatcher *dispatcher,
>                                      int migrate, int stream_video,
>                                      GArray *video_codecs,
>                                      uint32_t n_surfaces)
> @@ -2246,6 +2247,7 @@ DisplayChannel* display_channel_new(RedsState
> *reds,
>                             "n-surfaces", n_surfaces,
>                             "video-codecs", video_codecs,
>                             "handle-acks", TRUE,
> +                           "dispatcher", dispatcher,
>                             NULL);
>      if (display) {
>          display_channel_set_stream_video(display, stream_video);
> diff --git a/server/display-channel.h b/server/display-channel.h
> index 948018cf..f64da0bd 100644
> --- a/server/display-channel.h
> +++ b/server/display-channel.h
> @@ -99,6 +99,7 @@ struct Drawable {
>  DisplayChannel*            display_channel_new                      
>  (RedsState *reds,
>                                                                      
>   QXLInstance *qxl,
>                                                                      
>   const SpiceCoreInterfaceInternal *core,
> +                                                                    
>   Dispatcher *dispatcher,
>                                                                      
>   int migrate,
>                                                                      
>   int stream_video,
>                                                                      
>   GArray *video_codecs,
> diff --git a/server/red-channel.c b/server/red-channel.c
> index 1d88739e..8d05c971 100644
> --- a/server/red-channel.c
> +++ b/server/red-channel.c
> @@ -70,6 +70,9 @@ struct RedChannelPrivate
>      uint32_t type;
>      uint32_t id;
>  
> +    /* "core" interface to register events.
> +     * Can be thread specific.
> +     */

useful comment, but not really related to this commit...

>      SpiceCoreInterfaceInternal *core;
>      gboolean handle_acks;
>  
> @@ -90,12 +93,29 @@ struct RedChannelPrivate
>      // TODO: when different channel_clients are in different threads
>      // from Channel -> need to protect!
>      pthread_t thread_id;


Add comment explaining why and when a dispatcher would be used?

> +    Dispatcher *dispatcher;
>      RedsState *reds;
>      RedStatNode stat;
>  };
>  
>  G_DEFINE_ABSTRACT_TYPE_WITH_PRIVATE(RedChannel, red_channel,
> G_TYPE_OBJECT)
>  


Add a comment that these messages are used with the dispatcher to
dispatch client callbacks to the appropriate thread.

> +typedef struct RedMessageConnect {
> +    RedChannel *channel;
> +    RedClient *client;
> +    RedStream *stream;
> +    RedChannelCapabilities caps;
> +    int migration;
> +} RedMessageConnect;
> +
> +typedef struct RedMessageDisconnect {
> +    RedChannelClient *rcc;
> +} RedMessageDisconnect;
> +
> +typedef struct RedMessageMigrate {
> +    RedChannelClient *rcc;
> +} RedMessageMigrate;
> +
>  enum {
>      PROP0,
>      PROP_SPICE_SERVER,
> @@ -103,7 +123,8 @@ enum {
>      PROP_TYPE,
>      PROP_ID,
>      PROP_HANDLE_ACKS,
> -    PROP_MIGRATION_FLAGS
> +    PROP_MIGRATION_FLAGS,
> +    PROP_DISPATCHER,
>  };
>  
>  static void
> @@ -134,6 +155,9 @@ red_channel_get_property(GObject *object,
>          case PROP_MIGRATION_FLAGS:
>              g_value_set_uint(value, self->priv->migration_flags);
>              break;
> +        case PROP_DISPATCHER:
> +            g_value_set_object(value, self->priv->dispatcher);
> +            break;
>          default:
>              G_OBJECT_WARN_INVALID_PROPERTY_ID(object, property_id,
> pspec);
>      }
> @@ -167,6 +191,12 @@ red_channel_set_property(GObject *object,
>          case PROP_MIGRATION_FLAGS:
>              self->priv->migration_flags = g_value_get_uint(value);
>              break;
> +        case PROP_DISPATCHER:
> +            if (self->priv->dispatcher) {
> +                g_object_unref(self->priv->dispatcher);
> +            }

Above check is not strictly necessary since it's a construct-only
property, but better to be defensive, I suppose.

> +            self->priv->dispatcher = g_value_dup_object(value);
> +            break;
>          default:
>              G_OBJECT_WARN_INVALID_PROPERTY_ID(object, property_id,
> pspec);
>      }
> @@ -177,6 +207,7 @@ red_channel_finalize(GObject *object)
>  {
>      RedChannel *self = RED_CHANNEL(object);
>  
> +    g_clear_object(&self->priv->dispatcher);
>      red_channel_capabilities_reset(&self->priv->local_caps);
>  
>      G_OBJECT_CLASS(red_channel_parent_class)->finalize(object);
> @@ -279,6 +310,14 @@ red_channel_class_init(RedChannelClass *klass)
>                               G_PARAM_CONSTRUCT_ONLY |
>                               G_PARAM_STATIC_STRINGS);
>      g_object_class_install_property(object_class,
> PROP_MIGRATION_FLAGS, spec);
> +
> +    spec = g_param_spec_object("dispatcher", "dispatcher",
> +                               "Dispatcher bound to channel thread",
> +                               TYPE_DISPATCHER,
> +                               G_PARAM_STATIC_STRINGS
> +                               | G_PARAM_READWRITE
> +                               | G_PARAM_CONSTRUCT_ONLY);
> +    g_object_class_install_property(object_class, PROP_DISPATCHER,
> spec);
>  }
>  
>  static void
> @@ -483,11 +522,40 @@ void red_channel_disconnect(RedChannel
> *channel)
>      red_channel_foreach_client(channel,
> red_channel_client_disconnect);
>  }
>  
> +static void handle_dispatcher_connect(void *opaque, void *payload)
> +{
> +    RedMessageConnect *msg = payload;
> +    RedChannel *channel = msg->channel;
> +
> +    channel->priv->client_cbs.connect(channel, msg->client, msg-
> >stream,
> +                                      msg->migration, &msg->caps);
> +    g_object_unref(msg->client);
> +    red_channel_capabilities_reset(&msg->caps);
> +}
> +
>  void red_channel_connect(RedChannel *channel, RedClient *client,
>                           RedStream *stream, int migration,
>                           RedChannelCapabilities *caps)
>  {
> -    channel->priv->client_cbs.connect(channel, client, stream,
> migration, caps);
> +    if (channel->priv->dispatcher == NULL ||
> +        pthread_equal(pthread_self(), channel->priv->thread_id)) {
> +        channel->priv->client_cbs.connect(channel, client, stream,
> migration, caps);
> +        return;
> +    }
> +
> +    RedMessageConnect payload = {0,};

Why not just initialize the payload structure directly here as you do
below for the other callbacks?

> +    Dispatcher *dispatcher = channel->priv->dispatcher;
> +
> +    // get a reference potentially the main channel can be destroyed
> in
> +    // the main thread causing RedClient to be destroyed before
> using it
> +    payload.channel = channel;
> +    payload.client = g_object_ref(client);
> +    payload.stream = stream;
> +    payload.migration = migration;
> +    red_channel_capabilities_init(&payload.caps, caps);
> +
> +    dispatcher_send_message_generic(dispatcher,
> handle_dispatcher_connect,
> +                                    &payload, sizeof(payload),
> false);
>  }
>  
>  GList *red_channel_get_clients(RedChannel *channel)
> @@ -690,12 +758,47 @@ const RedChannelCapabilities*
> red_channel_get_local_capabilities(RedChannel *sel
>      return &self->priv->local_caps;
>  }
>  
> +static void handle_dispatcher_migrate(void *opaque, void *payload)
> +{
> +    RedMessageMigrate *msg = payload;
> +    RedChannel *channel = red_channel_client_get_channel(msg->rcc);
> +
> +    channel->priv->client_cbs.migrate(msg->rcc);
> +    g_object_unref(msg->rcc);
> +}
> +
>  void red_channel_migrate_client(RedChannel *channel,
> RedChannelClient *rcc)
>  {
> -    channel->priv->client_cbs.migrate(rcc);
> +    if (channel->priv->dispatcher == NULL ||
> +        pthread_equal(pthread_self(), channel->priv->thread_id)) {
> +        channel->priv->client_cbs.migrate(rcc);
> +        return;
> +    }
> +
> +    RedMessageMigrate payload = { .rcc = g_object_ref(rcc) };
> +    dispatcher_send_message_generic(channel->priv->dispatcher,
> handle_dispatcher_migrate,
> +                                    &payload, sizeof(payload),
> false);
> +}
> +
> +static void handle_dispatcher_disconnect(void *opaque, void
> *payload)
> +{
> +    RedMessageDisconnect *msg = payload;
> +    RedChannel *channel = red_channel_client_get_channel(msg->rcc);
> +
> +    channel->priv->client_cbs.disconnect(msg->rcc);
>  }
>  
>  void red_channel_disconnect_client(RedChannel *channel,
> RedChannelClient *rcc)
>  {
> -    channel->priv->client_cbs.disconnect(rcc);
> +    if (channel->priv->dispatcher == NULL ||
> +        pthread_equal(pthread_self(), channel->priv->thread_id)) {
> +        channel->priv->client_cbs.disconnect(rcc);
> +        return;
> +    }
> +
> +    // TODO: we turned it to be sync, due to client_destroy . Should
> we support async? - for this we will need ref count
> +    // for channels

I don't really understand this comment. But apparently it is just
copied from red-qxl.c...

> +    RedMessageDisconnect payload = { .rcc = rcc };
> +    dispatcher_send_message_generic(channel->priv->dispatcher,
> handle_dispatcher_disconnect,
> +                                    &payload, sizeof(payload),
> true);
>  }
> diff --git a/server/red-qxl.c b/server/red-qxl.c
> index 0dd26b22..af2c7ed5 100644
> --- a/server/red-qxl.c
> +++ b/server/red-qxl.c
> @@ -75,102 +75,6 @@ int red_qxl_check_qxl_version(QXLInstance *qxl,
> int major, int minor)
>              ((qxl_major == major) && (qxl_minor >= minor)));
>  }
>  
> -static void red_qxl_set_display_peer(RedChannel *channel, RedClient
> *client,
> -                                     RedStream *stream, int
> migration,
> -                                     RedChannelCapabilities *caps)
> -{
> -    RedWorkerMessageDisplayConnect payload = {0,};
> -    Dispatcher *dispatcher;
> -
> -    spice_debug("%s", "");
> -    dispatcher = (Dispatcher *)g_object_get_data(G_OBJECT(channel),
> "dispatcher");
> -    // get a reference potentially the main channel can be destroyed
> in
> -    // the main thread causing RedClient to be destroyed before
> using it
> -    payload.client = g_object_ref(client);
> -    payload.stream = stream;
> -    payload.migration = migration;
> -    red_channel_capabilities_init(&payload.caps, caps);
> -
> -    dispatcher_send_message(dispatcher,
> -                            RED_WORKER_MESSAGE_DISPLAY_CONNECT,
> -                            &payload);
> -}
> -
> -static void red_qxl_disconnect_display_peer(RedChannelClient *rcc)
> -{
> -    RedWorkerMessageDisplayDisconnect payload;
> -    Dispatcher *dispatcher;
> -    RedChannel *channel = red_channel_client_get_channel(rcc);
> -
> -    dispatcher = (Dispatcher *)g_object_get_data(G_OBJECT(channel),
> "dispatcher");
> -
> -    payload.rcc = rcc;
> -
> -    // TODO: we turned it to be sync, due to client_destroy . Should
> we support async? - for this we will need ref count
> -    // for channels
> -    dispatcher_send_message(dispatcher,
> -                            RED_WORKER_MESSAGE_DISPLAY_DISCONNECT,
> -                            &payload);
> -}
> -
> -static void red_qxl_display_migrate(RedChannelClient *rcc)
> -{
> -    RedWorkerMessageDisplayMigrate payload;
> -    Dispatcher *dispatcher;
> -    RedChannel *channel = red_channel_client_get_channel(rcc);
> -
> -    dispatcher = (Dispatcher *)g_object_get_data(G_OBJECT(channel),
> "dispatcher");
> -    payload.rcc = g_object_ref(rcc);
> -    dispatcher_send_message(dispatcher,
> -                            RED_WORKER_MESSAGE_DISPLAY_MIGRATE,
> -                            &payload);
> -}
> -
> -static void red_qxl_set_cursor_peer(RedChannel *channel, RedClient
> *client, RedStream *stream,
> -                                    int migration,
> -                                    RedChannelCapabilities *caps)
> -{
> -    RedWorkerMessageCursorConnect payload = {0,};
> -    Dispatcher *dispatcher = (Dispatcher
> *)g_object_get_data(G_OBJECT(channel), "dispatcher");
> -    // get a reference potentially the main channel can be destroyed
> in
> -    // the main thread causing RedClient to be destroyed before
> using it
> -    payload.client = g_object_ref(client);
> -    payload.stream = stream;
> -    payload.migration = migration;
> -    red_channel_capabilities_init(&payload.caps, caps);
> -
> -    dispatcher_send_message(dispatcher,
> -                            RED_WORKER_MESSAGE_CURSOR_CONNECT,
> -                            &payload);
> -}
> -
> -static void red_qxl_disconnect_cursor_peer(RedChannelClient *rcc)
> -{
> -    RedWorkerMessageCursorDisconnect payload;
> -    Dispatcher *dispatcher;
> -    RedChannel *channel = red_channel_client_get_channel(rcc);
> -
> -    dispatcher = (Dispatcher *)g_object_get_data(G_OBJECT(channel),
> "dispatcher");
> -    payload.rcc = rcc;
> -
> -    dispatcher_send_message(dispatcher,
> -                            RED_WORKER_MESSAGE_CURSOR_DISCONNECT,
> -                            &payload);
> -}
> -
> -static void red_qxl_cursor_migrate(RedChannelClient *rcc)
> -{
> -    RedWorkerMessageCursorMigrate payload;
> -    Dispatcher *dispatcher;
> -    RedChannel *channel = red_channel_client_get_channel(rcc);
> -
> -    dispatcher = (Dispatcher *)g_object_get_data(G_OBJECT(channel),
> "dispatcher");
> -    payload.rcc = g_object_ref(rcc);
> -    dispatcher_send_message(dispatcher,
> -                            RED_WORKER_MESSAGE_CURSOR_MIGRATE,
> -                            &payload);
> -}
> -
>  static void red_qxl_update_area(QXLState *qxl_state, uint32_t
> surface_id,
>                                  QXLRect *qxl_area, QXLRect
> *qxl_dirty_rects,
>                                  uint32_t num_dirty_rects, uint32_t
> clear_dirty_region)
> @@ -914,8 +818,6 @@ size_t red_qxl_get_monitors_count(const
> QXLInstance *qxl)
>  void red_qxl_init(RedsState *reds, QXLInstance *qxl)
>  {
>      QXLState *qxl_state;
> -    ClientCbs client_cursor_cbs = { NULL, };
> -    ClientCbs client_display_cbs = { NULL, };
>  
>      spice_return_if_fail(qxl != NULL);
>  
> @@ -948,17 +850,7 @@ void red_qxl_init(RedsState *reds, QXLInstance
> *qxl)
>      qxl_state->max_monitors = UINT_MAX;
>      qxl->st = qxl_state;
>  
> -    // TODO: move to their respective channel files
> -    client_cursor_cbs.connect = red_qxl_set_cursor_peer;
> -    client_cursor_cbs.disconnect = red_qxl_disconnect_cursor_peer;
> -    client_cursor_cbs.migrate = red_qxl_cursor_migrate;
> -
> -    client_display_cbs.connect = red_qxl_set_display_peer;
> -    client_display_cbs.disconnect = red_qxl_disconnect_display_peer;
> -    client_display_cbs.migrate = red_qxl_display_migrate;
> -
> -    qxl_state->worker = red_worker_new(qxl, &client_cursor_cbs,
> -                                       &client_display_cbs);
> +    qxl_state->worker = red_worker_new(qxl);
>  
>      red_worker_run(qxl_state->worker);
>  }
> diff --git a/server/red-replay-qxl.c b/server/red-replay-qxl.c
> index 4884e97e..8124c1fd 100644
> --- a/server/red-replay-qxl.c
> +++ b/server/red-replay-qxl.c
> @@ -1283,9 +1283,6 @@ static void replay_handle_dev_input(QXLWorker
> *worker, SpiceReplay *replay,
>          break;
>      case RED_WORKER_MESSAGE_UPDATE:
>          // XXX do anything? we record the correct bitmaps already.
> -    case RED_WORKER_MESSAGE_DISPLAY_CONNECT:
> -        // we want to ignore this one - it is sent on client
> connection, we
> -        // shall have our own clients
>      case RED_WORKER_MESSAGE_WAKEUP:
>          // safe to ignore
>          break;
> diff --git a/server/red-stream-device.c b/server/red-stream-device.c
> index 78a8c908..cf5a6e9a 100644
> --- a/server/red-stream-device.c
> +++ b/server/red-stream-device.c
> @@ -696,7 +696,7 @@ stream_device_create_channel(StreamDevice *dev)
>  
>      StreamChannel *stream_channel = stream_channel_new(reds, id);
>  
> -    CursorChannel *cursor_channel = cursor_channel_new(reds, id,
> core);
> +    CursorChannel *cursor_channel = cursor_channel_new(reds, id,
> core, NULL);
>      ClientCbs client_cbs = { NULL, };
>      client_cbs.connect = (channel_client_connect_proc)
> cursor_channel_connect;
>      client_cbs.migrate = cursor_channel_client_migrate;
> diff --git a/server/red-worker.c b/server/red-worker.c
> index 3cb12b9c..07329b17 100644
> --- a/server/red-worker.c
> +++ b/server/red-worker.c
> @@ -694,20 +694,22 @@ static void
> handle_dev_create_primary_surface_async(void *opaque, void *payload)
>      red_qxl_async_complete(worker->qxl, msg->base.cookie);
>  }
>  
> -static void handle_dev_display_connect(void *opaque, void *payload)
> +static void
> +handle_dev_display_connect(RedChannel *channel, RedClient *client,
> +                           RedStream *stream, int migration,
> +                           RedChannelCapabilities *caps)
>  {
> -    RedWorkerMessageDisplayConnect *msg = payload;
> -    RedWorker *worker = opaque;
> -    DisplayChannel *display = worker->display_channel;
> +    DisplayChannel *display = DISPLAY_CHANNEL(channel);
>      DisplayChannelClient *dcc;
> +    RedWorker *worker = g_object_get_data(G_OBJECT(channel),
> "worker");
>  
>      spice_debug("connect new client");
> -    spice_return_if_fail(display);
>  
> -    dcc = dcc_new(display, msg->client, msg->stream, msg->migration, 
> &msg->caps,
> -                  worker->image_compression, worker->jpeg_state,
> worker->zlib_glz_state);
> -    g_object_unref(msg->client);
> -    red_channel_capabilities_reset(&msg->caps);
> +    // FIXME not sure how safe is reading directly from reds

I suppose you're talking about the calls to _get_image_compression() /
get_jpeg_state() below? Since these calls happen in the RedWorker
thread, I suppose it's technically not safe. You already have a worker
variable in scope here, you could just use that?

> +    SpiceServer *reds = red_channel_get_server(channel);
> +    dcc = dcc_new(display, client, stream, migration, caps,
> +                  spice_server_get_image_compression(reds),
> reds_get_jpeg_state(reds),
> +                  reds_get_zlib_glz_state(reds));
>      if (!dcc) {
>          return;
>      }
> @@ -716,30 +718,21 @@ static void handle_dev_display_connect(void
> *opaque, void *payload)
>      dcc_start(dcc);
>  }
>  
> -static void handle_dev_display_disconnect(void *opaque, void
> *payload)
> +static void
> +handle_dev_display_disconnect(RedChannelClient *rcc)
>  {
> -    RedWorkerMessageDisplayDisconnect *msg = payload;
> -    RedChannelClient *rcc = msg->rcc;
> -    RedWorker *worker = opaque;
> -
> -    spice_debug("disconnect display client");

Looks like you removed the debug output here, but left in in the
connect callback above. I think I'd keep the debug statement here.

> -    spice_assert(rcc);
> +    RedChannel *channel = red_channel_client_get_channel(rcc);
> +    RedWorker *worker = g_object_get_data(G_OBJECT(channel),
> "worker");
>  
>      guest_set_client_capabilities(worker);
>  
>      red_channel_client_disconnect(rcc);
>  }
>  
> -static void handle_dev_display_migrate(void *opaque, void *payload)
> +static void handle_dev_display_migrate(RedChannelClient *rcc)
>  {
> -    RedWorkerMessageDisplayMigrate *msg = payload;
> -    RedWorker *worker = opaque;
> -
> -    RedChannelClient *rcc = msg->rcc;
> -    spice_debug("migrate display client");
> -    spice_assert(rcc);
> -    red_migrate_display(worker->display_channel, rcc);
> -    g_object_unref(rcc);
> +    DisplayChannel *display =
> DISPLAY_CHANNEL(red_channel_client_get_channel(rcc));
> +    red_migrate_display(display, rcc);
>  }
>  
>  static inline uint32_t qxl_monitors_config_size(uint32_t heads)
> @@ -791,40 +784,6 @@ async_complete:
>      red_qxl_async_complete(worker->qxl, msg->base.cookie);
>  }
>  
> -/* TODO: special, perhaps use another dispatcher? */
> -static void handle_dev_cursor_connect(void *opaque, void *payload)
> -{
> -    RedWorkerMessageCursorConnect *msg = payload;
> -    RedWorker *worker = opaque;
> -
> -    spice_debug("cursor connect");
> -    cursor_channel_connect(worker->cursor_channel,
> -                           msg->client, msg->stream, msg->migration,
> -                           &msg->caps);
> -    g_object_unref(msg->client);
> -    red_channel_capabilities_reset(&msg->caps);
> -}
> -
> -static void handle_dev_cursor_disconnect(void *opaque, void
> *payload)
> -{
> -    RedWorkerMessageCursorDisconnect *msg = payload;
> -    RedChannelClient *rcc = msg->rcc;
> -
> -    spice_debug("disconnect cursor client");
> -    spice_return_if_fail(rcc);
> -    red_channel_client_disconnect(rcc);
> -}
> -
> -static void handle_dev_cursor_migrate(void *opaque, void *payload)
> -{
> -    RedWorkerMessageCursorMigrate *msg = payload;
> -    RedChannelClient *rcc = msg->rcc;
> -
> -    spice_debug("migrate cursor client");
> -    cursor_channel_client_migrate(rcc);
> -    g_object_unref(rcc);
> -}
> -
>  static void handle_dev_set_compression(void *opaque, void *payload)
>  {
>      RedWorkerMessageSetCompression *msg = payload;
> @@ -998,36 +957,6 @@ static void worker_dispatcher_record(void
> *opaque, uint32_t message_type, void *
>  static void register_callbacks(Dispatcher *dispatcher)
>  {
>      /* TODO: register cursor & display specific msg in respective
> channel files */
> -    dispatcher_register_handler(dispatcher,
> -                                RED_WORKER_MESSAGE_DISPLAY_CONNECT,
> -                                handle_dev_display_connect,
> -                                sizeof(RedWorkerMessageDisplayConnec
> t),
> -                                false);
> -    dispatcher_register_handler(dispatcher,
> -                                RED_WORKER_MESSAGE_DISPLAY_DISCONNEC
> T,
> -                                handle_dev_display_disconnect,
> -                                sizeof(RedWorkerMessageDisplayDiscon
> nect),
> -                                true);
> -    dispatcher_register_handler(dispatcher,
> -                                RED_WORKER_MESSAGE_DISPLAY_MIGRATE,
> -                                handle_dev_display_migrate,
> -                                sizeof(RedWorkerMessageDisplayMigrat
> e),
> -                                false);
> -    dispatcher_register_handler(dispatcher,
> -                                RED_WORKER_MESSAGE_CURSOR_CONNECT,
> -                                handle_dev_cursor_connect,
> -                                sizeof(RedWorkerMessageCursorConnect
> ),
> -                                false);
> -    dispatcher_register_handler(dispatcher,
> -                                RED_WORKER_MESSAGE_CURSOR_DISCONNECT
> ,
> -                                handle_dev_cursor_disconnect,
> -                                sizeof(RedWorkerMessageCursorDisconn
> ect),
> -                                true);
> -    dispatcher_register_handler(dispatcher,
> -                                RED_WORKER_MESSAGE_CURSOR_MIGRATE,
> -                                handle_dev_cursor_migrate,
> -                                sizeof(RedWorkerMessageCursorMigrate
> ),
> -                                false);
>      dispatcher_register_handler(dispatcher,
>                                  RED_WORKER_MESSAGE_UPDATE,
>                                  handle_dev_update,
> @@ -1259,9 +1188,7 @@ static GSourceFuncs worker_source_funcs = {
>      .dispatch = worker_source_dispatch,
>  };
>  
> -RedWorker* red_worker_new(QXLInstance *qxl,
> -                          const ClientCbs *client_cursor_cbs,
> -                          const ClientCbs *client_display_cbs)
> +RedWorker* red_worker_new(QXLInstance *qxl)
>  {
>      QXLDevInitInfo init_info;
>      RedWorker *worker;
> @@ -1317,21 +1244,31 @@ RedWorker* red_worker_new(QXLInstance *qxl,
>      worker->event_timeout = INF_EVENT_WAIT;
>  
>      worker->cursor_channel = cursor_channel_new(reds, qxl->id,
> -                                                &worker->core);
> +                                                &worker->core,
> dispatcher);
>      channel = RED_CHANNEL(worker->cursor_channel);
>      red_channel_init_stat_node(channel, &worker->stat,
> "cursor_channel");
> -    red_channel_register_client_cbs(channel, client_cursor_cbs);
> -    g_object_set_data(G_OBJECT(channel), "dispatcher", dispatcher);
> +
> +    ClientCbs client_cursor_cbs = { NULL, };
> +    client_cursor_cbs.connect = (channel_client_connect_proc)
> cursor_channel_connect;
> +    client_cursor_cbs.disconnect = NULL;
> +    client_cursor_cbs.migrate = cursor_channel_client_migrate;
> +    red_channel_register_client_cbs(channel, &client_cursor_cbs);
>  
>      // TODO: handle seamless migration. Temp, setting migrate to
> FALSE
> -    worker->display_channel = display_channel_new(reds, qxl,
> &worker->core, FALSE,
> +    worker->display_channel = display_channel_new(reds, qxl,
> &worker->core, dispatcher,
> +                                                  FALSE,
>                                                    reds_get_streaming
> _video(reds),
>                                                    reds_get_video_cod
> ecs(reds),
>                                                    init_info.n_surfac
> es);
>      channel = RED_CHANNEL(worker->display_channel);
>      red_channel_init_stat_node(channel, &worker->stat,
> "display_channel");
> -    red_channel_register_client_cbs(channel, client_display_cbs);
> -    g_object_set_data(G_OBJECT(channel), "dispatcher", dispatcher);
> +    g_object_set_data(G_OBJECT(channel), "worker", worker);
> +
> +    ClientCbs client_display_cbs = { NULL, };
> +    client_display_cbs.connect = handle_dev_display_connect;
> +    client_display_cbs.disconnect = handle_dev_display_disconnect;
> +    client_display_cbs.migrate = handle_dev_display_migrate;
> +    red_channel_register_client_cbs(channel, &client_display_cbs);
>  
>      return worker;
>  }
> diff --git a/server/red-worker.h b/server/red-worker.h
> index 767329da..3d2841ba 100644
> --- a/server/red-worker.h
> +++ b/server/red-worker.h
> @@ -28,9 +28,7 @@
>  
>  typedef struct RedWorker RedWorker;
>  
> -RedWorker* red_worker_new(QXLInstance *qxl,
> -                          const ClientCbs *client_cursor_cbs,
> -                          const ClientCbs *client_display_cbs);
> +RedWorker* red_worker_new(QXLInstance *qxl);
>  bool       red_worker_run(RedWorker *worker);
>  void red_worker_free(RedWorker *worker);
>  
> @@ -51,14 +49,14 @@ enum {
>      RED_WORKER_MESSAGE_OOM,
>      RED_WORKER_MESSAGE_READY, /* unused */
>  
> -    RED_WORKER_MESSAGE_DISPLAY_CONNECT,
> -    RED_WORKER_MESSAGE_DISPLAY_DISCONNECT,
> -    RED_WORKER_MESSAGE_DISPLAY_MIGRATE,
> +    RED_WORKER_MESSAGE_DISPLAY_CONNECT_DEPRECATED,
> +    RED_WORKER_MESSAGE_DISPLAY_DISCONNECT_DEPRECATED,
> +    RED_WORKER_MESSAGE_DISPLAY_MIGRATE_DEPRECATED,
>      RED_WORKER_MESSAGE_START,
>      RED_WORKER_MESSAGE_STOP,
> -    RED_WORKER_MESSAGE_CURSOR_CONNECT,
> -    RED_WORKER_MESSAGE_CURSOR_DISCONNECT,
> -    RED_WORKER_MESSAGE_CURSOR_MIGRATE,
> +    RED_WORKER_MESSAGE_CURSOR_CONNECT_DEPRECATED,
> +    RED_WORKER_MESSAGE_CURSOR_DISCONNECT_DEPRECATED,
> +    RED_WORKER_MESSAGE_CURSOR_MIGRATE_DEPRECATED,
>      RED_WORKER_MESSAGE_SET_COMPRESSION,
>      RED_WORKER_MESSAGE_SET_STREAMING_VIDEO,
>      RED_WORKER_MESSAGE_SET_MOUSE_MODE,
> @@ -97,36 +95,6 @@ enum {
>      RED_WORKER_MESSAGE_COUNT // LAST
>  };
>  
> -typedef struct RedWorkerMessageDisplayConnect {
> -    RedClient * client;
> -    RedStream * stream;
> -    RedChannelCapabilities caps;   // red_worker should reset
> -    int migration;
> -} RedWorkerMessageDisplayConnect;
> -
> -typedef struct RedWorkerMessageDisplayDisconnect {
> -    RedChannelClient *rcc;
> -} RedWorkerMessageDisplayDisconnect;
> -
> -typedef struct RedWorkerMessageDisplayMigrate {
> -    RedChannelClient *rcc;
> -} RedWorkerMessageDisplayMigrate;
> -
> -typedef struct RedWorkerMessageCursorConnect {
> -    RedClient *client;
> -    RedStream *stream;
> -    int migration;
> -    RedChannelCapabilities caps;   // red_worker should reset
> -} RedWorkerMessageCursorConnect;
> -
> -typedef struct RedWorkerMessageCursorDisconnect {
> -    RedChannelClient *rcc;
> -} RedWorkerMessageCursorDisconnect;
> -
> -typedef struct RedWorkerMessageCursorMigrate {
> -    RedChannelClient *rcc;
> -} RedWorkerMessageCursorMigrate;
> -
>  typedef struct RedWorkerMessageUpdate {
>      uint32_t surface_id;
>      QXLRect * qxl_area;


Reviewed-by: Jonathon Jongsma <jjongsma at redhat.com>




More information about the Spice-devel mailing list