[Spice-devel] [PATCH spice-server 3/4] Move Cursor and Display client cbs to channel file

Christophe Fergeau cfergeau at redhat.com
Wed Aug 30 06:57:52 UTC 2017


Yup, had a very similar patch locally, which I never tested/sent /o\
Should we change the naming of the methods? (red_qxl_xxx)

Acked-by: Christophe Fergeau <cfergeau at redhat.com>


On Tue, Aug 29, 2017 at 05:28:08PM -0500, Jonathon Jongsma wrote:
> Since we can retrieve the dispatcher from the channel's qxl instance, we
> can move the client callbacks into the channel implementation rather
> than having it within the qxl implementation.
> 
> Signed-off-by: Jonathon Jongsma <jjongsma at redhat.com>
> ---
>  server/cursor-channel.c  |  75 +++++++++++++++++++++++++
>  server/display-channel.c |  73 ++++++++++++++++++++++++
>  server/red-qxl.c         | 141 +----------------------------------------------
>  server/red-worker.c      |   6 +-
>  server/red-worker.h      |   4 +-
>  5 files changed, 151 insertions(+), 148 deletions(-)
> 
> diff --git a/server/cursor-channel.c b/server/cursor-channel.c
> index 63d6886d9..02a9fe3a1 100644
> --- a/server/cursor-channel.c
> +++ b/server/cursor-channel.c
> @@ -436,12 +436,87 @@ cursor_channel_finalize(GObject *object)
>      G_OBJECT_CLASS(cursor_channel_parent_class)->finalize(object);
>  }
>  
> +static void red_qxl_set_cursor_peer(RedChannel *channel, RedClient *client, RedsStream *stream,
> +                                    int migration,
> +                                    RedChannelCapabilities *caps)
> +{
> +    RedWorkerMessageCursorConnect payload = {0,};
> +    QXLInstance *qxl = common_graphics_channel_get_qxl(COMMON_GRAPHICS_CHANNEL(channel));
> +    Dispatcher *dispatcher = red_qxl_get_dispatcher(qxl);
> +    spice_printerr("");
> +    payload.client = 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;
> +    QXLInstance *qxl;
> +    Dispatcher *dispatcher;
> +    RedChannel *channel = red_channel_client_get_channel(rcc);
> +
> +    if (!channel) {
> +        return;
> +    }
> +
> +    qxl = common_graphics_channel_get_qxl(COMMON_GRAPHICS_CHANNEL(channel));
> +    dispatcher = red_qxl_get_dispatcher(qxl);
> +    spice_printerr("");
> +    payload.rcc = rcc;
> +
> +    dispatcher_send_message(dispatcher,
> +                            RED_WORKER_MESSAGE_CURSOR_DISCONNECT,
> +                            &payload);
> +}
> +
> +static void red_qxl_cursor_migrate(RedChannelClient *rcc)
> +{
> +    RedWorkerMessageCursorMigrate payload;
> +    QXLInstance *qxl;
> +    Dispatcher *dispatcher;
> +    RedChannel *channel = red_channel_client_get_channel(rcc);
> +    uint32_t type, id;
> +
> +    if (!channel) {
> +        return;
> +    }
> +    g_object_get(channel, "channel-type", &type, "id", &id, NULL);
> +    spice_printerr("channel type %u id %u", type, id);
> +    qxl = common_graphics_channel_get_qxl(COMMON_GRAPHICS_CHANNEL(channel));
> +    dispatcher = red_qxl_get_dispatcher(qxl);
> +    payload.rcc = rcc;
> +    dispatcher_send_message(dispatcher,
> +                            RED_WORKER_MESSAGE_CURSOR_MIGRATE,
> +                            &payload);
> +}
> +
> +static void
> +cursor_channel_constructed(GObject *object)
> +{
> +    RedChannel *channel = RED_CHANNEL(object);
> +    ClientCbs client_cbs = { NULL, };
> +
> +    G_OBJECT_CLASS(cursor_channel_parent_class)->constructed(object);
> +
> +    client_cbs.connect = red_qxl_set_cursor_peer;
> +    client_cbs.disconnect = red_qxl_disconnect_cursor_peer;
> +    client_cbs.migrate = red_qxl_cursor_migrate;
> +    red_channel_register_client_cbs(channel, &client_cbs, NULL);
> +}
> +
>  static void
>  cursor_channel_class_init(CursorChannelClass *klass)
>  {
>      GObjectClass *object_class = G_OBJECT_CLASS(klass);
>      RedChannelClass *channel_class = RED_CHANNEL_CLASS(klass);
>  
> +    object_class->constructed = cursor_channel_constructed;
>      object_class->finalize = cursor_channel_finalize;
>  
>      channel_class->parser = spice_get_client_channel_parser(SPICE_CHANNEL_CURSOR, NULL);
> diff --git a/server/display-channel.c b/server/display-channel.c
> index 792fbd25b..cccf1dea9 100644
> --- a/server/display-channel.c
> +++ b/server/display-channel.c
> @@ -2291,11 +2291,78 @@ display_channel_init(DisplayChannel *self)
>      self->priv->image_surfaces.ops = &image_surfaces_ops;
>  }
>  
> +static void red_qxl_set_display_peer(RedChannel *channel, RedClient *client,
> +                                     RedsStream *stream, int migration,
> +                                     RedChannelCapabilities *caps)
> +{
> +    RedWorkerMessageDisplayConnect payload = {0,};
> +    QXLInstance *qxl = common_graphics_channel_get_qxl(COMMON_GRAPHICS_CHANNEL(channel));
> +    Dispatcher *dispatcher = red_qxl_get_dispatcher(qxl);
> +
> +    spice_debug("%s", "");
> +    payload.client = 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;
> +    QXLInstance *qxl;
> +    Dispatcher *dispatcher;
> +    RedChannel *channel = red_channel_client_get_channel(rcc);
> +
> +    if (!channel) {
> +        return;
> +    }
> +
> +    qxl = common_graphics_channel_get_qxl(COMMON_GRAPHICS_CHANNEL(channel));
> +    dispatcher = red_qxl_get_dispatcher(qxl);
> +
> +    spice_printerr("");
> +    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;
> +    QXLInstance *qxl;
> +    Dispatcher *dispatcher;
> +    RedChannel *channel = red_channel_client_get_channel(rcc);
> +    uint32_t type, id;
> +
> +    if (!channel) {
> +        return;
> +    }
> +    g_object_get(channel, "channel-type", &type, "id", &id,
> +                 NULL);
> +
> +    qxl = common_graphics_channel_get_qxl(COMMON_GRAPHICS_CHANNEL(channel));
> +    dispatcher = red_qxl_get_dispatcher(qxl);
> +    spice_printerr("channel type %u id %u", type, id);
> +    payload.rcc = rcc;
> +    dispatcher_send_message(dispatcher,
> +                            RED_WORKER_MESSAGE_DISPLAY_MIGRATE,
> +                            &payload);
> +}
> +
>  static void
>  display_channel_constructed(GObject *object)
>  {
>      DisplayChannel *self = DISPLAY_CHANNEL(object);
>      RedChannel *channel = RED_CHANNEL(self);
> +    ClientCbs client_cbs = { NULL, };
>  
>      G_OBJECT_CLASS(display_channel_parent_class)->constructed(object);
>  
> @@ -2322,6 +2389,12 @@ display_channel_constructed(GObject *object)
>      red_channel_set_cap(channel, SPICE_DISPLAY_CAP_PREF_COMPRESSION);
>      red_channel_set_cap(channel, SPICE_DISPLAY_CAP_PREF_VIDEO_CODEC_TYPE);
>      red_channel_set_cap(channel, SPICE_DISPLAY_CAP_STREAM_REPORT);
> +
> +    client_cbs.connect = red_qxl_set_display_peer;
> +    client_cbs.disconnect = red_qxl_disconnect_display_peer;
> +    client_cbs.migrate = red_qxl_display_migrate;
> +
> +    red_channel_register_client_cbs(channel, &client_cbs, NULL);
>  }
>  
>  void display_channel_process_surface_cmd(DisplayChannel *display,
> diff --git a/server/red-qxl.c b/server/red-qxl.c
> index 4a8f523b7..03a538e1d 100644
> --- a/server/red-qxl.c
> +++ b/server/red-qxl.c
> @@ -37,7 +37,6 @@
>  #include "dispatcher.h"
>  #include "red-parse-qxl.h"
>  #include "red-channel-client.h"
> -#include "common-graphics-channel.h"
>  
>  #include "red-qxl.h"
>  
> @@ -75,132 +74,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,
> -                                     RedsStream *stream, int migration,
> -                                     RedChannelCapabilities *caps)
> -{
> -    RedWorkerMessageDisplayConnect payload = {0,};
> -    QXLInstance *qxl = common_graphics_channel_get_qxl(COMMON_GRAPHICS_CHANNEL(channel));
> -    Dispatcher *dispatcher = red_qxl_get_dispatcher(qxl);
> -
> -    spice_debug("%s", "");
> -    payload.client = 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;
> -    QXLInstance *qxl;
> -    Dispatcher *dispatcher;
> -    RedChannel *channel = red_channel_client_get_channel(rcc);
> -
> -    if (!channel) {
> -        return;
> -    }
> -
> -    qxl = common_graphics_channel_get_qxl(COMMON_GRAPHICS_CHANNEL(channel));
> -    dispatcher = red_qxl_get_dispatcher(qxl);
> -
> -    spice_printerr("");
> -    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;
> -    QXLInstance *qxl;
> -    Dispatcher *dispatcher;
> -    RedChannel *channel = red_channel_client_get_channel(rcc);
> -    uint32_t type, id;
> -
> -    if (!channel) {
> -        return;
> -    }
> -    g_object_get(channel, "channel-type", &type, "id", &id,
> -                 NULL);
> -
> -    qxl = common_graphics_channel_get_qxl(COMMON_GRAPHICS_CHANNEL(channel));
> -    dispatcher = red_qxl_get_dispatcher(qxl);
> -    spice_printerr("channel type %u id %u", type, id);
> -    payload.rcc = rcc;
> -    dispatcher_send_message(dispatcher,
> -                            RED_WORKER_MESSAGE_DISPLAY_MIGRATE,
> -                            &payload);
> -}
> -
> -static void red_qxl_set_cursor_peer(RedChannel *channel, RedClient *client, RedsStream *stream,
> -                                    int migration,
> -                                    RedChannelCapabilities *caps)
> -{
> -    RedWorkerMessageCursorConnect payload = {0,};
> -    QXLInstance *qxl = common_graphics_channel_get_qxl(COMMON_GRAPHICS_CHANNEL(channel));
> -    Dispatcher *dispatcher = red_qxl_get_dispatcher(qxl);
> -    spice_printerr("");
> -    payload.client = 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;
> -    QXLInstance *qxl;
> -    Dispatcher *dispatcher;
> -    RedChannel *channel = red_channel_client_get_channel(rcc);
> -
> -    if (!channel) {
> -        return;
> -    }
> -
> -    qxl = common_graphics_channel_get_qxl(COMMON_GRAPHICS_CHANNEL(channel));
> -    dispatcher = red_qxl_get_dispatcher(qxl);
> -    spice_printerr("");
> -    payload.rcc = rcc;
> -
> -    dispatcher_send_message(dispatcher,
> -                            RED_WORKER_MESSAGE_CURSOR_DISCONNECT,
> -                            &payload);
> -}
> -
> -static void red_qxl_cursor_migrate(RedChannelClient *rcc)
> -{
> -    RedWorkerMessageCursorMigrate payload;
> -    QXLInstance *qxl;
> -    Dispatcher *dispatcher;
> -    RedChannel *channel = red_channel_client_get_channel(rcc);
> -    uint32_t type, id;
> -
> -    if (!channel) {
> -        return;
> -    }
> -    g_object_get(channel, "channel-type", &type, "id", &id, NULL);
> -    spice_printerr("channel type %u id %u", type, id);
> -    qxl = common_graphics_channel_get_qxl(COMMON_GRAPHICS_CHANNEL(channel));
> -    dispatcher = red_qxl_get_dispatcher(qxl);
> -    payload.rcc = 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)
> @@ -965,8 +838,6 @@ void red_qxl_gl_draw_async_complete(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);
>  
> @@ -998,17 +869,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-worker.c b/server/red-worker.c
> index c43bb1244..f1225d2e2 100644
> --- a/server/red-worker.c
> +++ b/server/red-worker.c
> @@ -1286,9 +1286,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;
> @@ -1344,7 +1342,6 @@ RedWorker* red_worker_new(QXLInstance *qxl,
>      worker->cursor_channel = cursor_channel_new(reds, qxl, &worker->core);
>      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, NULL);
>      reds_register_channel(reds, channel);
>  
>      // TODO: handle seemless migration. Temp, setting migrate to FALSE
> @@ -1354,7 +1351,6 @@ RedWorker* red_worker_new(QXLInstance *qxl,
>                                                    init_info.n_surfaces);
>      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, NULL);
>      reds_register_channel(reds, channel);
>  
>      return worker;
> diff --git a/server/red-worker.h b/server/red-worker.h
> index 8ec28f144..b9677a559 100644
> --- a/server/red-worker.h
> +++ b/server/red-worker.h
> @@ -25,9 +25,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);
>  
> -- 
> 2.13.3
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel


More information about the Spice-devel mailing list