[Spice-devel] [PATCH spice-server 09/10] Make channel client callbacks virtual functions

Frediano Ziglio fziglio at redhat.com
Wed Mar 20 09:59:18 UTC 2019


From: Jonathon Jongsma <jjongsma at redhat.com>

Rather than having an API to register client callbacks for each channel
type, make them vfuncs.

Since the client callbacks are registered identically for each channel
of the same type, it doesn't make sense for to require these to be
registered separately for each object.  It's cleaner to have these be
per-class properties, so they've been converted to virtual functions.
---
 server/cursor-channel.c     |  4 ++++
 server/display-channel.c    |  5 +++++
 server/inputs-channel.c     |  9 ++++----
 server/main-channel.c       |  7 +++----
 server/red-channel.c        | 42 ++++++++++++++-----------------------
 server/red-channel.h        | 19 +++++++----------
 server/red-stream-device.c  |  5 -----
 server/red-worker.c         | 12 -----------
 server/smartcard.c          |  6 ++----
 server/sound.c              | 18 +++++++---------
 server/spicevmc.c           |  7 +++----
 server/stream-channel.c     |  5 +----
 server/tests/test-channel.c | 13 +-----------
 13 files changed, 55 insertions(+), 97 deletions(-)

diff --git a/server/cursor-channel.c b/server/cursor-channel.c
index e8af01b0..d1294322 100644
--- a/server/cursor-channel.c
+++ b/server/cursor-channel.c
@@ -392,6 +392,10 @@ cursor_channel_class_init(CursorChannelClass *klass)
     channel_class->handle_message = red_channel_client_handle_message;
 
     channel_class->send_item = cursor_channel_send_item;
+
+    // client callbacks
+    channel_class->connect = (channel_client_connect_proc) cursor_channel_connect;
+    channel_class->migrate = cursor_channel_client_migrate;
 }
 
 static void
diff --git a/server/display-channel.c b/server/display-channel.c
index a322045e..fde4d5c3 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -2500,6 +2500,11 @@ display_channel_class_init(DisplayChannelClass *klass)
     channel_class->handle_migrate_data = handle_migrate_data;
     channel_class->handle_migrate_data_get_serial = handle_migrate_data_get_serial;
 
+    // client callbacks
+    channel_class->connect = display_channel_connect;
+    channel_class->disconnect = display_channel_disconnect;
+    channel_class->migrate = display_channel_migrate;
+
     g_object_class_install_property(object_class,
                                     PROP_N_SURFACES,
                                     g_param_spec_uint("n-surfaces",
diff --git a/server/inputs-channel.c b/server/inputs-channel.c
index a7df62e2..eff5421a 100644
--- a/server/inputs-channel.c
+++ b/server/inputs-channel.c
@@ -542,17 +542,12 @@ InputsChannel* inputs_channel_new(RedsState *reds)
 static void
 inputs_channel_constructed(GObject *object)
 {
-    ClientCbs client_cbs = { NULL, };
     InputsChannel *self = INPUTS_CHANNEL(object);
     RedsState *reds = red_channel_get_server(RED_CHANNEL(self));
     SpiceCoreInterfaceInternal *core = red_channel_get_core_interface(RED_CHANNEL(self));
 
     G_OBJECT_CLASS(inputs_channel_parent_class)->constructed(object);
 
-    client_cbs.connect = inputs_connect;
-    client_cbs.migrate = inputs_migrate;
-    red_channel_register_client_cbs(RED_CHANNEL(self), &client_cbs);
-
     red_channel_set_cap(RED_CHANNEL(self), SPICE_INPUTS_CAP_KEY_SCANCODE);
     reds_register_channel(reds, RED_CHANNEL(self));
 
@@ -596,6 +591,10 @@ inputs_channel_class_init(InputsChannelClass *klass)
     channel_class->send_item = inputs_channel_send_item;
     channel_class->handle_migrate_data = inputs_channel_handle_migrate_data;
     channel_class->handle_migrate_flush_mark = inputs_channel_handle_migrate_flush_mark;
+
+    // client callbacks
+    channel_class->connect = inputs_connect;
+    channel_class->migrate = inputs_migrate;
 }
 
 static SpiceKbdInstance* inputs_channel_get_keyboard(InputsChannel *inputs)
diff --git a/server/main-channel.c b/server/main-channel.c
index f866fb4a..a48e74be 100644
--- a/server/main-channel.c
+++ b/server/main-channel.c
@@ -264,15 +264,11 @@ static void
 main_channel_constructed(GObject *object)
 {
     MainChannel *self = MAIN_CHANNEL(object);
-    ClientCbs client_cbs = { NULL, };
 
     G_OBJECT_CLASS(main_channel_parent_class)->constructed(object);
 
     red_channel_set_cap(RED_CHANNEL(self), SPICE_MAIN_CAP_SEMI_SEAMLESS_MIGRATE);
     red_channel_set_cap(RED_CHANNEL(self), SPICE_MAIN_CAP_SEAMLESS_MIGRATE);
-
-    client_cbs.migrate = main_channel_client_migrate;
-    red_channel_register_client_cbs(RED_CHANNEL(self), &client_cbs);
 }
 
 static void
@@ -295,6 +291,9 @@ main_channel_class_init(MainChannelClass *klass)
     channel_class->send_item = main_channel_client_send_item;
     channel_class->handle_migrate_flush_mark = main_channel_handle_migrate_flush_mark;
     channel_class->handle_migrate_data = main_channel_handle_migrate_data;
+
+    // client callbacks
+    channel_class->migrate = main_channel_client_migrate;
 }
 
 static int main_channel_connect_semi_seamless(MainChannel *main_channel)
diff --git a/server/red-channel.c b/server/red-channel.c
index 8d05c971..3c0d126e 100644
--- a/server/red-channel.c
+++ b/server/red-channel.c
@@ -89,7 +89,6 @@ struct RedChannelPrivate
     RedChannelCapabilities local_caps;
     uint32_t migration_flags;
 
-    ClientCbs client_cbs;
     // TODO: when different channel_clients are in different threads
     // from Channel -> need to protect!
     pthread_t thread_id;
@@ -318,6 +317,10 @@ red_channel_class_init(RedChannelClass *klass)
                                | G_PARAM_READWRITE
                                | G_PARAM_CONSTRUCT_ONLY);
     g_object_class_install_property(object_class, PROP_DISPATCHER, spec);
+
+    klass->connect = red_channel_client_default_connect;
+    klass->disconnect = red_channel_client_default_disconnect;
+    klass->migrate = red_channel_client_default_migrate;
 }
 
 static void
@@ -327,10 +330,6 @@ red_channel_init(RedChannel *self)
 
     red_channel_set_common_cap(self, SPICE_COMMON_CAP_MINI_HEADER);
     self->priv->thread_id = pthread_self();
-
-    self->priv->client_cbs.connect = red_channel_client_default_connect;
-    self->priv->client_cbs.disconnect = red_channel_client_default_disconnect;
-    self->priv->client_cbs.migrate = red_channel_client_default_migrate;
 }
 
 // utility to avoid possible invalid function cast
@@ -396,20 +395,6 @@ const RedStatNode *red_channel_get_stat_node(RedChannel *channel)
     return &channel->priv->stat;
 }
 
