Re: Spice-devel Digest, Vol 72, Issue 136

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

 






Hi list
          I don't want to receive the mail from this list, please tell me how to un-subscribe the mail?
          Thanks a lot.
  
At 2016-01-22 03:57:52, spice-devel-request@xxxxxxxxxxxxxxxxxxxxx wrote:
>Send Spice-devel mailing list submissions to
>	spice-devel@xxxxxxxxxxxxxxxxxxxxx
>
>To subscribe or unsubscribe via the World Wide Web, visit
>	http://lists.freedesktop.org/mailman/listinfo/spice-devel
>or, via email, send a message with subject or body 'help' to
>	spice-devel-request@xxxxxxxxxxxxxxxxxxxxx
>
>You can reach the person managing the list at
>	spice-devel-owner@xxxxxxxxxxxxxxxxxxxxx
>
>When replying, please edit your Subject line so it is more specific
>than "Re: Contents of Spice-devel digest..."
>
>
>Today's Topics:
>
>   1. [PATCH v2 0/2] Solve misleading message when the last	USB
>      redirection channel is taken (Fabiano Fidêncio)
>   2. [PATCH v2 1/2] usb-device-{manager,	widget}: Add counter of
>      free channels (Fabiano Fidêncio)
>   3. [PATCH v2 2/2] usb-device-{manager,	widget}: Remove
>      misleading message when there are no channels to	redirect
>      (Fabiano Fidêncio)
>   4. Re: [PATCH v2 1/2] usb-device-{manager, widget}: Add counter
>      of free channels (Jonathon Jongsma)
>   5. Re: [PATCH v2 2/2] usb-device-{manager, widget}: Remove
>      misleading message when there are no channels to redirect
>      (Jonathon Jongsma)
>   6. Re: [PATCH v2 2/2] usb-device-{manager, widget}: Remove
>      misleading message when there are no channels to redirect
>      (Fabiano Fidêncio)
>
>
>----------------------------------------------------------------------
>
>Message: 1
>Date: Thu, 21 Jan 2016 19:09:04 +0100
>From: Fabiano Fidêncio <fidencio@xxxxxxxxxx>
>To: spice-devel@xxxxxxxxxxxxxxxxxxxxx
>Subject: [Spice-devel] [PATCH v2 0/2] Solve misleading message when
>	the last	USB redirection channel is taken
>Message-ID: <1453399746-21273-1-git-send-email-fidencio@xxxxxxxxxx>
>Content-Type: text/plain; charset=UTF-8
>
>This series tries to solve the issues reported on rhbz#1298772 in the less
>intrusive way possible. As mentioned in the commit log of the second patch
>and by Jonathon during the review of v1 this part of code needs some love
>and a refactoring task is more than appreciated, but I want to get back to
>it later.
>
>Also, Marc-André suggested using a property instead of a new function for
>exposing the number of free channels. I took the suggestion and introduced
>a new property: UsbDeviceManager::free-channels. It's just a read-only
>property as I didn't see the reason why the application using that should
>try to set the property (and consequently. no property notify added).
>
>Fabiano Fidêncio (2):
>  usb-device-{manager,widget}: Add counter of free channels
>  usb-device-{manager,widget}: Remove misleading message when there are
>    no channels to redirect
>
> src/spice-client.h       |  1 +
> src/usb-device-manager.c | 32 +++++++++++++++++++++++++++++++-
> src/usb-device-widget.c  | 43 ++++++++++++++++++++++++++++++++++---------
> 3 files changed, 66 insertions(+), 10 deletions(-)
>
>-- 
>2.5.0
>
>
>
>------------------------------
>
>Message: 2
>Date: Thu, 21 Jan 2016 19:09:05 +0100
>From: Fabiano Fidêncio <fidencio@xxxxxxxxxx>
>To: spice-devel@xxxxxxxxxxxxxxxxxxxxx
>Subject: [Spice-devel] [PATCH v2 1/2] usb-device-{manager,	widget}:
>	Add counter of free channels
>Message-ID: <1453399746-21273-2-git-send-email-fidencio@xxxxxxxxxx>
>
>As the message showed when the last usbredir channel is taken can be a
>bit confusing, let's add a counter of free channels to the widget's
>label.
>In order to add the counter, a new property for SpiceUsbDeviceManager
>was introduced ("free-channels").
>
>Related: rhbz#1298772
>---
> src/usb-device-manager.c | 28 ++++++++++++++++++++++++++++
> src/usb-device-widget.c  | 37 ++++++++++++++++++++++++++++---------
> 2 files changed, 56 insertions(+), 9 deletions(-)
>
>diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c
>index c62f56e..9e0e785 100644
>--- a/src/usb-device-manager.c
>+++ b/src/usb-device-manager.c
>@@ -89,6 +89,7 @@ enum {
>     PROP_AUTO_CONNECT,
>     PROP_AUTO_CONNECT_FILTER,
>     PROP_REDIRECT_ON_CONNECT,
>+    PROP_FREE_CHANNELS,
> };
> 
> enum
>@@ -390,6 +391,19 @@ static void spice_usb_device_manager_get_property(GObject     *gobject,
>     case PROP_REDIRECT_ON_CONNECT:
>         g_value_set_string(value, priv->redirect_on_connect);
>         break;
>+    case PROP_FREE_CHANNELS: {
>+        int free_channels = 0;
>+#if USE_USBREDIR
>+        for (int i = 0; i < priv->channels->len; i++) {
>+            SpiceUsbredirChannel *channel = g_ptr_array_index(priv->channels, i);
>+
>+            if (!spice_usbredir_channel_get_device(channel))
>+                free_channels++;
>+        }
>+#endif
>+        g_value_set_int(value, free_channels);
>+        break;
>+    }
>     default:
>         G_OBJECT_WARN_INVALID_PROPERTY_ID(gobject, prop_id, pspec);
>         break;
>@@ -550,6 +564,20 @@ static void spice_usb_device_manager_class_init(SpiceUsbDeviceManagerClass *klas
>                                     pspec);
> 
>     /**
>+     * SpiceUsbDeviceManager:n-free-channels:
>+     *
>+     * Get a list of avaialable channels for redirecting USB devices.
>+     */
>+    pspec = g_param_spec_int("free-channels", "Free channels",
>+               "The number of available channels for redirecting USB devices",
>+               0,
>+               G_MAXINT,
>+               0,
>+               G_PARAM_READABLE);
>+    g_object_class_install_property(gobject_class, PROP_FREE_CHANNELS,
>+                                    pspec);
>+
>+    /**
>      * SpiceUsbDeviceManager::device-added:
>      * @manager: the #SpiceUsbDeviceManager that emitted the signal
>      * @device: #SpiceUsbDevice boxed object corresponding to the added device
>diff --git a/src/usb-device-widget.c b/src/usb-device-widget.c
>index fe983c9..0866adb 100644
>--- a/src/usb-device-widget.c
>+++ b/src/usb-device-widget.c
>@@ -72,6 +72,7 @@ struct _SpiceUsbDeviceWidgetPrivate {
>     gchar *device_format_string;
>     SpiceUsbDeviceManager *manager;
>     GtkWidget *info_bar;
>+    GtkWidget *label;
>     gchar *err_msg;
>     gsize device_count;
> };
>@@ -181,9 +182,8 @@ static GObject *spice_usb_device_widget_constructor(
>     SpiceUsbDeviceWidgetPrivate *priv;
>     GPtrArray *devices = NULL;
>     GError *err = NULL;
>-    GtkWidget *label;
>-    gchar *str;
>-    int i;
>+    gchar *str, *markup_str;
>+    int i, free_channels;
> 
>     {
>         /* Always chain up to the parent constructor */
>@@ -197,12 +197,12 @@ static GObject *spice_usb_device_widget_constructor(
>     if (!priv->session)
>         g_error("SpiceUsbDeviceWidget constructed without a session");
> 
>-    label = gtk_label_new(NULL);
>-    str = g_strdup_printf("<b>%s</b>", _("Select USB devices to redirect"));
>-    gtk_label_set_markup(GTK_LABEL (label), str);
>-    g_free(str);
>-    gtk_misc_set_alignment(GTK_MISC(label), 0.0, 0.5);
>-    gtk_box_pack_start(GTK_BOX(self), label, FALSE, FALSE, 0);
>+    priv->label = gtk_label_new(NULL);
>+    markup_str = g_strdup_printf("<b>%s</b>", _("Select USB devices to redirect"));
>+    gtk_label_set_markup(GTK_LABEL (priv->label), markup_str);
>+    g_free(markup_str);
>+    gtk_misc_set_alignment(GTK_MISC(priv->label), 0.0, 0.5);
>+    gtk_box_pack_start(GTK_BOX(self), priv->label, FALSE, FALSE, 0);
> 
>     priv->manager = spice_usb_device_manager_get(priv->session, &err);
>     if (err) {
>@@ -213,6 +213,14 @@ static GObject *spice_usb_device_widget_constructor(
>         return obj;
>     }
> 
>+    g_object_get(priv->manager, "free-channels", &free_channels, NULL);
>+    str = g_strdup_printf(_("Select USB devices to redirect (%d channels free)"),
>+                          free_channels);
>+    markup_str = g_strdup_printf("<b>%s</b>", str);
>+    gtk_label_set_markup(GTK_LABEL (priv->label), markup_str);
>+    g_free(markup_str);
>+    g_free(str);
>+
>     g_signal_connect(priv->manager, "device-added",
>                      G_CALLBACK(device_added_cb), self);
>     g_signal_connect(priv->manager, "device-removed",
>@@ -463,6 +471,8 @@ static void checkbox_clicked_cb(GtkWidget *check, gpointer user_data)
>     SpiceUsbDeviceWidget *self = SPICE_USB_DEVICE_WIDGET(user_data);
>     SpiceUsbDeviceWidgetPrivate *priv = self->priv;
>     SpiceUsbDevice *device;
>+    gchar *str, *markup_str;
>+    int free_channels;
> 
>     device = g_object_get_data(G_OBJECT(check), "usb-device");
> 
>@@ -479,6 +489,15 @@ static void checkbox_clicked_cb(GtkWidget *check, gpointer user_data)
>         spice_usb_device_manager_disconnect_device(priv->manager,
>                                                    device);
>     }
>+
>+    g_object_get(priv->manager, "free-channels", &free_channels, NULL);
>+    str = g_strdup_printf(_("Select USB devices to redirect (%d channels free)"),
>+                          free_channels);
>+    markup_str = g_strdup_printf("<b>%s</b>", str);
>+    gtk_label_set_markup(GTK_LABEL (priv->label), markup_str);
>+    g_free(markup_str);
>+    g_free(str);
>+
>     spice_usb_device_widget_update_status(self);
> }
> 
>-- 
>2.5.0
>
>
>
>------------------------------
>
>Message: 3
>Date: Thu, 21 Jan 2016 19:09:06 +0100
>From: Fabiano Fidêncio <fidencio@xxxxxxxxxx>
>To: spice-devel@xxxxxxxxxxxxxxxxxxxxx
>Subject: [Spice-devel] [PATCH v2 2/2] usb-device-{manager,	widget}:
>	Remove misleading message when there are no channels to	redirect
>Message-ID: <1453399746-21273-3-git-send-email-fidencio@xxxxxxxxxx>
>
>The whole code from usb-device-manager is quite a mess as pointed out by
>Jonathon during the first round of code review [0].
>Keeping this in mind and knowing that that code needs a refactor, I'm
>proposing a simple (and messy) fix that can be backported easily before
>actually dig into a code refactoring task.
>The solution is basically adding a new error and using it to filter out
>this specific kind out situation, avoiding to show that misleading
>warning message when the last usb redirection channel is taken.
>
>[0]:
>http://lists.freedesktop.org/archives/spice-devel/2016-January/025864.html
>
>Related: rhbz#1298772
>---
> src/spice-client.h       | 1 +
> src/usb-device-manager.c | 4 +++-
> src/usb-device-widget.c  | 6 ++++++
> 3 files changed, 10 insertions(+), 1 deletion(-)
>
>diff --git a/src/spice-client.h b/src/spice-client.h
>index 32b79ea..ea7a557 100644
>--- a/src/spice-client.h
>+++ b/src/spice-client.h
>@@ -82,6 +82,7 @@ typedef enum
>     SPICE_CLIENT_ERROR_AUTH_NEEDS_USERNAME,
>     SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD_AND_USERNAME,
>     SPICE_CLIENT_ERROR_USB_SERVICE,
>+    SPICE_CLIENT_ERROR_NO_USB_CHANNELS_AVAILABLE,
> } SpiceClientError;
> 
> #ifndef SPICE_DISABLE_DEPRECATED
>diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c
>index 9e0e785..f40cc67 100644
>--- a/src/usb-device-manager.c
>+++ b/src/usb-device-manager.c
>@@ -1701,7 +1701,9 @@ spice_usb_device_manager_can_redirect_device(SpiceUsbDeviceManager  *self,
>             break;
>     }
>     if (i == priv->channels->len) {
>-        g_set_error_literal(err, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
>+        g_set_error_literal(err,
>+                            SPICE_CLIENT_ERROR,
>+                            SPICE_CLIENT_ERROR_NO_USB_CHANNELS_AVAILABLE,
>                             _("There are no free USB channels"));
>         return FALSE;
>     }
>diff --git a/src/usb-device-widget.c b/src/usb-device-widget.c
>index 0866adb..544ceaa 100644
>--- a/src/usb-device-widget.c
>+++ b/src/usb-device-widget.c
>@@ -386,6 +386,11 @@ static void check_can_redirect(GtkWidget *widget, gpointer user_data)
>                                                                 device, &err);
>     gtk_widget_set_sensitive(widget, can_redirect);
> 
>+    if (g_error_matches(err,
>+                        SPICE_CLIENT_ERROR,
>+                        SPICE_CLIENT_ERROR_NO_USB_CHANNELS_AVAILABLE))
>+        goto end;
>+
>     /* If we cannot redirect this device, append the error message to
>        err_msg, but only if it is *not* already there! */
>     if (!can_redirect) {
>@@ -402,6 +407,7 @@ static void check_can_redirect(GtkWidget *widget, gpointer user_data)
>         }
>     }
> 
>+end:
>     g_clear_error(&err);
> }
> 
>-- 
>2.5.0
>
>
>
>------------------------------
>
>Message: 4
>Date: Thu, 21 Jan 2016 13:46:20 -0600
>From: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
>To: Fabiano Fidêncio <fidencio@xxxxxxxxxx>,
>	spice-devel@xxxxxxxxxxxxxxxxxxxxx
>Subject: Re: [Spice-devel] [PATCH v2 1/2] usb-device-{manager,
>	widget}: Add counter of free channels
>Message-ID: <1453405580.14354.126.camel@xxxxxxxxxx>
>Content-Type: text/plain; charset="UTF-8"
>
>On Thu, 2016-01-21 at 19:09 +0100, Fabiano Fidêncio wrote:
>> As the message showed when the last usbredir channel is taken can be a
>> bit confusing, let's add a counter of free channels to the widget's
>> label.
>> In order to add the counter, a new property for SpiceUsbDeviceManager
>> was introduced ("free-channels").
>> 
>> Related: rhbz#1298772
>> ---
>>  src/usb-device-manager.c | 28 ++++++++++++++++++++++++++++
>>  src/usb-device-widget.c  | 37 ++++++++++++++++++++++++++++---------
>>  2 files changed, 56 insertions(+), 9 deletions(-)
>> 
>> diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c
>> index c62f56e..9e0e785 100644
>> --- a/src/usb-device-manager.c
>> +++ b/src/usb-device-manager.c
>> @@ -89,6 +89,7 @@ enum {
>>      PROP_AUTO_CONNECT,
>>      PROP_AUTO_CONNECT_FILTER,
>>      PROP_REDIRECT_ON_CONNECT,
>> +    PROP_FREE_CHANNELS,
>>  };
>>  
>>  enum
>> @@ -390,6 +391,19 @@ static void spice_usb_device_manager_get_property(GObject
>>      *gobject,
>>      case PROP_REDIRECT_ON_CONNECT:
>>          g_value_set_string(value, priv->redirect_on_connect);
>>          break;
>> +    case PROP_FREE_CHANNELS: {
>> +        int free_channels = 0;
>> +#if USE_USBREDIR
>> +        for (int i = 0; i < priv->channels->len; i++) {
>> +            SpiceUsbredirChannel *channel = g_ptr_array_index(priv->channels,
>> i);
>> +
>> +            if (!spice_usbredir_channel_get_device(channel))
>> +                free_channels++;
>> +        }
>> +#endif
>> +        g_value_set_int(value, free_channels);
>> +        break;
>> +    }
>>      default:
>>          G_OBJECT_WARN_INVALID_PROPERTY_ID(gobject, prop_id, pspec);
>>          break;
>> @@ -550,6 +564,20 @@ static void
>> spice_usb_device_manager_class_init(SpiceUsbDeviceManagerClass *klas
>>                                      pspec);
>>  
>>      /**
>> +     * SpiceUsbDeviceManager:n-free-channels:
>> +     *
>> +     * Get a list of avaialable channels for redirecting USB devices.
>> +     */
>> +    pspec = g_param_spec_int("free-channels", "Free channels",
>> +               "The number of available channels for redirecting USB
>> devices",
>> +               0,
>> +               G_MAXINT,
>> +               0,
>> +               G_PARAM_READABLE);
>> +    g_object_class_install_property(gobject_class, PROP_FREE_CHANNELS,
>> +                                    pspec);
>> +
>> +    /**
>>       * SpiceUsbDeviceManager::device-added:
>>       * @manager: the #SpiceUsbDeviceManager that emitted the signal
>>       * @device: #SpiceUsbDevice boxed object corresponding to the added
>> device
>> diff --git a/src/usb-device-widget.c b/src/usb-device-widget.c
>> index fe983c9..0866adb 100644
>> --- a/src/usb-device-widget.c
>> +++ b/src/usb-device-widget.c
>> @@ -72,6 +72,7 @@ struct _SpiceUsbDeviceWidgetPrivate {
>>      gchar *device_format_string;
>>      SpiceUsbDeviceManager *manager;
>>      GtkWidget *info_bar;
>> +    GtkWidget *label;
>>      gchar *err_msg;
>>      gsize device_count;
>>  };
>> @@ -181,9 +182,8 @@ static GObject *spice_usb_device_widget_constructor(
>>      SpiceUsbDeviceWidgetPrivate *priv;
>>      GPtrArray *devices = NULL;
>>      GError *err = NULL;
>> -    GtkWidget *label;
>> -    gchar *str;
>> -    int i;
>> +    gchar *str, *markup_str;
>> +    int i, free_channels;
>>  
>>      {
>>          /* Always chain up to the parent constructor */
>> @@ -197,12 +197,12 @@ static GObject *spice_usb_device_widget_constructor(
>>      if (!priv->session)
>>          g_error("SpiceUsbDeviceWidget constructed without a session");
>>  
>> -    label = gtk_label_new(NULL);
>> -    str = g_strdup_printf("<b>%s</b>", _("Select USB devices to redirect"));
>> -    gtk_label_set_markup(GTK_LABEL (label), str);
>> -    g_free(str);
>> -    gtk_misc_set_alignment(GTK_MISC(label), 0.0, 0.5);
>> -    gtk_box_pack_start(GTK_BOX(self), label, FALSE, FALSE, 0);
>> +    priv->label = gtk_label_new(NULL);
>> +    markup_str = g_strdup_printf("<b>%s</b>", _("Select USB devices to
>> redirect"));
>> +    gtk_label_set_markup(GTK_LABEL (priv->label), markup_str);
>> +    g_free(markup_str);
>> +    gtk_misc_set_alignment(GTK_MISC(priv->label), 0.0, 0.5);
>> +    gtk_box_pack_start(GTK_BOX(self), priv->label, FALSE, FALSE, 0);
>>  
>>      priv->manager = spice_usb_device_manager_get(priv->session, &err);
>>      if (err) {
>> @@ -213,6 +213,14 @@ static GObject *spice_usb_device_widget_constructor(
>>          return obj;
>>      }
>>  
>> +    g_object_get(priv->manager, "free-channels", &free_channels, NULL);
>> +    str = g_strdup_printf(_("Select USB devices to redirect (%d channels
>> free)"),
>
>This doesn't work very well with all values of %d. For instance, in english, "1
>channels" isn't correct. We probably should use ngettext here.
>
>> +                          free_channels);
>> +    markup_str = g_strdup_printf("<b>%s</b>", str);
>> +    gtk_label_set_markup(GTK_LABEL (priv->label), markup_str);
>> +    g_free(markup_str);
>> +    g_free(str);
>> +
>>      g_signal_connect(priv->manager, "device-added",
>>                       G_CALLBACK(device_added_cb), self);
>>      g_signal_connect(priv->manager, "device-removed",
>> @@ -463,6 +471,8 @@ static void checkbox_clicked_cb(GtkWidget *check, gpointer
>> user_data)
>>      SpiceUsbDeviceWidget *self = SPICE_USB_DEVICE_WIDGET(user_data);
>>      SpiceUsbDeviceWidgetPrivate *priv = self->priv;
>>      SpiceUsbDevice *device;
>> +    gchar *str, *markup_str;
>> +    int free_channels;
>>  
>>      device = g_object_get_data(G_OBJECT(check), "usb-device");
>>  
>> @@ -479,6 +489,15 @@ static void checkbox_clicked_cb(GtkWidget *check,
>> gpointer user_data)
>>          spice_usb_device_manager_disconnect_device(priv->manager,
>>                                                     device);
>>      }
>> +
>> +    g_object_get(priv->manager, "free-channels", &free_channels, NULL);
>> +    str = g_strdup_printf(_("Select USB devices to redirect (%d channels
>> free)"),
>> +                          free_channels);
>> +    markup_str = g_strdup_printf("<b>%s</b>", str);
>> +    gtk_label_set_markup(GTK_LABEL (priv->label), markup_str);
>> +    g_free(markup_str);
>> +    g_free(str);
>> +
>>      spice_usb_device_widget_update_status(self);
>
>Here we're basically duplicating what you did in the constructor. What about
>moving this into the spice_usb_device_widget_update_status() function so that
>it's not duplicated?
>
>>  }
>>  
>
>Reviewed-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
>
>
>
>------------------------------
>
>Message: 5
>Date: Thu, 21 Jan 2016 13:49:51 -0600
>From: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
>To: Fabiano Fidêncio <fidencio@xxxxxxxxxx>,
>	spice-devel@xxxxxxxxxxxxxxxxxxxxx
>Subject: Re: [Spice-devel] [PATCH v2 2/2] usb-device-{manager,
>	widget}: Remove misleading message when there are no channels to
>	redirect
>Message-ID: <1453405791.14354.129.camel@xxxxxxxxxx>
>Content-Type: text/plain; charset="UTF-8"
>
>On Thu, 2016-01-21 at 19:09 +0100, Fabiano Fidêncio wrote:
>> The whole code from usb-device-manager is quite a mess as pointed out by
>
>Perhaps you could use a slightly gentler description :)
>
>> Jonathon during the first round of code review [0].
>> Keeping this in mind and knowing that that code needs a refactor, I'm
>> proposing a simple (and messy) fix that can be backported easily before
>> actually dig into a code refactoring task.
>> The solution is basically adding a new error and using it to filter out
>> this specific kind out situation, avoiding to show that misleading
>> warning message when the last usb redirection channel is taken.
>> 
>> [0]:
>> http://lists.freedesktop.org/archives/spice-devel/2016-January/025864.html
>> 
>> Related: rhbz#1298772
>> ---
>>  src/spice-client.h       | 1 +
>>  src/usb-device-manager.c | 4 +++-
>>  src/usb-device-widget.c  | 6 ++++++
>>  3 files changed, 10 insertions(+), 1 deletion(-)
>> 
>> diff --git a/src/spice-client.h b/src/spice-client.h
>> index 32b79ea..ea7a557 100644
>> --- a/src/spice-client.h
>> +++ b/src/spice-client.h
>> @@ -82,6 +82,7 @@ typedef enum
>>      SPICE_CLIENT_ERROR_AUTH_NEEDS_USERNAME,
>>      SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD_AND_USERNAME,
>>      SPICE_CLIENT_ERROR_USB_SERVICE,
>> +    SPICE_CLIENT_ERROR_NO_USB_CHANNELS_AVAILABLE,
>>  } SpiceClientError;
>>  
>>  #ifndef SPICE_DISABLE_DEPRECATED
>> diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c
>> index 9e0e785..f40cc67 100644
>> --- a/src/usb-device-manager.c
>> +++ b/src/usb-device-manager.c
>> @@ -1701,7 +1701,9 @@
>> spice_usb_device_manager_can_redirect_device(SpiceUsbDeviceManager  *self,
>>              break;
>>      }
>>      if (i == priv->channels->len) {
>> -        g_set_error_literal(err, SPICE_CLIENT_ERROR,
>> SPICE_CLIENT_ERROR_FAILED,
>> +        g_set_error_literal(err,
>> +                            SPICE_CLIENT_ERROR,
>> +                            SPICE_CLIENT_ERROR_NO_USB_CHANNELS_AVAILABLE,
>>                              _("There are no free USB channels"));
>>          return FALSE;
>>      }
>> diff --git a/src/usb-device-widget.c b/src/usb-device-widget.c
>> index 0866adb..544ceaa 100644
>> --- a/src/usb-device-widget.c
>> +++ b/src/usb-device-widget.c
>> @@ -386,6 +386,11 @@ static void check_can_redirect(GtkWidget *widget,
>> gpointer user_data)
>>                                                                  device,
>> &err);
>>      gtk_widget_set_sensitive(widget, can_redirect);
>>  
>> +    if (g_error_matches(err,
>> +                        SPICE_CLIENT_ERROR,
>> +                        SPICE_CLIENT_ERROR_NO_USB_CHANNELS_AVAILABLE))
>> +        goto end;
>> +
>>      /* If we cannot redirect this device, append the error message to
>>         err_msg, but only if it is *not* already there! */
>>      if (!can_redirect) {
>> @@ -402,6 +407,7 @@ static void check_can_redirect(GtkWidget *widget, gpointer
>> user_data)
>>          }
>>      }
>>  
>> +end:
>>      g_clear_error(&err);
>>  }
>>  
>
>
>As a downstream patch, I think this is perfectly fine. I'm not convinced that we
>should commit it upstream though. What do you think?
>
>Reviewed-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
>
>
>------------------------------
>
>Message: 6
>Date: Thu, 21 Jan 2016 20:57:49 +0100
>From: Fabiano Fidêncio <fidencio@xxxxxxxxxx>
>To: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
>Cc: spice-devel <spice-devel@xxxxxxxxxxxxxxxxxxxxx>
>Subject: Re: [Spice-devel] [PATCH v2 2/2] usb-device-{manager,
>	widget}: Remove misleading message when there are no channels to
>	redirect
>Message-ID:
>	<CAAY6XseC6XLkj5tuKNDdSccCF55=UxekxD72E7ZWfCoF6uTiOA@xxxxxxxxxxxxxx>
>Content-Type: text/plain; charset=UTF-8
>
>On Thu, Jan 21, 2016 at 8:49 PM, Jonathon Jongsma <jjongsma@xxxxxxxxxx> wrote:
>> On Thu, 2016-01-21 at 19:09 +0100, Fabiano Fidêncio wrote:
>>> The whole code from usb-device-manager is quite a mess as pointed out by
>>
>> Perhaps you could use a slightly gentler description :)
>>
>>> Jonathon during the first round of code review [0].
>>> Keeping this in mind and knowing that that code needs a refactor, I'm
>>> proposing a simple (and messy) fix that can be backported easily before
>>> actually dig into a code refactoring task.
>>> The solution is basically adding a new error and using it to filter out
>>> this specific kind out situation, avoiding to show that misleading
>>> warning message when the last usb redirection channel is taken.
>>>
>>> [0]:
>>> http://lists.freedesktop.org/archives/spice-devel/2016-January/025864.html
>>>
>>> Related: rhbz#1298772
>>> ---
>>>  src/spice-client.h       | 1 +
>>>  src/usb-device-manager.c | 4 +++-
>>>  src/usb-device-widget.c  | 6 ++++++
>>>  3 files changed, 10 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/spice-client.h b/src/spice-client.h
>>> index 32b79ea..ea7a557 100644
>>> --- a/src/spice-client.h
>>> +++ b/src/spice-client.h
>>> @@ -82,6 +82,7 @@ typedef enum
>>>      SPICE_CLIENT_ERROR_AUTH_NEEDS_USERNAME,
>>>      SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD_AND_USERNAME,
>>>      SPICE_CLIENT_ERROR_USB_SERVICE,
>>> +    SPICE_CLIENT_ERROR_NO_USB_CHANNELS_AVAILABLE,
>>>  } SpiceClientError;
>>>
>>>  #ifndef SPICE_DISABLE_DEPRECATED
>>> diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c
>>> index 9e0e785..f40cc67 100644
>>> --- a/src/usb-device-manager.c
>>> +++ b/src/usb-device-manager.c
>>> @@ -1701,7 +1701,9 @@
>>> spice_usb_device_manager_can_redirect_device(SpiceUsbDeviceManager  *self,
>>>              break;
>>>      }
>>>      if (i == priv->channels->len) {
>>> -        g_set_error_literal(err, SPICE_CLIENT_ERROR,
>>> SPICE_CLIENT_ERROR_FAILED,
>>> +        g_set_error_literal(err,
>>> +                            SPICE_CLIENT_ERROR,
>>> +                            SPICE_CLIENT_ERROR_NO_USB_CHANNELS_AVAILABLE,
>>>                              _("There are no free USB channels"));
>>>          return FALSE;
>>>      }
>>> diff --git a/src/usb-device-widget.c b/src/usb-device-widget.c
>>> index 0866adb..544ceaa 100644
>>> --- a/src/usb-device-widget.c
>>> +++ b/src/usb-device-widget.c
>>> @@ -386,6 +386,11 @@ static void check_can_redirect(GtkWidget *widget,
>>> gpointer user_data)
>>>                                                                  device,
>>> &err);
>>>      gtk_widget_set_sensitive(widget, can_redirect);
>>>
>>> +    if (g_error_matches(err,
>>> +                        SPICE_CLIENT_ERROR,
>>> +                        SPICE_CLIENT_ERROR_NO_USB_CHANNELS_AVAILABLE))
>>> +        goto end;
>>> +
>>>      /* If we cannot redirect this device, append the error message to
>>>         err_msg, but only if it is *not* already there! */
>>>      if (!can_redirect) {
>>> @@ -402,6 +407,7 @@ static void check_can_redirect(GtkWidget *widget, gpointer
>>> user_data)
>>>          }
>>>      }
>>>
>>> +end:
>>>      g_clear_error(&err);
>>>  }
>>>
>>
>>
>> As a downstream patch, I think this is perfectly fine. I'm not convinced that we
>> should commit it upstream though. What do you think?
>
>Works for me.
>
>>
>> Reviewed-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
>
>
>------------------------------
>
>Subject: Digest Footer
>
>_______________________________________________
>Spice-devel mailing list
>Spice-devel@xxxxxxxxxxxxxxxxxxxxx
>http://lists.freedesktop.org/mailman/listinfo/spice-devel
>
>
>------------------------------
>
>End of Spice-devel Digest, Vol 72, Issue 136
>********************************************


 

_______________________________________________
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]