[Spice-devel,usbredir,5/6] usbredirhost: claim/release: ignore NOT_SUPPORTED on attach|detach kernel driver

Submitted by Uri Lublin on May 3, 2012, 3:04 p.m.

Details

Message ID 1336057479-22629-6-git-send-email-uril@redhat.com
State New
Headers show

Not browsing as part of any series.

Commit Message

Uri Lublin May 3, 2012, 3:04 p.m.
On Windows libusb_(attach|detach)_kernel_driver are not supported.
A libusb driver is already installed (device was successfully opened).
In that case just continue as if operation was successful.
---
 usbredirhost/usbredirhost.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

Patch hide | download patch | download mbox

diff --git a/usbredirhost/usbredirhost.c b/usbredirhost/usbredirhost.c
index 305dd35..3d77ac8 100644
--- a/usbredirhost/usbredirhost.c
+++ b/usbredirhost/usbredirhost.c
@@ -486,7 +486,8 @@  static int usbredirhost_claim(struct usbredirhost *host, int initial_claim)
         n = host->config->interface[i].altsetting[0].bInterfaceNumber;

         r = libusb_detach_kernel_driver(host->handle, n);
-        if (r < 0 && r != LIBUSB_ERROR_NOT_FOUND) {
+        if (r < 0 && r != LIBUSB_ERROR_NOT_FOUND
+                  && r != LIBUSB_ERROR_NOT_SUPPORTED) {
             ERROR("could not detach driver from interface %d (configuration %d): %d",
                   n, host->config->bConfigurationValue, r);
             return libusb_status_or_error_to_redir_status(host, r);
@@ -544,6 +545,7 @@  static void usbredirhost_release(struct usbredirhost *host, int attach_drivers)
         r = libusb_attach_kernel_driver(host->handle, n);
         if (r < 0 && r != LIBUSB_ERROR_NOT_FOUND /* No driver */
                   && r != LIBUSB_ERROR_NO_DEVICE /* Device unplugged */
+                  && r != LIBUSB_ERROR_NOT_SUPPORTED /* Not supported */
                   && r != LIBUSB_ERROR_BUSY /* driver rebound already */) {
             ERROR("could not re-attach driver to interface %d (configuration %d): %d",
                   n, host->config->bConfigurationValue, r);

Comments

On Thu, May 03, 2012 at 06:04:38PM +0300, Uri Lublin wrote:
> On Windows libusb_(attach|detach)_kernel_driver are not supported.
> A libusb driver is already installed (device was successfully opened).
> In that case just continue as if operation was successful.

Do we want to only do that check on Windows in case this indicates real
issues on other systems? (BSDs for example)

Christophe

> ---
>  usbredirhost/usbredirhost.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/usbredirhost/usbredirhost.c b/usbredirhost/usbredirhost.c
> index 305dd35..3d77ac8 100644
> --- a/usbredirhost/usbredirhost.c
> +++ b/usbredirhost/usbredirhost.c
> @@ -486,7 +486,8 @@ static int usbredirhost_claim(struct usbredirhost *host, int initial_claim)
>          n = host->config->interface[i].altsetting[0].bInterfaceNumber;
> 
>          r = libusb_detach_kernel_driver(host->handle, n);
> -        if (r < 0 && r != LIBUSB_ERROR_NOT_FOUND) {
> +        if (r < 0 && r != LIBUSB_ERROR_NOT_FOUND
> +                  && r != LIBUSB_ERROR_NOT_SUPPORTED) {
>              ERROR("could not detach driver from interface %d (configuration %d): %d",
>                    n, host->config->bConfigurationValue, r);
>              return libusb_status_or_error_to_redir_status(host, r);
> @@ -544,6 +545,7 @@ static void usbredirhost_release(struct usbredirhost *host, int attach_drivers)
>          r = libusb_attach_kernel_driver(host->handle, n);
>          if (r < 0 && r != LIBUSB_ERROR_NOT_FOUND /* No driver */
>                    && r != LIBUSB_ERROR_NO_DEVICE /* Device unplugged */
> +                  && r != LIBUSB_ERROR_NOT_SUPPORTED /* Not supported */
>                    && r != LIBUSB_ERROR_BUSY /* driver rebound already */) {
>              ERROR("could not re-attach driver to interface %d (configuration %d): %d",
>                    n, host->config->bConfigurationValue, r);
> -- 
> 1.7.7.6
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
Hi,

On 05/03/2012 06:24 PM, Christophe Fergeau wrote:
> On Thu, May 03, 2012 at 06:04:38PM +0300, Uri Lublin wrote:
>> On Windows libusb_(attach|detach)_kernel_driver are not supported.
>> A libusb driver is already installed (device was successfully opened).
>> In that case just continue as if operation was successful.
>
> Do we want to only do that check on Windows in case this indicates real
> issues on other systems? (BSDs for example)

I don't think that will be necessary, not_supported is only returned
when something is not supported by the backend, never as a "normal"
error on backends where some operation is supported.

Regards,

Hans

>
> Christophe
>
>> ---
>>   usbredirhost/usbredirhost.c |    4 +++-
>>   1 files changed, 3 insertions(+), 1 deletions(-)
>>
>> diff --git a/usbredirhost/usbredirhost.c b/usbredirhost/usbredirhost.c
>> index 305dd35..3d77ac8 100644
>> --- a/usbredirhost/usbredirhost.c
>> +++ b/usbredirhost/usbredirhost.c
>> @@ -486,7 +486,8 @@ static int usbredirhost_claim(struct usbredirhost *host, int initial_claim)
>>           n = host->config->interface[i].altsetting[0].bInterfaceNumber;
>>
>>           r = libusb_detach_kernel_driver(host->handle, n);
>> -        if (r<  0&&  r != LIBUSB_ERROR_NOT_FOUND) {
>> +        if (r<  0&&  r != LIBUSB_ERROR_NOT_FOUND
>> +&&  r != LIBUSB_ERROR_NOT_SUPPORTED) {
>>               ERROR("could not detach driver from interface %d (configuration %d): %d",
>>                     n, host->config->bConfigurationValue, r);
>>               return libusb_status_or_error_to_redir_status(host, r);
>> @@ -544,6 +545,7 @@ static void usbredirhost_release(struct usbredirhost *host, int attach_drivers)
>>           r = libusb_attach_kernel_driver(host->handle, n);
>>           if (r<  0&&  r != LIBUSB_ERROR_NOT_FOUND /* No driver */
>>                     &&  r != LIBUSB_ERROR_NO_DEVICE /* Device unplugged */
>> +&&  r != LIBUSB_ERROR_NOT_SUPPORTED /* Not supported */
>>                     &&  r != LIBUSB_ERROR_BUSY /* driver rebound already */) {
>>               ERROR("could not re-attach driver to interface %d (configuration %d): %d",
>>                     n, host->config->bConfigurationValue, r);
>> --
>> 1.7.7.6
>>
>> _______________________________________________
>> Spice-devel mailing list
>> Spice-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>>
>>
>> _______________________________________________
>> Spice-devel mailing list
>> Spice-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/spice-devel
On Thu, May 3, 2012 at 11:04 PM, Uri Lublin <uril@redhat.com> wrote:
> On Windows libusb_(attach|detach)_kernel_driver are not supported.
> A libusb driver is already installed (device was successfully opened).
> In that case just continue as if operation was successful.
> ---
>  usbredirhost/usbredirhost.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/usbredirhost/usbredirhost.c b/usbredirhost/usbredirhost.c
> index 305dd35..3d77ac8 100644
> --- a/usbredirhost/usbredirhost.c
> +++ b/usbredirhost/usbredirhost.c
> @@ -486,7 +486,8 @@ static int usbredirhost_claim(struct usbredirhost *host, int initial_claim)
>         n = host->config->interface[i].altsetting[0].bInterfaceNumber;
>
>         r = libusb_detach_kernel_driver(host->handle, n);
> -        if (r < 0 && r != LIBUSB_ERROR_NOT_FOUND) {
> +        if (r < 0 && r != LIBUSB_ERROR_NOT_FOUND
> +                  && r != LIBUSB_ERROR_NOT_SUPPORTED) {
>             ERROR("could not detach driver from interface %d (configuration %d): %d",
>                   n, host->config->bConfigurationValue, r);
>             return libusb_status_or_error_to_redir_status(host, r);
> @@ -544,6 +545,7 @@ static void usbredirhost_release(struct usbredirhost *host, int attach_drivers)
>         r = libusb_attach_kernel_driver(host->handle, n);
>         if (r < 0 && r != LIBUSB_ERROR_NOT_FOUND /* No driver */
>                   && r != LIBUSB_ERROR_NO_DEVICE /* Device unplugged */
> +                  && r != LIBUSB_ERROR_NOT_SUPPORTED /* Not supported */
>                   && r != LIBUSB_ERROR_BUSY /* driver rebound already */) {
>             ERROR("could not re-attach driver to interface %d (configuration %d): %d",
>                   n, host->config->bConfigurationValue, r);
> --
> 1.7.7.6

To me the above is not the right fix, the right fix is to limit the call
libusb_detach_kernel_driver() to Linux only. Do not use this
call for other systems since all the other OS does not support this
call -- be it Mac OS X, Windows, or BSDs.
Hi Xiaofan,

I did not know you where following this list too :)

On 05/04/2012 09:04 AM, Xiaofan Chen wrote:
> On Thu, May 3, 2012 at 11:04 PM, Uri Lublin<uril@redhat.com>  wrote:
>> On Windows libusb_(attach|detach)_kernel_driver are not supported.
>> A libusb driver is already installed (device was successfully opened).
>> In that case just continue as if operation was successful.
>> ---
>>   usbredirhost/usbredirhost.c |    4 +++-
>>   1 files changed, 3 insertions(+), 1 deletions(-)
>>
>> diff --git a/usbredirhost/usbredirhost.c b/usbredirhost/usbredirhost.c
>> index 305dd35..3d77ac8 100644
>> --- a/usbredirhost/usbredirhost.c
>> +++ b/usbredirhost/usbredirhost.c
>> @@ -486,7 +486,8 @@ static int usbredirhost_claim(struct usbredirhost *host, int initial_claim)
>>          n = host->config->interface[i].altsetting[0].bInterfaceNumber;
>>
>>          r = libusb_detach_kernel_driver(host->handle, n);
>> -        if (r<  0&&  r != LIBUSB_ERROR_NOT_FOUND) {
>> +        if (r<  0&&  r != LIBUSB_ERROR_NOT_FOUND
>> +&&  r != LIBUSB_ERROR_NOT_SUPPORTED) {
>>              ERROR("could not detach driver from interface %d (configuration %d): %d",
>>                    n, host->config->bConfigurationValue, r);
>>              return libusb_status_or_error_to_redir_status(host, r);
>> @@ -544,6 +545,7 @@ static void usbredirhost_release(struct usbredirhost *host, int attach_drivers)
>>          r = libusb_attach_kernel_driver(host->handle, n);
>>          if (r<  0&&  r != LIBUSB_ERROR_NOT_FOUND /* No driver */
>>                    &&  r != LIBUSB_ERROR_NO_DEVICE /* Device unplugged */
>> +&&  r != LIBUSB_ERROR_NOT_SUPPORTED /* Not supported */
>>                    &&  r != LIBUSB_ERROR_BUSY /* driver rebound already */) {
>>              ERROR("could not re-attach driver to interface %d (configuration %d): %d",
>>                    n, host->config->bConfigurationValue, r);
>> --
>> 1.7.7.6
>
> To me the above is not the right fix, the right fix is to limit the call
> libusb_detach_kernel_driver() to Linux only. Do not use this
> call for other systems since all the other OS does not support this
> call -- be it Mac OS X, Windows, or BSDs.

I agree that an #ifdef __linux__ would be a good fix too. Note that the NOT_SUPPORTED
check will work as well, but you're right that we might just as well #ifdef out the
code.

Regards,

Hans
On Fri, May 4, 2012 at 3:09 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi Xiaofan,
>
> I did not know you where following this list too :)

:-)

>> To me the above is not the right fix, the right fix is to limit the call
>> libusb_detach_kernel_driver() to Linux only. Do not use this
>> call for other systems since all the other OS does not support this
>> call -- be it Mac OS X, Windows, or BSDs.
>
> I agree that an #ifdef __linux__ would be a good fix too. Note that the
> NOT_SUPPORTED check will work as well, but you're right that we
> might just as well #ifdef out the code.

Thanks. Yes the fix Uri proposed will also work.

On the other hand, at least for the codes using libusb-0.1, ifdef is
the norm since libusb-win32 does not have the driver detach function
at all. In the cases of  libusb-1.0/libusbx, then the function is there
for platforms other than Linux but it return unsupported error.