[virt-tools-list] [PATCH] usbredir: Don't depend on channel ordering
Hans de Goede
hdegoede at redhat.com
Sat Jul 6 07:07:17 UTC 2013
Hi,
On 06/28/2013 12:20 AM, Marc-André Lureau wrote:
> Is there a bug for that?
No, and no one has seen any problems due to this, except for
Guannan Ren when working on usbredir support for virt-manager.
He apparently has a somewhat special setup triggering this.
> Are you adding dynamic usbredir channels?
Nope.
> ack
Thanks, pushed.
Regards,
Hans
>
> ----- Mensaje original -----
>> Before this patch-set virt-viewer was calling spice_session_has_channel_type(
>> session, SPICE_CHANNEL_USBREDIR) from the session-initialized signal handler,
>>
>> So as soon as the display channel gets added to the session, the check was
>> done. This causes the check to return FALSE for usbredir capable vms if
>> the usbredir channel(s) get added to the session after the display channed.
>>
>> This patch refactors things to not depend on channel creation order.
>>
>> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
>> ---
>> src/virt-viewer-app.c | 18 +++++++++++++-----
>> src/virt-viewer-session-spice.c | 28 +++++++++++++++-------------
>> src/virt-viewer-session.c | 39 ++++++++++++++++++++++++++++++++-------
>> src/virt-viewer-session.h | 5 +++--
>> 4 files changed, 63 insertions(+), 27 deletions(-)
>>
>> diff --git a/src/virt-viewer-app.c b/src/virt-viewer-app.c
>> index c2ecc67..69cc328 100644
>> --- a/src/virt-viewer-app.c
>> +++ b/src/virt-viewer-app.c
>> @@ -589,8 +589,8 @@ virt_viewer_app_set_nth_window(VirtViewerApp *self, gint
>> nth, VirtViewerWindow *
>> virt_viewer_app_set_window_subtitle(self, win, nth);
>> virt_viewer_app_update_menu_displays(self);
>> if (self->priv->session) {
>> - gboolean has_usb = virt_viewer_session_has_usb(self->priv->session);
>> - virt_viewer_window_set_usb_options_sensitive(win, has_usb);
>> + virt_viewer_window_set_usb_options_sensitive(win,
>> +
>> virt_viewer_session_get_has_usbredir(self->priv->session));
>> }
>>
>> g_signal_emit(self, signals[SIGNAL_WINDOW_ADDED], 0, win);
>> @@ -748,6 +748,15 @@ virt_viewer_app_display_updated(VirtViewerSession
>> *session G_GNUC_UNUSED,
>> virt_viewer_app_update_menu_displays(self);
>> }
>>
>> +static void
>> +virt_viewer_app_has_usbredir_updated(VirtViewerSession *session,
>> + GParamSpec *pspec G_GNUC_UNUSED,
>> + VirtViewerApp *self)
>> +{
>> + virt_viewer_app_set_usb_options_sensitive(self,
>> + virt_viewer_session_get_has_usbredir(session));
>> +}
>> +
>> int
>> virt_viewer_app_create_session(VirtViewerApp *self, const gchar *type)
>> {
>> @@ -800,6 +809,8 @@ virt_viewer_app_create_session(VirtViewerApp *self, const
>> gchar *type)
>> G_CALLBACK(virt_viewer_app_display_removed), self);
>> g_signal_connect(priv->session, "session-display-updated",
>> G_CALLBACK(virt_viewer_app_display_updated), self);
>> + g_signal_connect(priv->session, "notify::has-usbredir",
>> + G_CALLBACK(virt_viewer_app_has_usbredir_updated),
>> self);
>>
>> g_signal_connect(priv->session, "session-cut-text",
>> G_CALLBACK(virt_viewer_app_server_cut_text), self);
>> @@ -1145,10 +1156,7 @@ static void
>> virt_viewer_app_initialized(VirtViewerSession *session G_GNUC_UNUSED,
>> VirtViewerApp *self)
>> {
>> - gboolean has_usb = virt_viewer_session_has_usb(self->priv->session);
>> -
>> virt_viewer_app_update_title(self);
>> - virt_viewer_app_set_usb_options_sensitive(self, has_usb);
>> }
>>
>> static void
>> diff --git a/src/virt-viewer-session-spice.c
>> b/src/virt-viewer-session-spice.c
>> index 06e0005..74fa3fe 100644
>> --- a/src/virt-viewer-session-spice.c
>> +++ b/src/virt-viewer-session-spice.c
>> @@ -51,6 +51,7 @@ struct _VirtViewerSessionSpicePrivate {
>> SpiceMainChannel *main_channel;
>> const SpiceAudio *audio;
>> int channel_count;
>> + int usbredir_channel_count;
>> };
>>
>> #define VIRT_VIEWER_SESSION_SPICE_GET_PRIVATE(o)
>> (G_TYPE_INSTANCE_GET_PRIVATE((o), VIRT_VIEWER_TYPE_SESSION_SPICE,
>> VirtViewerSessionSpicePrivate))
>> @@ -66,7 +67,6 @@ static gboolean
>> virt_viewer_session_spice_open_fd(VirtViewerSession *session, in
>> static gboolean virt_viewer_session_spice_open_host(VirtViewerSession
>> *session, const gchar *host, const gchar *port, const gchar *tlsport);
>> static gboolean virt_viewer_session_spice_open_uri(VirtViewerSession
>> *session, const gchar *uri, GError **error);
>> static gboolean virt_viewer_session_spice_channel_open_fd(VirtViewerSession
>> *session, VirtViewerSessionChannel *channel, int fd);
>> -static gboolean virt_viewer_session_spice_has_usb(VirtViewerSession
>> *session);
>> static void virt_viewer_session_spice_usb_device_selection(VirtViewerSession
>> *session, GtkWindow *parent);
>> static void virt_viewer_session_spice_channel_new(SpiceSession *s,
>> SpiceChannel *channel,
>> @@ -145,7 +145,6 @@
>> virt_viewer_session_spice_class_init(VirtViewerSessionSpiceClass *klass)
>> dclass->open_host = virt_viewer_session_spice_open_host;
>> dclass->open_uri = virt_viewer_session_spice_open_uri;
>> dclass->channel_open_fd = virt_viewer_session_spice_channel_open_fd;
>> - dclass->has_usb = virt_viewer_session_spice_has_usb;
>> dclass->usb_device_selection =
>> virt_viewer_session_spice_usb_device_selection;
>> dclass->smartcard_insert = virt_viewer_session_spice_smartcard_insert;
>> dclass->smartcard_remove = virt_viewer_session_spice_smartcard_remove;
>> @@ -461,17 +460,6 @@
>> virt_viewer_session_spice_main_channel_event(SpiceChannel *channel
>> G_GNUC_UNUSED
>> g_free(password);
>> }
>>
>> -static gboolean
>> -virt_viewer_session_spice_has_usb(VirtViewerSession *session)
>> -{
>> - VirtViewerSessionSpice *self = VIRT_VIEWER_SESSION_SPICE(session);
>> - VirtViewerSessionSpicePrivate *priv = self->priv;
>> -
>> - return spice_usb_device_manager_get(priv->session, NULL) &&
>> - spice_session_has_channel_type(priv->session,
>> - SPICE_CHANNEL_USBREDIR);
>> -}
>> -
>> static void remove_cb(GtkContainer *container G_GNUC_UNUSED,
>> GtkWidget *widget G_GNUC_UNUSED,
>> void *user_data)
>> @@ -645,6 +633,13 @@ virt_viewer_session_spice_channel_new(SpiceSession *s,
>> self->priv->audio = spice_audio_get(s, NULL);
>> }
>>
>> + if (SPICE_IS_USBREDIR_CHANNEL(channel)) {
>> + DEBUG_LOG("new usbredir channel");
>> + self->priv->usbredir_channel_count++;
>> + if (spice_usb_device_manager_get(self->priv->session, NULL))
>> + virt_viewer_session_set_has_usbredir(session, TRUE);
>> + }
>> +
>> self->priv->channel_count++;
>> }
>>
>> @@ -729,6 +724,13 @@ virt_viewer_session_spice_channel_destroy(G_GNUC_UNUSED
>> SpiceSession *s,
>> self->priv->audio = NULL;
>> }
>>
>> + if (SPICE_IS_USBREDIR_CHANNEL(channel)) {
>> + DEBUG_LOG("zap usbredir channel");
>> + self->priv->usbredir_channel_count--;
>> + if (self->priv->usbredir_channel_count == 0)
>> + virt_viewer_session_set_has_usbredir(session, FALSE);
>> + }
>> +
>> self->priv->channel_count--;
>> if (self->priv->channel_count == 0)
>> g_signal_emit_by_name(self, "session-disconnected");
>> diff --git a/src/virt-viewer-session.c b/src/virt-viewer-session.c
>> index 07e7226..690f691 100644
>> --- a/src/virt-viewer-session.c
>> +++ b/src/virt-viewer-session.c
>> @@ -37,6 +37,7 @@ struct _VirtViewerSessionPrivate
>> GList *displays;
>> VirtViewerApp *app;
>> gboolean auto_usbredir;
>> + gboolean has_usbredir;
>> gchar *uri;
>> VirtViewerFile *file;
>> };
>> @@ -48,6 +49,7 @@ enum {
>>
>> PROP_APP,
>> PROP_AUTO_USBREDIR,
>> + PROP_HAS_USBREDIR,
>> PROP_FILE
>> };
>>
>> @@ -82,6 +84,10 @@ virt_viewer_session_set_property(GObject *object,
>> virt_viewer_session_set_auto_usbredir(self,
>> g_value_get_boolean(value));
>> break;
>>
>> + case PROP_HAS_USBREDIR:
>> + self->priv->has_usbredir = g_value_get_boolean(value);
>> + break;
>> +
>> case PROP_APP:
>> self->priv->app = g_value_get_object(value);
>> break;
>> @@ -109,6 +115,10 @@ virt_viewer_session_get_property(GObject *object,
>> g_value_set_boolean(value,
>> virt_viewer_session_get_auto_usbredir(self));
>> break;
>>
>> + case PROP_HAS_USBREDIR:
>> + g_value_set_boolean(value, self->priv->has_usbredir);
>> + break;
>> +
>> case PROP_APP:
>> g_value_set_object(value, self->priv->app);
>> break;
>> @@ -143,6 +153,16 @@ virt_viewer_session_class_init(VirtViewerSessionClass
>> *class)
>> G_PARAM_STATIC_STRINGS));
>>
>> g_object_class_install_property(object_class,
>> + PROP_HAS_USBREDIR,
>> + g_param_spec_boolean("has-usbredir",
>> + "has USB
>> redirection",
>> + "has USB
>> redirection",
>> + FALSE,
>> + G_PARAM_READWRITE |
>> + G_PARAM_CONSTRUCT |
>> +
>> G_PARAM_STATIC_STRINGS));
>> +
>> + g_object_class_install_property(object_class,
>> PROP_APP,
>> g_param_spec_object("app",
>> "VirtViewerApp",
>> @@ -429,17 +449,22 @@ gboolean
>> virt_viewer_session_get_auto_usbredir(VirtViewerSession *self)
>> return self->priv->auto_usbredir;
>> }
>>
>> -gboolean virt_viewer_session_has_usb(VirtViewerSession *self)
>> +void virt_viewer_session_set_has_usbredir(VirtViewerSession *self, gboolean
>> has_usbredir)
>> {
>> - VirtViewerSessionClass *klass;
>> + g_return_if_fail(VIRT_VIEWER_IS_SESSION(self));
>>
>> - g_return_val_if_fail(VIRT_VIEWER_IS_SESSION(self), FALSE);
>> + if (self->priv->has_usbredir == has_usbredir)
>> + return;
>>
>> - klass = VIRT_VIEWER_SESSION_GET_CLASS(self);
>> - if (klass->has_usb == NULL)
>> - return FALSE;
>> + self->priv->has_usbredir = has_usbredir;
>> + g_object_notify(G_OBJECT(self), "has-usbredir");
>> +}
>> +
>> +gboolean virt_viewer_session_get_has_usbredir(VirtViewerSession *self)
>> +{
>> + g_return_val_if_fail(VIRT_VIEWER_IS_SESSION(self), FALSE);
>>
>> - return klass->has_usb(self);
>> + return self->priv->has_usbredir;
>> }
>>
>> void virt_viewer_session_usb_device_selection(VirtViewerSession *self,
>> diff --git a/src/virt-viewer-session.h b/src/virt-viewer-session.h
>> index 5aa748c..0467724 100644
>> --- a/src/virt-viewer-session.h
>> +++ b/src/virt-viewer-session.h
>> @@ -70,7 +70,6 @@ struct _VirtViewerSessionClass {
>> gboolean (* open_host) (VirtViewerSession* session, const gchar *host,
>> const gchar *port, const gchar *tlsport);
>> gboolean (* open_uri) (VirtViewerSession* session, const gchar *uri,
>> GError **error);
>> gboolean (* channel_open_fd) (VirtViewerSession* session,
>> VirtViewerSessionChannel *channel, int fd);
>> - gboolean (* has_usb) (VirtViewerSession* session);
>> void (* usb_device_selection) (VirtViewerSession* session, GtkWindow
>> *parent);
>> void (* smartcard_insert) (VirtViewerSession* session);
>> void (* smartcard_remove) (VirtViewerSession* session);
>> @@ -119,7 +118,9 @@ gboolean virt_viewer_session_open_uri(VirtViewerSession
>> *session, const gchar *u
>> void virt_viewer_session_set_auto_usbredir(VirtViewerSession* session,
>> gboolean auto_usbredir);
>> gboolean virt_viewer_session_get_auto_usbredir(VirtViewerSession* session);
>>
>> -gboolean virt_viewer_session_has_usb(VirtViewerSession *self);
>> +void virt_viewer_session_set_has_usbredir(VirtViewerSession* session,
>> gboolean has_usbredir);
>> +gboolean virt_viewer_session_get_has_usbredir(VirtViewerSession *self);
>> +
>> void virt_viewer_session_usb_device_selection(VirtViewerSession *self,
>> GtkWindow *parent);
>> void virt_viewer_session_smartcard_insert(VirtViewerSession *self);
>> void virt_viewer_session_smartcard_remove(VirtViewerSession *self);
>> --
>> 1.8.3.1
>>
>> _______________________________________________
>> virt-tools-list mailing list
>> virt-tools-list at redhat.com
>> https://www.redhat.com/mailman/listinfo/virt-tools-list
>>
More information about the virt-tools-list
mailing list