On Fri, Jul 3, 2015 at 3:58 PM, Christophe Fergeau <cfergeau@xxxxxxxxxx> wrote:
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 <kirill@xxxxxxxxxx>
> ---
> 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?
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.
PS sorry for broken Linux build, i'll do my best to send fixed version with all your comments today
> + 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?
> _______________________________________________
> +
> 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 = "" 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 = "" 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
>
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel