Re: [PATCH 3/5] Make start redirection asynchronous

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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

[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]