[Spice-devel] Spice-devel Digest, Vol 72, Issue 136
nlx
nlxswig at 126.com
Thu Jan 21 20:12:58 PST 2016
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 at lists.freedesktop.org wrote:
>Send Spice-devel mailing list submissions to
> spice-devel at lists.freedesktop.org
>
>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 at lists.freedesktop.org
>
>You can reach the person managing the list at
> spice-devel-owner at lists.freedesktop.org
>
>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 at redhat.com>
>To: spice-devel at lists.freedesktop.org
>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 at redhat.com>
>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 at redhat.com>
>To: spice-devel at lists.freedesktop.org
>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 at redhat.com>
>
>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 at redhat.com>
>To: spice-devel at lists.freedesktop.org
>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 at redhat.com>
>
>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 at redhat.com>
>To: Fabiano Fidêncio <fidencio at redhat.com>,
> spice-devel at lists.freedesktop.org
>Subject: Re: [Spice-devel] [PATCH v2 1/2] usb-device-{manager,
> widget}: Add counter of free channels
>Message-ID: <1453405580.14354.126.camel at redhat.com>
>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 at redhat.com>
>
>
>
>------------------------------
>
>Message: 5
>Date: Thu, 21 Jan 2016 13:49:51 -0600
>From: Jonathon Jongsma <jjongsma at redhat.com>
>To: Fabiano Fidêncio <fidencio at redhat.com>,
> spice-devel at lists.freedesktop.org
>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 at redhat.com>
>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 at redhat.com>
>
>
>------------------------------
>
>Message: 6
>Date: Thu, 21 Jan 2016 20:57:49 +0100
>From: Fabiano Fidêncio <fidencio at redhat.com>
>To: Jonathon Jongsma <jjongsma at redhat.com>
>Cc: spice-devel <spice-devel at lists.freedesktop.org>
>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 at mail.gmail.com>
>Content-Type: text/plain; charset=UTF-8
>
>On Thu, Jan 21, 2016 at 8:49 PM, Jonathon Jongsma <jjongsma at redhat.com> 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 at redhat.com>
>
>
>------------------------------
>
>Subject: Digest Footer
>
>_______________________________________________
>Spice-devel mailing list
>Spice-devel at lists.freedesktop.org
>http://lists.freedesktop.org/mailman/listinfo/spice-devel
>
>
>------------------------------
>
>End of Spice-devel Digest, Vol 72, Issue 136
>********************************************
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20160122/ef2ce803/attachment-0001.html>
More information about the Spice-devel
mailing list