[Spice-devel] [PATCH v2 2/2] usb-device-{manager, widget}: Remove misleading message when there are no channels to redirect

[Fabiano Fidêncio]

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>