-void red_channel_register_client_cbs(RedChannel *channel, const ClientCbs *client_cbs)
-{
-    spice_assert(client_cbs->connect || channel->priv->type == SPICE_CHANNEL_MAIN);
-    channel->priv->client_cbs.connect = client_cbs->connect;
-
-    if (client_cbs->disconnect) {
-        channel->priv->client_cbs.disconnect = client_cbs->disconnect;
-    }
-
-    if (client_cbs->migrate) {
-        channel->priv->client_cbs.migrate = client_cbs->migrate;
-    }
-}
-
 static void add_capability(uint32_t **caps, int *num_caps, uint32_t cap)
 {
     int nbefore, n;
@@ -526,9 +511,9 @@ static void handle_dispatcher_connect(void *opaque, void *payload)
 {
     RedMessageConnect *msg = payload;
     RedChannel *channel = msg->channel;
+    RedChannelClass *klass = RED_CHANNEL_GET_CLASS(channel);
 
-    channel->priv->client_cbs.connect(channel, msg->client, msg->stream,
-                                      msg->migration, &msg->caps);
+    klass->connect(channel, msg->client, msg->stream, msg->migration, &msg->caps);
     g_object_unref(msg->client);
     red_channel_capabilities_reset(&msg->caps);
 }
@@ -539,7 +524,8 @@ void red_channel_connect(RedChannel *channel, RedClient *client,
 {
     if (channel->priv->dispatcher == NULL ||
         pthread_equal(pthread_self(), channel->priv->thread_id)) {
-        channel->priv->client_cbs.connect(channel, client, stream, migration, caps);
+        RedChannelClass *klass = RED_CHANNEL_GET_CLASS(channel);
+        klass->connect(channel, client, stream, migration, caps);
         return;
     }
 
@@ -762,8 +748,9 @@ static void handle_dispatcher_migrate(void *opaque, void *payload)
 {
     RedMessageMigrate *msg = payload;
     RedChannel *channel = red_channel_client_get_channel(msg->rcc);
+    RedChannelClass *klass = RED_CHANNEL_GET_CLASS(channel);
 
-    channel->priv->client_cbs.migrate(msg->rcc);
+    klass->migrate(msg->rcc);
     g_object_unref(msg->rcc);
 }
 
@@ -771,7 +758,8 @@ void red_channel_migrate_client(RedChannel *channel, RedChannelClient *rcc)
 {
     if (channel->priv->dispatcher == NULL ||
         pthread_equal(pthread_self(), channel->priv->thread_id)) {
-        channel->priv->client_cbs.migrate(rcc);
+        RedChannelClass *klass = RED_CHANNEL_GET_CLASS(channel);
+        klass->migrate(rcc);
         return;
     }
 
@@ -784,15 +772,17 @@ static void handle_dispatcher_disconnect(void *opaque, void *payload)
 {
     RedMessageDisconnect *msg = payload;
     RedChannel *channel = red_channel_client_get_channel(msg->rcc);
+    RedChannelClass *klass = RED_CHANNEL_GET_CLASS(channel);
 
-    channel->priv->client_cbs.disconnect(msg->rcc);
+    klass->disconnect(msg->rcc);
 }
 
 void red_channel_disconnect_client(RedChannel *channel, RedChannelClient *rcc)
 {
     if (channel->priv->dispatcher == NULL ||
         pthread_equal(pthread_self(), channel->priv->thread_id)) {
-        channel->priv->client_cbs.disconnect(rcc);
+        RedChannelClass *klass = RED_CHANNEL_GET_CLASS(channel);
+        klass->disconnect(rcc);
         return;
     }
 
diff --git a/server/red-channel.h b/server/red-channel.h
index bb3a95e8..4bfd81ee 100644
--- a/server/red-channel.h
+++ b/server/red-channel.h
@@ -63,16 +63,6 @@ typedef void (*channel_client_disconnect_proc)(RedChannelClient *base);
 typedef void (*channel_client_migrate_proc)(RedChannelClient *base);
 
 
-/*
- * callbacks that are triggered from client events.
- * They should be called from the thread that handles the RedClient
- */
-typedef struct {
-    channel_client_connect_proc connect;
-    channel_client_disconnect_proc disconnect;
-    channel_client_migrate_proc migrate;
-} ClientCbs;
-
 static inline gboolean test_capability(const uint32_t *caps, int num_caps, uint32_t cap)
 {
     return VD_AGENT_HAS_CAPABILITY(caps, num_caps, cap);
@@ -107,6 +97,14 @@ struct RedChannelClass
     channel_handle_migrate_flush_mark_proc handle_migrate_flush_mark;
     channel_handle_migrate_data_proc handle_migrate_data;
     channel_handle_migrate_data_get_serial_proc handle_migrate_data_get_serial;
+
+    /*
+     * callbacks that are triggered from client events.
+     * They should be called from the thread that handles the RedClient
+     */
+    channel_client_connect_proc connect;
+    channel_client_disconnect_proc disconnect;
+    channel_client_migrate_proc migrate;
 };
 
 #define FOREACH_CLIENT(_channel, _data) \
@@ -122,7 +120,6 @@ void red_channel_remove_client(RedChannel *channel, RedChannelClient *rcc);
 
 void red_channel_init_stat_node(RedChannel *channel, const RedStatNode *parent, const char *name);
 
-void red_channel_register_client_cbs(RedChannel *channel, const ClientCbs *client_cbs);
 // caps are freed when the channel is destroyed
 void red_channel_set_common_cap(RedChannel *channel, uint32_t cap);
 void red_channel_set_cap(RedChannel *channel, uint32_t cap);
diff --git a/server/red-stream-device.c b/server/red-stream-device.c
index cf5a6e9a..b4c32f71 100644
--- a/server/red-stream-device.c
+++ b/server/red-stream-device.c
@@ -695,12 +695,7 @@ stream_device_create_channel(StreamDevice *dev)
     g_return_if_fail(id >= 0);
 
     StreamChannel *stream_channel = stream_channel_new(reds, id);
-
     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;
-    red_channel_register_client_cbs(RED_CHANNEL(cursor_channel), &client_cbs);
 
     dev->stream_channel = stream_channel;
     dev->cursor_channel = cursor_channel;
diff --git a/server/red-worker.c b/server/red-worker.c
index b742925a..a459d711 100644
--- a/server/red-worker.c
+++ b/server/red-worker.c
@@ -1144,12 +1144,6 @@ RedWorker* red_worker_new(QXLInstance *qxl)
     channel = RED_CHANNEL(worker->cursor_channel);
     red_channel_init_stat_node(channel, &worker->stat, "cursor_channel");
 
-    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, dispatcher,
                                                   FALSE,
@@ -1159,12 +1153,6 @@ RedWorker* red_worker_new(QXLInstance *qxl)
     channel = RED_CHANNEL(worker->display_channel);
     red_channel_init_stat_node(channel, &worker->stat, "display_channel");
 
-    ClientCbs client_display_cbs = { NULL, };
-    client_display_cbs.connect = display_channel_connect;
-    client_display_cbs.disconnect = display_channel_disconnect;
-    client_display_cbs.migrate = display_channel_migrate;
-    red_channel_register_client_cbs(channel, &client_display_cbs);
-
     return worker;
 }
 
diff --git a/server/smartcard.c b/server/smartcard.c
index 9ccfbc6b..e3e746ca 100644
--- a/server/smartcard.c
+++ b/server/smartcard.c
@@ -535,13 +535,9 @@ red_smartcard_channel_constructed(GObject *object)
 {
     RedSmartcardChannel *self = RED_SMARTCARD_CHANNEL(object);
     RedsState *reds = red_channel_get_server(RED_CHANNEL(self));
-    ClientCbs client_cbs = { NULL, };
 
     G_OBJECT_CLASS(red_smartcard_channel_parent_class)->constructed(object);
 
-    client_cbs.connect = smartcard_connect_client;
-    red_channel_register_client_cbs(RED_CHANNEL(self), &client_cbs);
-
     reds_register_channel(reds, RED_CHANNEL(self));
 }
 
@@ -559,6 +555,8 @@ red_smartcard_channel_class_init(RedSmartcardChannelClass *klass)
     channel_class->handle_migrate_flush_mark = smartcard_channel_client_handle_migrate_flush_mark;
     channel_class->handle_migrate_data = smartcard_channel_client_handle_migrate_data;
 
+    // client callbacks
+    channel_class->connect = smartcard_connect_client;
 }
 
 static void smartcard_init(RedsState *reds)
diff --git a/server/sound.c b/server/sound.c
index 44b27dec..167f68c5 100644
--- a/server/sound.c
+++ b/server/sound.c
@@ -1377,16 +1377,11 @@ playback_channel_init(PlaybackChannel *self)
 static void
 playback_channel_constructed(GObject *object)
 {
-    ClientCbs client_cbs = { NULL, };
     SndChannel *self = SND_CHANNEL(object);
     RedsState *reds = red_channel_get_server(RED_CHANNEL(self));
 
     G_OBJECT_CLASS(playback_channel_parent_class)->constructed(object);
 
-    client_cbs.connect = snd_set_playback_peer;
-    client_cbs.migrate = snd_migrate_channel_client;
-    red_channel_register_client_cbs(RED_CHANNEL(self), &client_cbs);
-
     if (snd_codec_is_capable(SPICE_AUDIO_DATA_MODE_CELT_0_5_1, SND_CODEC_ANY_FREQUENCY)) {
         red_channel_set_cap(RED_CHANNEL(self), SPICE_PLAYBACK_CAP_CELT_0_5_1);
     }
@@ -1407,6 +1402,10 @@ playback_channel_class_init(PlaybackChannelClass *klass)
     channel_class->parser = spice_get_client_channel_parser(SPICE_CHANNEL_PLAYBACK, NULL);
     channel_class->handle_message = red_channel_client_handle_message;
     channel_class->send_item = playback_channel_send_item;
+
+    // client callbacks
+    channel_class->connect = snd_set_playback_peer;
+    channel_class->migrate = snd_migrate_channel_client;
 }
 
 void snd_attach_playback(RedsState *reds, SpicePlaybackInstance *sin)
@@ -1427,16 +1426,11 @@ record_channel_init(RecordChannel *self)
 static void
 record_channel_constructed(GObject *object)
 {
-    ClientCbs client_cbs = { NULL, };
     SndChannel *self = SND_CHANNEL(object);
     RedsState *reds = red_channel_get_server(RED_CHANNEL(self));
 
     G_OBJECT_CLASS(record_channel_parent_class)->constructed(object);
 
-    client_cbs.connect = snd_set_record_peer;
-    client_cbs.migrate = snd_migrate_channel_client;
-    red_channel_register_client_cbs(RED_CHANNEL(self), &client_cbs);
-
     if (snd_codec_is_capable(SPICE_AUDIO_DATA_MODE_CELT_0_5_1, SND_CODEC_ANY_FREQUENCY)) {
         red_channel_set_cap(RED_CHANNEL(self), SPICE_RECORD_CAP_CELT_0_5_1);
     }
@@ -1457,6 +1451,10 @@ record_channel_class_init(RecordChannelClass *klass)
     channel_class->parser = spice_get_client_channel_parser(SPICE_CHANNEL_RECORD, NULL);
     channel_class->handle_message = record_channel_handle_message;
     channel_class->send_item = record_channel_send_item;
+
+    // client callbacks
+    channel_class->connect = snd_set_record_peer;
+    channel_class->migrate = snd_migrate_channel_client;
 }
 
 void snd_attach_record(RedsState *reds, SpiceRecordInstance *sin)
diff --git a/server/spicevmc.c b/server/spicevmc.c
index 51550c1a..d094166e 100644
--- a/server/spicevmc.c
+++ b/server/spicevmc.c
@@ -185,14 +185,10 @@ static void
 red_vmc_channel_constructed(GObject *object)
 {
     RedVmcChannel *self = RED_VMC_CHANNEL(object);
-    ClientCbs client_cbs = { NULL, };
     RedsState *reds = red_channel_get_server(RED_CHANNEL(self));
 
     G_OBJECT_CLASS(red_vmc_channel_parent_class)->constructed(object);
 
-    client_cbs.connect = spicevmc_connect;
-    red_channel_register_client_cbs(RED_CHANNEL(self), &client_cbs);
-
     red_channel_init_stat_node(RED_CHANNEL(self), NULL, "spicevmc");
     const RedStatNode *stat = red_channel_get_stat_node(RED_CHANNEL(self));
     stat_init_counter(&self->in_data, reds, stat, "in_data", TRUE);
@@ -723,6 +719,9 @@ red_vmc_channel_class_init(RedVmcChannelClass *klass)
     channel_class->send_item = spicevmc_red_channel_send_item;
     channel_class->handle_migrate_flush_mark = spicevmc_channel_client_handle_migrate_flush_mark;
     channel_class->handle_migrate_data = spicevmc_channel_client_handle_migrate_data;
+
+    // client callbacks
+    channel_class->connect = spicevmc_connect;
 }
 
 static void
diff --git a/server/stream-channel.c b/server/stream-channel.c
index c38ee043..ed844d6d 100644
--- a/server/stream-channel.c
+++ b/server/stream-channel.c
@@ -443,15 +443,11 @@ stream_channel_connect(RedChannel *red_channel, RedClient *red_client, RedStream
 static void
 stream_channel_constructed(GObject *object)
 {
-    ClientCbs client_cbs = { NULL, };
     RedChannel *red_channel = RED_CHANNEL(object);
     RedsState *reds = red_channel_get_server(red_channel);
 
     G_OBJECT_CLASS(stream_channel_parent_class)->constructed(object);
 
-    client_cbs.connect = stream_channel_connect;
-    red_channel_register_client_cbs(red_channel, &client_cbs);
-
     red_channel_set_cap(red_channel, SPICE_DISPLAY_CAP_MONITORS_CONFIG);
     red_channel_set_cap(red_channel, SPICE_DISPLAY_CAP_STREAM_REPORT);
 
@@ -470,6 +466,7 @@ stream_channel_class_init(StreamChannelClass *klass)
     channel_class->handle_message = handle_message;
 
     channel_class->send_item = stream_channel_send_item;
+    channel_class->connect = stream_channel_connect;
 }
 
 static void
diff --git a/server/tests/test-channel.c b/server/tests/test-channel.c
index 1c9148df..9700e31c 100644
--- a/server/tests/test-channel.c
+++ b/server/tests/test-channel.c
@@ -105,25 +105,14 @@ test_connect_client(RedChannel *channel, RedClient *client, RedStream *stream,
     }
 }
 
-static void
-red_test_channel_constructed(GObject *object)
-{
-    G_OBJECT_CLASS(red_test_channel_parent_class)->constructed(object);
-
-    ClientCbs client_cbs = { .connect = test_connect_client, };
-    red_channel_register_client_cbs(RED_CHANNEL(object), &client_cbs);
-}
-
 static void
 red_test_channel_class_init(RedTestChannelClass *klass)
 {
-    GObjectClass *object_class = G_OBJECT_CLASS(klass);
-    object_class->constructed = red_test_channel_constructed;
-
     RedChannelClass *channel_class = RED_CHANNEL_CLASS(klass);
     channel_class->parser = spice_get_client_channel_parser(SPICE_CHANNEL_PORT, NULL);
     channel_class->handle_message = red_channel_client_handle_message;
     channel_class->send_item = test_channel_send_item;
+    channel_class->connect = test_connect_client;
 }
 
 static uint8_t *
-- 
2.20.1



More information about the Spice-devel mailing list