<div dir="ltr"> <div class="gmail_extra"> <div class="gmail_quote">On Fri, Jul 3, 2015 at 3:58 PM, Christophe Fergeau <<a href="mailto:cfergeau@redhat.com" target="_blank">cfergeau@redhat.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">On Thu, Jul 02, 2015 at 04:41:33PM +0300, Kirill Moizik wrote: > 1)grey out usbredirection widget > 2)start async redirection > 3)on finish callback update state, update list of devices (we need to do it since we cant query device list while redirecting, so we could miss device changes) > 4) ungrey widget I'd tend to move the UI updates in a separate commit > > Signed-off-by: Kirill Moizik <<a href="mailto:kirill@daynix.com">kirill@daynix.com</a>> > --- > src/channel-usbredir.c | 32 ++++++++++++++++++++--------- > src/usb-device-widget.c | 54 ++++++++++++++++++++++++++++++++++++++++++++----- > 2 files changed, 71 insertions(+), 15 deletions(-) > > diff --git a/src/channel-usbredir.c b/src/channel-usbredir.c > index 292b82f..97003dc 100644 > --- a/src/channel-usbredir.c > +++ b/src/channel-usbredir.c > @@ -308,6 +308,23 @@ static void spice_usbredir_channel_open_acl_cb( > } > #endif > > +static void > +spice_usbredir_channel_open_device_async(GSimpleAsyncResult *simple, > + GObject *object, > + GCancellable *cancellable) > +{ > + GError *err = NULL; This was wrapped in a #ifdef ! USE_POLKIT, any reason for dropping this? </blockquote><div> </div><div>We do not need GError *err in this function, since before the change we were calling spice_usbredir_channel_open_device synchronously with GError err as output parameter, and then we were propagating error to GSimpleAsyncResult ( g_simple_async_result_take_error() call )to deal with it in cb after g_simple_async_result_complete_in_idle was called. After the change we are starting new thread ( g_simple_async_result_run_in_thread() call) , so error will be handled in cb as before , but we will provide error to GSimpleAsyncResult inside new thread.</div><div> </div><div>PS sorry for broken Linux build, i'll do my best to send fixed version with all your comments today</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"> > + SpiceUsbredirChannel *channel= (SpiceUsbredirChannel *)object; > + SpiceUsbredirChannelPrivate *priv = channel->priv; > + if (!spice_usbredir_channel_open_device(channel, &err)) { > + g_simple_async_result_take_error(simple, err); > + libusb_unref_device(priv->device); > + priv->device = NULL; > + g_boxed_free(spice_usb_device_get_type(), priv->spice_device); > + priv->spice_device = NULL; > + } > +} This is running in thread context, have you checked that this shared context (priv, channel) and all these methods are thread-safe/cannot be called concurrently to this code? <div><div class="h5"> > + > G_GNUC_INTERNAL > void spice_usbredir_channel_connect_device_async( > SpiceUsbredirChannel *channel, > @@ -319,9 +336,6 @@ void spice_usbredir_channel_connect_device_async( > { > SpiceUsbredirChannelPrivate *priv = channel->priv; > GSimpleAsyncResult *result; > -#if ! USE_POLKIT > - GError *err = NULL; > -#endif > > g_return_if_fail(SPICE_IS_USBREDIR_CHANNEL(channel)); > g_return_if_fail(device != NULL); > @@ -362,13 +376,11 @@ void spice_usbredir_channel_connect_device_async( > channel); > return; > #else > - if (!spice_usbredir_channel_open_device(channel, &err)) { > - g_simple_async_result_take_error(result, err); > - libusb_unref_device(priv->device); > - priv->device = NULL; > - g_boxed_free(spice_usb_device_get_type(), priv->spice_device); > - priv->spice_device = NULL; > - } > + g_simple_async_result_run_in_thread(result, > + spice_usbredir_channel_open_device_async, > + G_PRIORITY_DEFAULT, > + cancellable); > + return; > #endif > > done: > diff --git a/src/usb-device-widget.c b/src/usb-device-widget.c > index 1ec30e3..4c466ca 100644 > --- a/src/usb-device-widget.c > +++ b/src/usb-device-widget.c > @@ -25,6 +25,7 @@ > #include "spice-client.h" > #include "spice-marshal.h" > #include "usb-device-widget.h" > +#include "win-usb-dev.h" > > /** > * SECTION:usb-device-widget > @@ -52,6 +53,9 @@ static gboolean spice_usb_device_widget_update_status(gpointer user_data); > /* ------------------------------------------------------------------ */ > /* gobject glue */ > > + > +static void set_sensitive_all(GtkWidget *widget, gpointer user_data); > + > #define SPICE_USB_DEVICE_WIDGET_GET_PRIVATE(obj) \ > (G_TYPE_INSTANCE_GET_PRIVATE((obj), SPICE_TYPE_USB_DEVICE_WIDGET, \ > SpiceUsbDeviceWidgetPrivate)) > @@ -401,6 +405,10 @@ static gboolean spice_usb_device_widget_update_status(gpointer user_data) > SpiceUsbDeviceWidgetPrivate *priv = self->priv; > > priv->device_count = 0; > + > + if (spice_usb_device_manager_get_redirecting(priv->manager)) { > + return FALSE; > + } > gtk_container_foreach(GTK_CONTAINER(self), check_can_redirect, self); > > if (priv->err_msg) { > @@ -425,6 +433,23 @@ typedef struct _connect_cb_data { > SpiceUsbDeviceWidget *self; > } connect_cb_data; > > +static void set_redirecting(SpiceUsbDeviceWidget *self, gboolean val) > +{ > + spice_usb_device_manager_set_redirecting(self->priv->manager , val); > + spice_g_udev_set_redirecting(val); > + gboolean sensitive = !val; > + if (val == TRUE) { > + spice_usb_device_widget_show_info_bar(self, _("Redirecting Usb Device"), > + GTK_MESSAGE_INFO, > + GTK_STOCK_DIALOG_INFO); > + } else { > + spice_g_udev_handle_device_change(); > + spice_usb_device_widget_hide_info_bar(self); > + } > + gtk_container_foreach(GTK_CONTAINER(self), > + set_sensitive_all, (gpointer) &sensitive); > +} > + > static void connect_cb(GObject *gobject, GAsyncResult *res, gpointer user_data) > { > SpiceUsbDeviceManager *manager = SPICE_USB_DEVICE_MANAGER(gobject); > @@ -435,6 +460,7 @@ static void connect_cb(GObject *gobject, GAsyncResult *res, gpointer user_data) > GError *err = NULL; > gchar *desc; > > + set_redirecting (self,FALSE); > spice_usb_device_manager_connect_device_finish(manager, res, &err); > if (err) { > device = g_object_get_data(G_OBJECT(data->check), "usb-device"); > @@ -448,9 +474,9 @@ static void connect_cb(GObject *gobject, GAsyncResult *res, gpointer user_data) > g_error_free(err); > > gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(data->check), FALSE); > - spice_usb_device_widget_update_status(self); > - } > > + } > + spice_usb_device_widget_update_status(self); > g_object_unref(data->check); > g_object_unref(data->self); > g_free(data); > @@ -463,11 +489,12 @@ static void checkbox_clicked_cb(GtkWidget *check, gpointer user_data) > SpiceUsbDevice *device; > > device = g_object_get_data(G_OBJECT(check), "usb-device"); > + connect_cb_data *data = g_new(connect_cb_data, 1); > + data->check = g_object_ref(check); > + data->self = g_object_ref(self); > + set_redirecting(self, TRUE); > > if (gtk_toggle_button_get_active(GTK_TOGGLE_BUTTON(check))) { > - connect_cb_data *data = g_new(connect_cb_data, 1); > - data->check = g_object_ref(check); > - data->self = g_object_ref(self); > spice_usb_device_manager_connect_device_async(priv->manager, > device, > NULL, > @@ -502,6 +529,10 @@ static void device_added_cb(SpiceUsbDeviceManager *manager, > device)) > gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(check), TRUE); > > + > + if (spice_usb_device_manager_get_redirecting(priv->manager)) { > + gtk_widget_set_sensitive(check, FALSE); > + } > g_object_set_data_full( > G_OBJECT(check), "usb-device", > g_boxed_copy(spice_usb_device_get_type(), device), > @@ -542,6 +573,19 @@ static void set_inactive_by_usb_device(GtkWidget *widget, gpointer user_data) > } > } > > +static void set_sensitive_all(GtkWidget *widget, gpointer user_data) > +{ > + gboolean sensitive = *((gboolean *)user_data); > + SpiceUsbDevice *device; > + if (GTK_IS_BIN(widget)) { > + GtkWidget *check = gtk_bin_get_child(GTK_BIN(widget)); > + device = get_usb_device(widget); > + if (!device) > + return; /* Non device widget, ie the info_bar */ > + gtk_widget_set_sensitive(check, sensitive); > + } > +} > + > static void device_error_cb(SpiceUsbDeviceManager *manager, > SpiceUsbDevice *device, GError *err, gpointer user_data) > { > -- > 2.1.0 > </div></div>> _______________________________________________ > Spice-devel mailing list > <a href="mailto:Spice-devel@lists.freedesktop.org">Spice-devel@lists.freedesktop.org</a> > <a href="http://lists.freedesktop.org/mailman/listinfo/spice-devel" rel="noreferrer" target="_blank">http://lists.freedesktop.org/mailman/listinfo/spice-devel</a> </blockquote></div> </div></div>