From: Jonathon Jongsma <jjong...@redhat.com>

Previously, spicevmc_device_connect() created a channel, which then
internally created a device. Then we returned the internal device from
the channel to the caller. The channel essentially owned the device, but
it makes more sense for the device to own the channel, because a channel
can't really exist without the associated device.

So this refactory inverts things: spicevmc_device_connect() now creates
a char device and returns that directly. Internally, that device creates
an associated channel.
---
 server/spicevmc.c | 122 +++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 83 insertions(+), 39 deletions(-)

diff --git a/server/spicevmc.c b/server/spicevmc.c
index 0fcdc96..ba6e358 100644
--- a/server/spicevmc.c
+++ b/server/spicevmc.c
@@ -78,6 +78,7 @@ typedef struct RedCharDeviceSpiceVmcClass 
RedCharDeviceSpiceVmcClass;
 
 struct RedCharDeviceSpiceVmc {
     RedCharDevice parent;
+    uint8_t channel_type;
     RedVmcChannel *channel;
 };
 
@@ -89,7 +90,7 @@ struct RedCharDeviceSpiceVmcClass
 static GType red_char_device_spicevmc_get_type(void) G_GNUC_CONST;
 static RedCharDevice *red_char_device_spicevmc_new(SpiceCharDeviceInstance 
*sin,
                                                    RedsState *reds,
-                                                   void *opaque);
+                                                   uint8_t channel_type);
 
 G_DEFINE_TYPE(RedCharDeviceSpiceVmc, red_char_device_spicevmc, 
RED_TYPE_CHAR_DEVICE)
 
@@ -109,8 +110,7 @@ struct RedVmcChannel
     RedChannel parent;
 
     RedChannelClient *rcc;
-    RedCharDevice *chardev;
-    SpiceCharDeviceInstance *chardev_sin;
+    RedCharDevice *chardev; /* weak */
     RedVmcPipeItem *pipe_item;
     RedCharDeviceWriteBuffer *recv_from_client_buf;
     uint8_t port_opened;
@@ -180,7 +180,7 @@ G_DEFINE_TYPE(RedVmcChannelPort, red_vmc_channel_port, 
RED_TYPE_VMC_CHANNEL)
 
 enum {
     PROP0,
-    PROP_DEVICE_INSTANCE
+    PROP_CHAR_DEVICE
 };
 
 static void
