[Spice-devel] [PATCH spice-gtk 6/7] usb-device-manager: One instance per session instead of a singleton

Hans de Goede hdegoede at redhat.com
Thu Oct 6 11:07:07 PDT 2011


Since usb device manager keeps track of which usb channels there are and
if they have usb devices attached there should be one usb-device-manager
instance per session, rather then one global singleton.

Tying the usb-device-manager to the session also allows us to get rid of
spice_usb_device_manager_[un]register_channel and the need for SpiceDisplay
to call these.

Signed-off-by: Hans de Goede <hdegoede at redhat.com>
---
 doc/reference/spice-gtk-sections.txt |    2 -
 gtk/map-file                         |    2 -
 gtk/spice-widget.c                   |   25 +-----
 gtk/spicy.c                          |   19 +++--
 gtk/usb-device-manager.c             |  180 +++++++++++++++++++++-------------
 gtk/usb-device-manager.h             |    8 +-
 6 files changed, 125 insertions(+), 111 deletions(-)

diff --git a/doc/reference/spice-gtk-sections.txt b/doc/reference/spice-gtk-sections.txt
index 56ae829..870352d 100644
--- a/doc/reference/spice-gtk-sections.txt
+++ b/doc/reference/spice-gtk-sections.txt
@@ -265,8 +265,6 @@ SpiceUsbDeviceManager
 SpiceUsbDeviceManagerClass
 <SUBSECTION>
 spice_usb_device_manager_get
-spice_usb_device_manager_register_channel
-spice_usb_device_manager_unregister_channel
 spice_usb_device_manager_get_devices
 spice_usb_device_manager_is_device_connected
 spice_usb_device_manager_connect_device
diff --git a/gtk/map-file b/gtk/map-file
index 82440c0..0b1b26f 100644
--- a/gtk/map-file
+++ b/gtk/map-file
@@ -84,8 +84,6 @@ spice_usb_device_get_type;
 spice_usb_device_get_description;
 spice_usb_device_manager_get_type;
 spice_usb_device_manager_get;
-spice_usb_device_manager_register_channel;
-spice_usb_device_manager_unregister_channel;
 spice_usb_device_manager_get_devices;
 spice_usb_device_manager_is_device_connected;
 spice_usb_device_manager_connect_device;
diff --git a/gtk/spice-widget.c b/gtk/spice-widget.c
index b52a2fa..1e6c922 100644
--- a/gtk/spice-widget.c
+++ b/gtk/spice-widget.c
@@ -631,7 +631,7 @@ static void update_auto_usbredir(SpiceDisplay *display)
         auto_connect = TRUE;
 
     /* FIXME: allow specifying a different GMainContext then the default */
-    manager = spice_usb_device_manager_get(NULL /* FIXME */, NULL);
+    manager = spice_usb_device_manager_get(d->session, NULL /* FIXME */, NULL);
     if (manager) {
         g_object_set(manager, "auto-connect", auto_connect, NULL);
     }
@@ -1604,18 +1604,6 @@ static void channel_new(SpiceSession *s, SpiceChannel *channel, gpointer data)
     }
 #endif
 
-    if (SPICE_IS_USBREDIR_CHANNEL(channel)) {
-        SpiceUsbDeviceManager *manager;
-
-        /* FIXME: allow specifying a different GMainContext then the default */
-        manager = spice_usb_device_manager_get(NULL /* FIXME */, NULL);
-        if (manager) {
-            spice_usb_device_manager_register_channel(manager,
-                                              SPICE_USBREDIR_CHANNEL(channel));
-        }
-        return;
-    }
-
     return;
 }
 
@@ -1659,17 +1647,6 @@ static void channel_destroy(SpiceSession *s, SpiceChannel *channel, gpointer dat
     }
 #endif
 
-    if (SPICE_IS_USBREDIR_CHANNEL(channel)) {
-        SpiceUsbDeviceManager *manager;
-
-        manager = spice_usb_device_manager_get(NULL /* FIXME */, NULL);
-        if (manager) {
-            spice_usb_device_manager_unregister_channel(manager,
-                                              SPICE_USBREDIR_CHANNEL(channel));
-        }
-        return;
-    }
-
     return;
 }
 
diff --git a/gtk/spicy.c b/gtk/spicy.c
index bf77a9f..533d7f7 100644
--- a/gtk/spicy.c
+++ b/gtk/spicy.c
@@ -96,6 +96,10 @@ static void connection_disconnect(spice_connection *conn);
 static void connection_destroy(spice_connection *conn);
 static void resolution_fullscreen(struct spice_window *win);
 static void resolution_restore(struct spice_window *win);
+static void auto_connect_failed(SpiceUsbDeviceManager *manager,
+                                SpiceUsbDevice        *device,
+                                GError                *error,
+                                gpointer               data);
 
 /* ------------------------------------------------------------------ */
 
@@ -1458,6 +1462,7 @@ static void migration_state(GObject *session,
 static spice_connection *connection_new(void)
 {
     spice_connection *conn;
+    SpiceUsbDeviceManager *manager;
 
     conn = g_new0(spice_connection, 1);
     conn->session = spice_session_new();
@@ -1467,6 +1472,13 @@ static spice_connection *connection_new(void)
                      G_CALLBACK(channel_destroy), conn);
     g_signal_connect(conn->session, "notify::migration-state",
                      G_CALLBACK(migration_state), conn);
+
+    manager = spice_usb_device_manager_get(conn->session, NULL, NULL);
+    if (manager) {
+        g_signal_connect(manager, "auto-connect-failed",
+                         G_CALLBACK(auto_connect_failed), NULL);
+    }
+
     connections++;
     SPICE_DEBUG("%s (%d)", __FUNCTION__, connections);
     return conn;
@@ -1575,7 +1587,6 @@ int main(int argc, char *argv[])
     GOptionContext *context;
     spice_connection *conn;
     gchar *conf_file, *conf;
-    SpiceUsbDeviceManager *manager;
 
     g_thread_init(NULL);
     bindtextdomain(GETTEXT_PACKAGE, SPICE_GTK_LOCALEDIR);
@@ -1636,12 +1647,6 @@ int main(int argc, char *argv[])
         g_signal_connect(rrscreen, "changed", G_CALLBACK(on_screen_changed), NULL);
     on_screen_changed(rrscreen, NULL);
 
-    manager = spice_usb_device_manager_get(NULL, NULL);
-    if (manager) {
-        g_signal_connect(manager, "auto-connect-failed",
-                         G_CALLBACK(auto_connect_failed), NULL);
-    }
-
     conn = connection_new();
     spice_set_session_option(conn->session);
     spice_cmdline_session_setup(conn->session);
diff --git a/gtk/usb-device-manager.c b/gtk/usb-device-manager.c
index f2dbf90..929e636 100644
--- a/gtk/usb-device-manager.c
+++ b/gtk/usb-device-manager.c
@@ -48,13 +48,29 @@
  * devices plugging/unplugging. If #SpiceUsbDeviceManager:auto-connect
  * is set to %TRUE, it will automatically connect newly plugged USB
  * devices to available channels.
+ *
+ * There should always be a 1:1 relation between #SpiceUsbDeviceManager objects
+ * and #SpiceSession objects. Therefor there is no
+ * spice_usb_device_manager_new, instead there is
+ * spice_usb_device_manager_get() which ensures this 1:1 relation.
  */
 
+/* ------------------------------------------------------------------ */
+/* Prototypes for private functions */
+static void channel_new(SpiceSession *session, SpiceChannel *channel,
+                        gpointer user_data);
+static void channel_destroy(SpiceSession *session, SpiceChannel *channel,
+                            gpointer user_data);
+
+/* ------------------------------------------------------------------ */
+/* gobject glue                                                       */
+
 #define SPICE_USB_DEVICE_MANAGER_GET_PRIVATE(obj)                                  \
     (G_TYPE_INSTANCE_GET_PRIVATE ((obj), SPICE_TYPE_USB_DEVICE_MANAGER, SpiceUsbDeviceManagerPrivate))
 
 enum {
     PROP_0,
+    PROP_SESSION,
     PROP_MAIN_CONTEXT,
     PROP_AUTO_CONNECT,
 };
@@ -68,6 +84,7 @@ enum
 };
 
 struct _SpiceUsbDeviceManagerPrivate {
+    SpiceSession *session;
     GMainContext *main_context;
     gboolean auto_connect;
 #ifdef USE_USBREDIR
@@ -120,10 +137,13 @@ static gboolean spice_usb_device_manager_initable_init(GInitable  *initable,
                                                     GCancellable  *cancellable,
                                                     GError        **err)
 {
-#ifdef USE_USBREDIR
-    GError *my_err = NULL;
+    GList *list;
+    GList *it;
     SpiceUsbDeviceManager *self;
     SpiceUsbDeviceManagerPrivate *priv;
+#ifdef USE_USBREDIR
+    GError *my_err = NULL;
+#endif
 
     g_return_val_if_fail(SPICE_IS_USB_DEVICE_MANAGER(initable), FALSE);
     g_return_val_if_fail(err == NULL || *err == NULL, FALSE);
@@ -131,11 +151,29 @@ static gboolean spice_usb_device_manager_initable_init(GInitable  *initable,
     if (cancellable != NULL) {
         g_set_error_literal(err, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
                             "Cancellable initialization not supported");
+        return FALSE;
     }
 
     self = SPICE_USB_DEVICE_MANAGER(initable);
     priv = self->priv;
 
+    if (!priv->session) {
+        g_set_error_literal(err, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
+                "SpiceUsbDeviceManager constructed without a session");
+        return FALSE;
+    }
+
+    g_signal_connect(priv->session, "channel-new",
+                     G_CALLBACK(channel_new), self);
+    g_signal_connect(priv->session, "channel-destroy",
+                     G_CALLBACK(channel_destroy), self);
+    list = spice_session_get_channels(priv->session);
+    for (it = g_list_first(list); it != NULL; it = g_list_next(it)) {
+        channel_new(priv->session, it->data, (gpointer*)self);
+    }
+    g_list_free(list);
+
+#ifdef USE_USBREDIR
     priv->context = g_usb_context_new(&my_err);
     if (priv->context == NULL) {
         g_warning("Could not get a GUsbContext, disabling USB support: %s",
@@ -200,6 +238,9 @@ static void spice_usb_device_manager_get_property(GObject     *gobject,
     SpiceUsbDeviceManagerPrivate *priv = self->priv;
 
     switch (prop_id) {
+    case PROP_SESSION:
+        g_value_set_object(value, priv->session);
+        break;
     case PROP_MAIN_CONTEXT:
         g_value_set_pointer(value, priv->main_context);
         break;
@@ -221,6 +262,9 @@ static void spice_usb_device_manager_set_property(GObject       *gobject,
     SpiceUsbDeviceManagerPrivate *priv = self->priv;
 
     switch (prop_id) {
+    case PROP_SESSION:
+        priv->session = g_value_get_object(value);
+        break;
     case PROP_MAIN_CONTEXT:
         priv->main_context = g_value_get_pointer(value);
         break;
@@ -243,6 +287,21 @@ static void spice_usb_device_manager_class_init(SpiceUsbDeviceManagerClass *klas
     gobject_class->set_property = spice_usb_device_manager_set_property;
 
     /**
+     * SpiceUsbDeviceManager:session:
+     *
+     * #SpiceSession this #SpiceUsbDeviceManager is associated with
+     *
+     **/
+    g_object_class_install_property
+        (gobject_class, PROP_SESSION,
+         g_param_spec_object("session",
+                             "Session",
+                             "SpiceSession",
+                             SPICE_TYPE_SESSION,
+                             G_PARAM_CONSTRUCT_ONLY | G_PARAM_READWRITE |
+                             G_PARAM_STATIC_STRINGS));
+
+    /**
      * SpiceUsbDeviceManager:main-context:
      */
     pspec = g_param_spec_pointer("main-context", "Main Context",
@@ -326,6 +385,24 @@ static void spice_usb_device_manager_class_init(SpiceUsbDeviceManagerClass *klas
 /* ------------------------------------------------------------------ */
 /* callbacks                                                          */
 
+static void channel_new(SpiceSession *session, SpiceChannel *channel,
+                        gpointer user_data)
+{
+    SpiceUsbDeviceManager *self = user_data;
+
+    if (SPICE_IS_USBREDIR_CHANNEL(channel))
+        g_ptr_array_add(self->priv->channels, channel);
+}
+
+static void channel_destroy(SpiceSession *session, SpiceChannel *channel,
+                            gpointer user_data)
+{
+    SpiceUsbDeviceManager *self = user_data;
+
+    if (SPICE_IS_USBREDIR_CHANNEL(channel))
+        g_ptr_array_remove(self->priv->channels, channel);
+}
+
 #ifdef USE_USBREDIR
 static gboolean spice_usb_device_manager_source_callback(gpointer user_data)
 {
@@ -392,17 +469,13 @@ static void spice_usb_device_manager_dev_removed(GUsbDeviceList *devlist,
 }
 #endif
 
-struct spice_usb_device_manager_new_params {
-    GMainContext *main_context;
-    GError **err;
-};
-
-static SpiceUsbDeviceManager *spice_usb_device_manager_new(void *p)
+static void
+spice_usb_device_manager_spice_session_destroyed_cb(gpointer user_data,
+                                                    GObject *object)
 {
-    struct spice_usb_device_manager_new_params *params = p;
+    SpiceUsbDeviceManager *self = user_data;
 
-    return g_initable_new(SPICE_TYPE_USB_DEVICE_MANAGER, NULL, params->err,
-                          "main-context", params->main_context, NULL);
+    g_object_unref(self);
 }
 
 /* ------------------------------------------------------------------ */
@@ -429,76 +502,43 @@ static SpiceUsbredirChannel *spice_usb_device_manager_get_channel_for_dev(
 
 /**
  * spice_usb_device_manager_get:
+ * @session: #SpiceSession for which to get the #SpiceUsbDeviceManager
  * @main_context: #GMainContext to use. If %NULL, the default context is used.
  *
- * #SpiceUsbDeviceManager is a singleton, use this function to get a pointer
- * to it. A new #SpiceUsbDeviceManager instance will be created the first
- * time this function is called
+ * Gets the #SpiceUsbDeviceManager associated with the passed in #SpiceSession.
+ * A new #SpiceUsbDeviceManager instance will be created the first time this
+ * function is called for a certain #SpiceSession.
+ *
+ * Note that this function returns a weak reference, which should not be used
+ * after the #SpiceSession itself has been unref-ed by the caller.
  *
- * Returns: (transfer none): a weak reference to the #SpiceUsbDeviceManager singleton
+ * Returns: (transfer none): a weak reference to the #SpiceUsbDeviceManager associated with the passed in #SpiceSession
  */
-SpiceUsbDeviceManager *spice_usb_device_manager_get(GMainContext *main_context,
+SpiceUsbDeviceManager *spice_usb_device_manager_get(SpiceSession *session,
+                                                    GMainContext *main_context,
                                                     GError **err)
 {
-    static GOnce manager_singleton_once = G_ONCE_INIT;
-    struct spice_usb_device_manager_new_params params;
+    SpiceUsbDeviceManager *self;
+    static GStaticMutex mutex = G_STATIC_MUTEX_INIT;
 
     g_return_val_if_fail(err == NULL || *err == NULL, NULL);
 
-    params.main_context = main_context;
-    params.err = err;
-
-    return g_once(&manager_singleton_once,
-                  (GThreadFunc)spice_usb_device_manager_new,
-                  &params);
-}
-
-/**
- * spice_usb_device_manager_register_channel:
- * @manager: the #SpiceUsbDeviceManager manager
- * @channel: a #SpiceUsbredirChannel to register
- *
- * Register @channel to be managed by the USB device @manager.  When a
- * new device is added/plugged, the @manager will use an available
- * channel to establish the redirection with the Spice server.
- *
- * Note that this function takes a weak reference to the channel, it is the
- * callers responsibility to call spice_usb_device_manager_unregister_channel()
- * before it unrefs its own reference.
- **/
-void spice_usb_device_manager_register_channel(SpiceUsbDeviceManager *self,
-                                               SpiceUsbredirChannel *channel)
-{
-    SpiceUsbDeviceManagerPrivate *priv;
-    guint i;
-
-    g_return_if_fail(SPICE_IS_USB_DEVICE_MANAGER(self));
-    g_return_if_fail(SPICE_IS_USBREDIR_CHANNEL(channel));
-
-    priv = self->priv;
-
-    for (i = 0; i < priv->channels->len; i++) {
-        if (g_ptr_array_index(priv->channels, i) == channel) {
-            g_return_if_reached();
-        }
+    g_static_mutex_lock(&mutex);
+    self = g_object_get_data(G_OBJECT(session), "spice-usb-device-manager");
+    if (self == NULL) {
+        self = g_initable_new(SPICE_TYPE_USB_DEVICE_MANAGER, NULL, err,
+                              "session", session,
+                              "main-context", main_context, NULL);
+        g_object_set_data(G_OBJECT(session), "spice-usb-device-manager", self);
+        if (self)
+            /* Ensure we are destroyed together with the SpiceSession */
+            g_object_weak_ref(G_OBJECT(session),
+                          spice_usb_device_manager_spice_session_destroyed_cb,
+                          self);
     }
-    g_ptr_array_add(self->priv->channels, channel);
-}
-
-/**
- * spice_usb_device_manager_unregister_channel:
- * @manager: the #SpiceUsbDeviceManager manager
- * @channel: a #SpiceUsbredirChannel to unregister
- *
- * Remove @channel from the list of USB channels to be managed by @manager.
- */
-void spice_usb_device_manager_unregister_channel(SpiceUsbDeviceManager *self,
-                                                 SpiceUsbredirChannel *channel)
-{
-    g_return_if_fail(SPICE_IS_USB_DEVICE_MANAGER(self));
-    g_return_if_fail(SPICE_IS_USBREDIR_CHANNEL(channel));
+    g_static_mutex_unlock(&mutex);
 
-    g_warn_if_fail(g_ptr_array_remove(self->priv->channels, channel));
+    return self;
 }
 
 /**
diff --git a/gtk/usb-device-manager.h b/gtk/usb-device-manager.h
index 46a5e55..00f8eb2 100644
--- a/gtk/usb-device-manager.h
+++ b/gtk/usb-device-manager.h
@@ -88,14 +88,10 @@ GType spice_usb_device_manager_get_type(void);
 
 gchar *spice_usb_device_get_description(SpiceUsbDevice *device);
 
-SpiceUsbDeviceManager *spice_usb_device_manager_get(GMainContext *main_context,
+SpiceUsbDeviceManager *spice_usb_device_manager_get(SpiceSession *session,
+                                                    GMainContext *main_context,
                                                     GError **err);
 
-void spice_usb_device_manager_register_channel(SpiceUsbDeviceManager *manager,
-                                               SpiceUsbredirChannel *channel);
-void spice_usb_device_manager_unregister_channel(SpiceUsbDeviceManager *manager,
-                                                 SpiceUsbredirChannel *channel);
-
 GPtrArray *spice_usb_device_manager_get_devices(SpiceUsbDeviceManager *manager);
 
 gboolean spice_usb_device_manager_is_device_connected(SpiceUsbDeviceManager *manager,
-- 
1.7.6.4



More information about the Spice-devel mailing list