[Spice-devel] [PATCH 1/4] spicevmc: Channel is owned by device

Jonathon Jongsma jjongsma at redhat.com
Thu Nov 3 20:57:40 UTC 2016


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 d095378..522814e 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);
 }
 
@@ -427,9 +428,11 @@ static RedVmcChannel *spicevmc_red_channel_client_get_state(RedChannelClient *rc
 static void spicevmc_port_send_init(RedChannelClient *rcc)
 {
     RedVmcChannel *state = spicevmc_red_channel_client_get_state(rcc);
-    SpiceCharDeviceInstance *sin = state->chardev_sin;
+    SpiceCharDeviceInstance *sin;
     RedPortInitPipeItem *item = spice_malloc(sizeof(RedPortInitPipeItem));
 
+    g_object_get(state->chardev, "sin", &sin, NULL);
+
     red_pipe_item_init(&item->base, RED_PIPE_ITEM_TYPE_PORT_INIT);
     item->name = strdup(sin->portname);
     item->opened = state->port_opened;
@@ -487,6 +490,7 @@ static int spicevmc_red_channel_client_config_socket(RedChannelClient *rcc)
 static void spicevmc_red_channel_client_on_disconnect(RedChannelClient *rcc)
 {
     RedVmcChannel *state;
+    SpiceCharDeviceInstance *sin;
     SpiceCharDeviceInterface *sif;
     RedClient *client = red_channel_client_get_client(rcc);
 
@@ -499,13 +503,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(state->chardev, &state->recv_from_client_buf);
 
-    if (state->chardev) {
-        if (red_char_device_client_exists(state->chardev, client)) {
-            red_char_device_client_remove(state->chardev, client);
-        } else {
-            spice_printerr("client %p have already been removed from char dev %p",
-                           client, state->chardev);
-        }
+    g_object_get(state->chardev, "sin", &sin, NULL);
+    if (red_char_device_client_exists(state->chardev, client)) {
+        red_char_device_client_remove(state->chardev, client);
+    } else {
+        spice_printerr("client %p have already been removed from char dev %p",
+                       client, state->chardev);
     }
 
     /* Don't destroy the rcc if it is already being destroyed, as then
@@ -514,9 +517,9 @@ static void spicevmc_red_channel_client_on_disconnect(RedChannelClient *rcc)
         red_channel_client_destroy(rcc);
 
     state->rcc = NULL;
-    sif = spice_char_device_get_interface(state->chardev_sin);
+    sif = spice_char_device_get_interface(sin);
     if (sif->state) {
-        sif->state(state->chardev_sin, 0);
+        sif->state(sin, 0);
     }
 }
 
@@ -596,10 +599,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 *state;
+    SpiceCharDeviceInstance *sin;
     SpiceCharDeviceInterface *sif;
 
     state = spicevmc_red_channel_client_get_state(rcc);
-    sif = spice_char_device_get_interface(state->chardev_sin);
+    g_object_get(state->chardev, "sin", &sin, NULL);
+    sif = spice_char_device_get_interface(sin);
 
     switch (type) {
     case SPICE_MSGC_SPICEVMC_DATA:
@@ -617,7 +622,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(state->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);
@@ -782,10 +787,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));
@@ -826,8 +832,8 @@ static void spicevmc_connect(RedChannel *channel, RedClient *client,
     uint32_t type, id;
 
     state = RED_VMC_CHANNEL(channel);
-    sin = state->chardev_sin;
     g_object_get(channel, "channel-type", &type, "id", &id, NULL);
+    g_object_get(state->chardev, "sin", &sin, NULL);
 
     if (state->rcc) {
         spice_printerr("channel client %d:%d (%p) already connected, refusing second connection",
@@ -857,7 +863,7 @@ static void spicevmc_connect(RedChannel *channel, RedClient *client,
         return;
     }
 
-    sif = spice_char_device_get_interface(state->chardev_sin);
+    sif = spice_char_device_get_interface(sin);
     if (sif->state) {
         sif->state(sin, 1);
     }
@@ -867,13 +873,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. */
@@ -928,18 +928,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
@@ -950,13 +994,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



More information about the Spice-devel mailing list