@@ -193,8 +193,8 @@ red_vmc_channel_get_property(GObject *object,
 
     switch (property_id)
     {
-        case PROP_DEVICE_INSTANCE:
-            g_value_set_pointer(value, self->chardev_sin);
+        case PROP_CHAR_DEVICE:
+            g_value_set_object(value, self->chardev);
             break;
         default:
             G_OBJECT_WARN_INVALID_PROPERTY_ID(object, property_id, pspec);
@@ -211,8 +211,9 @@ red_vmc_channel_set_property(GObject *object,
 
     switch (property_id)
     {
-        case PROP_DEVICE_INSTANCE:
-            self->chardev_sin = g_value_get_pointer(value);
+        case PROP_CHAR_DEVICE:
+            /* don't take a ref */
+            self->chardev = g_value_get_object(value);
             break;
         default:
             G_OBJECT_WARN_INVALID_PROPERTY_ID(object, property_id, pspec);
@@ -241,8 +242,6 @@ red_vmc_channel_constructed(GObject *object)
 
     red_channel_init_outgoing_messages_window(RED_CHANNEL(self));
 
-    self->chardev = red_char_device_spicevmc_new(self->chardev_sin, reds, 
self);
-
     reds_register_channel(reds, RED_CHANNEL(self));
 }
 
@@ -265,7 +264,7 @@ red_vmc_channel_finalize(GObject *object)
 }
 
 static RedVmcChannel *red_vmc_channel_new(RedsState *reds, uint8_t 
channel_type,
-                                          SpiceCharDeviceInstance *sin)
+                                          RedCharDeviceSpiceVmc *chardev)
 {
     GType gtype = G_TYPE_NONE;
     static uint8_t id[256] = { 0, };
@@ -282,7 +281,9 @@ static RedVmcChannel *red_vmc_channel_new(RedsState *reds, 
uint8_t channel_type,
             break;
         default:
             g_error("Unsupported channel_type for red_vmc_channel_new(): %u", 
channel_type);
+            return NULL;
     }
+
     return g_object_new(gtype,
                         "spice-server", reds,
                         "core-interface", reds_get_core_interface(reds),
@@ -291,7 +292,7 @@ static RedVmcChannel *red_vmc_channel_new(RedsState *reds, 
uint8_t channel_type,
                         "handle-acks", FALSE,
                         "migration-flags",
                         (SPICE_MIGRATE_NEED_FLUSH | 
SPICE_MIGRATE_NEED_DATA_TRANSFER),
-                        "device-instance", sin,
+                        "char-device", chardev,
                         NULL);
 }
 
@@ -421,9 +422,11 @@ static void 
spicevmc_chardev_send_msg_to_client(RedPipeItem *msg,
 static void spicevmc_port_send_init(RedChannelClient *rcc)
 {
     RedVmcChannel *channel = 
RED_VMC_CHANNEL(red_channel_client_get_channel(rcc));
-    SpiceCharDeviceInstance *sin = channel->chardev_sin;
+    SpiceCharDeviceInstance *sin;
     RedPortInitPipeItem *item = spice_malloc(sizeof(RedPortInitPipeItem));
 
+    g_object_get(channel->chardev, "sin", &sin, NULL);
+
     red_pipe_item_init(&item->base, RED_PIPE_ITEM_TYPE_PORT_INIT);
     item->name = strdup(sin->portname);
     item->opened = channel->port_opened;
@@ -481,6 +484,7 @@ static int 
spicevmc_red_channel_client_config_socket(RedChannelClient *rcc)
 static void spicevmc_red_channel_client_on_disconnect(RedChannelClient *rcc)
 {
     RedVmcChannel *channel;
+    SpiceCharDeviceInstance *sin;
     SpiceCharDeviceInterface *sif;
     RedClient *client = red_channel_client_get_client(rcc);
 
@@ -493,13 +497,12 @@ static void 
spicevmc_red_channel_client_on_disconnect(RedChannelClient *rcc)
     /* partial message which wasn't pushed to device */
     red_char_device_write_buffer_release(channel->chardev, 
&channel->recv_from_client_buf);
 
-    if (channel->chardev) {
-        if (red_char_device_client_exists(channel->chardev, client)) {
-            red_char_device_client_remove(channel->chardev, client);
-        } else {
-            spice_printerr("client %p have already been removed from char dev 
%p",
-                           client, channel->chardev);
-        }
+    g_object_get(channel->chardev, "sin", &sin, NULL);
+    if (red_char_device_client_exists(channel->chardev, client)) {
+        red_char_device_client_remove(channel->chardev, client);
+    } else {
+        spice_printerr("client %p have already been removed from char dev %p",
+                       client, channel->chardev);
     }
 
     /* Don't destroy the rcc if it is already being destroyed, as then
@@ -508,9 +511,9 @@ static void 
spicevmc_red_channel_client_on_disconnect(RedChannelClient *rcc)
         red_channel_client_destroy(rcc);
 
     channel->rcc = NULL;
-    sif = spice_char_device_get_interface(channel->chardev_sin);
+    sif = spice_char_device_get_interface(sin);
     if (sif->state) {
-        sif->state(channel->chardev_sin, 0);
+        sif->state(sin, 0);
     }
 }
 
@@ -590,10 +593,12 @@ static int 
spicevmc_red_channel_client_handle_message_parsed(RedChannelClient *r
     /* NOTE: *msg free by free() (when cb to 
spicevmc_red_channel_release_msg_rcv_buf
      * with the compressed msg type) */
     RedVmcChannel *channel;
+    SpiceCharDeviceInstance *sin;
     SpiceCharDeviceInterface *sif;
 
     channel = RED_VMC_CHANNEL(red_channel_client_get_channel(rcc));
-    sif = spice_char_device_get_interface(channel->chardev_sin);
+    g_object_get(channel->chardev, "sin", &sin, NULL);
+    sif = spice_char_device_get_interface(sin);
 
     switch (type) {
     case SPICE_MSGC_SPICEVMC_DATA:
@@ -611,7 +616,7 @@ static int 
spicevmc_red_channel_client_handle_message_parsed(RedChannelClient *r
             return FALSE;
         }
         if (sif->base.minor_version >= 2 && sif->event != NULL)
-            sif->event(channel->chardev_sin, *(uint8_t*)msg);
+            sif->event(sin, *(uint8_t*)msg);
         break;
     default:
         return red_channel_client_handle_message(rcc, size, type, 
(uint8_t*)msg);
@@ -776,10 +781,11 @@ red_vmc_channel_class_init(RedVmcChannelClass *klass)
     channel_class->handle_migrate_data = 
spicevmc_channel_client_handle_migrate_data;
 
     g_object_class_install_property(object_class,
-                                    PROP_DEVICE_INSTANCE,
-                                    g_param_spec_pointer("device-instance",
-                                                         "device instance",
-                                                         "Device instance for 
this channel",
+                                    PROP_CHAR_DEVICE,
+                                    g_param_spec_object("char-device",
+                                                         "Char device",
+                                                         "Char device for this 
channel",
+                                                         
RED_TYPE_CHAR_DEVICE_SPICEVMC,
                                                          G_PARAM_READWRITE |
                                                          
G_PARAM_CONSTRUCT_ONLY |
                                                          
G_PARAM_STATIC_STRINGS));
@@ -820,8 +826,8 @@ static void spicevmc_connect(RedChannel *channel, RedClient 
*client,
     uint32_t type, id;
 
     vmc_channel = RED_VMC_CHANNEL(channel);
-    sin = vmc_channel->chardev_sin;
     g_object_get(channel, "channel-type", &type, "id", &id, NULL);
+    g_object_get(vmc_channel->chardev, "sin", &sin, NULL);
 
     if (vmc_channel->rcc) {
         spice_printerr("channel client %d:%d (%p) already connected, refusing 
second connection",
@@ -851,7 +857,7 @@ static void spicevmc_connect(RedChannel *channel, RedClient 
*client,
         return;
     }
 
-    sif = spice_char_device_get_interface(vmc_channel->chardev_sin);
+    sif = spice_char_device_get_interface(sin);
     if (sif->state) {
         sif->state(sin, 1);
     }
@@ -861,13 +867,7 @@ RedCharDevice *spicevmc_device_connect(RedsState *reds,
                                        SpiceCharDeviceInstance *sin,
                                        uint8_t channel_type)
 {
-    RedCharDeviceSpiceVmc *dev_state;
-    RedVmcChannel *state = red_vmc_channel_new(reds, channel_type, sin);
-
-    dev_state = RED_CHAR_DEVICE_SPICEVMC(state->chardev);
-    dev_state->channel = state;
-
-    return RED_CHAR_DEVICE(dev_state);
+    return red_char_device_spicevmc_new(sin, reds, channel_type);
 }
 
 /* Must be called from RedClient handling thread. */
@@ -922,18 +922,62 @@ red_char_device_spicevmc_dispose(GObject *object)
     }
 }
 
+enum {
+    PROP_CHANNEL_TYPE = 1
+};
+
+static void
+red_char_device_spicevmc_set_property(GObject *object,
+                                      guint property_id,
+                                      const GValue *value,
+                                      GParamSpec *pspec)
+{
+    RedCharDeviceSpiceVmc *self = RED_CHAR_DEVICE_SPICEVMC(object);
+
+    switch (property_id)
+    {
+        case PROP_CHANNEL_TYPE:
+            self->channel_type = g_value_get_uint(value);
+            break;
+        default:
+            G_OBJECT_WARN_INVALID_PROPERTY_ID(object, property_id, pspec);
+    }
+}
+
+static void
+red_char_device_spicevmc_constructed(GObject *object)
+{
+    RedCharDeviceSpiceVmc *self = RED_CHAR_DEVICE_SPICEVMC(object);
+    RedsState *reds = red_char_device_get_server(RED_CHAR_DEVICE(self));
+
+    self->channel = red_vmc_channel_new(reds, self->channel_type, self);
+    g_object_set(self, "opaque", self->channel, NULL);
+}
+
 static void
 red_char_device_spicevmc_class_init(RedCharDeviceSpiceVmcClass *klass)
 {
     GObjectClass *object_class = G_OBJECT_CLASS(klass);
     RedCharDeviceClass *char_dev_class = RED_CHAR_DEVICE_CLASS(klass);
 
+    object_class->set_property = red_char_device_spicevmc_set_property;
+    object_class->constructed = red_char_device_spicevmc_constructed;
     object_class->dispose = red_char_device_spicevmc_dispose;
 
     char_dev_class->read_one_msg_from_device = 
spicevmc_chardev_read_msg_from_dev;
     char_dev_class->send_msg_to_client = spicevmc_chardev_send_msg_to_client;
     char_dev_class->send_tokens_to_client = 
spicevmc_char_dev_send_tokens_to_client;
     char_dev_class->remove_client = spicevmc_char_dev_remove_client;
+
+    g_object_class_install_property(object_class,
+                                    PROP_CHANNEL_TYPE,
+                                    g_param_spec_uint("channel-type",
+                                                      "Channel type",
+                                                      "Channel type for this 
device",
+                                                      0, G_MAXUINT, 0,
+                                                      G_PARAM_WRITABLE |
+                                                      G_PARAM_CONSTRUCT_ONLY |
+                                                      G_PARAM_STATIC_STRINGS));
 }
 
 static void
@@ -944,13 +988,13 @@ red_char_device_spicevmc_init(RedCharDeviceSpiceVmc *self)
 static RedCharDevice *
 red_char_device_spicevmc_new(SpiceCharDeviceInstance *sin,
                              RedsState *reds,
-                             void *opaque)
+                             uint8_t channel_type)
 {
     return g_object_new(RED_TYPE_CHAR_DEVICE_SPICEVMC,
                         "sin", sin,
                         "spice-server", reds,
                         "client-tokens-interval", 0ULL,
                         "self-tokens", ~0ULL,
-                        "opaque", opaque,
+                        "channel-type", channel_type,
                         NULL);
 }
-- 
2.7.4

_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Reply via email to