[Spice-devel] [PATCH 3/5] Make start redirection asynchronous
Kirill Moizik
kirill at daynix.com
Mon Jul 6 02:25:13 PDT 2015
On Fri, Jul 3, 2015 at 3:58 PM, Christophe Fergeau <cfergeau at redhat.com>
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 at daynix.com>
> > ---
> > 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 = 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
> >
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/spice-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20150706/43a425d4/attachment-0001.html>
More information about the Spice-devel
mailing list