>
> For the Display Channel and the Cursor channel, move the call to
> reds_register_channel() to the _constructed() vfunc rather than calling
> it explicitly from RedWorker. This matches what other channels do.
I think this was different for these channels as they where created
in a different thread.
Is there a channel not registered to "reds"? I think not.
Would make it sense to move the registration to RedChannel?
However I would implement it with a Initiable interface, registering
a channel while constructing it does not seem a good idea.
Also would be good to update docs/spice_threading_model.txt documenting
that creating/shutdown should be handled in the main thread.
> ---
> server/cursor-channel.c | 12 ++++++++++++
> server/display-channel.c | 2 ++
> server/red-worker.c | 2 --
> 3 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/server/cursor-channel.c b/server/cursor-channel.c
> index 9dd69fa25..4220084f5 100644
> --- a/server/cursor-channel.c
> +++ b/server/cursor-channel.c
> @@ -366,12 +366,24 @@ cursor_channel_finalize(GObject *object)
> G_OBJECT_CLASS(cursor_channel_parent_class)->finalize(object);
> }
>
> +static void
> +cursor_channel_constructed(GObject *object)
> +{
> + RedChannel *red_channel = RED_CHANNEL(object);
> + RedsState *reds = red_channel_get_server(red_channel);
> +
> + G_OBJECT_CLASS(cursor_channel_parent_class)->constructed(object);
> +
> + reds_register_channel(reds, red_channel);
> +}
> +
> 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 e68ed10f8..6684135eb 100644
> --- a/server/display-channel.c
> +++ b/server/display-channel.c
> @@ -2304,6 +2304,8 @@ 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);
> +
> + reds_register_channel(reds, channel);
> }
>
> void display_channel_process_surface_cmd(DisplayChannel *display,
> diff --git a/server/red-worker.c b/server/red-worker.c
> index c74ae8886..3cb12b9c2 100644
> --- a/server/red-worker.c
> +++ b/server/red-worker.c
> @@ -1322,7 +1322,6 @@ RedWorker* red_worker_new(QXLInstance *qxl,
> 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);
> - reds_register_channel(reds, channel);
>
> // TODO: handle seamless migration. Temp, setting migrate to FALSE
> worker->display_channel = display_channel_new(reds, qxl, &worker->core,
> FALSE,
> @@ -1333,7 +1332,6 @@ RedWorker* red_worker_new(QXLInstance *qxl,
> 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);
> - reds_register_channel(reds, channel);
>
> return worker;
> }
Otherwise patch seems good, but I didn't test it.
In particular I think CursorChannel is also registered in the
StreamDevice so will be registered twice.
There should be a test for double registration.
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel