[Spice-devel] [PATCH spice-gtk 6/7] usb-device-manager: One instance per session instead of a singleton
Marc-André Lureau
marcandre.lureau at gmail.com
Thu Oct 6 12:02:34 PDT 2011
ack
On Thu, Oct 6, 2011 at 8:07 PM, Hans de Goede <hdegoede at redhat.com> wrote:
> 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,
> - ¶ms);
> -}
> -
> -/**
> - * 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
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>
--
Marc-André Lureau
More information about the Spice-devel
mailing list