[Spice-devel] [PATCH] usbredirhost: host should not be marked as claimed on failure

[Qiu Wenbo]

You are right. I have changed my patch.

diff --git a/usbredirhost/usbredirhost.c b/usbredirhost/usbredirhost.c
index 3666227..609cf09 100644
--- a/usbredirhost/usbredirhost.c
+++ b/usbredirhost/usbredirhost.c
@@ -557,12 +557,20 @@ static int usbredirhost_claim(struct usbredirhost 
*host, int initial_claim)
                 ERROR("could not claim interface %d (configuration 
%d): %s",
                       n, host->config->bConfigurationValue,
                       libusb_error_name(r));
-            return libusb_status_or_error_to_redir_status(host, r);
+            goto claim_failed;
         }
     }

     usbredirhost_parse_config(host);
     return usb_redir_success;
+
+claim_failed:
+    for (i--; i >= 0; i--) {
+        n = host->config->interface[i].altsetting[0].bInterfaceNumber;
+        libusb_release_interface(host->handle, n);
+    }
+    host->claimed = 0;
+    return libusb_status_or_error_to_redir_status(host, r);
 }

 /* Called from open/close and parser read callbacks */


On Tue, May 15, 2018 at 9:34 PM, Christophe Fergeau 
<cfergeau at redhat.com> wrote:
> Hey,
> 
> Sorry for the delay in looking at your patch,
> 
> On Sat, May 12, 2018 at 12:00:26PM +0800, Qiu Wenbo wrote:
>>  This is a patch trying to fix a bug caused by usbredir. The problem 
>> is when
>>  usbredir failed to claim all interface of a USB device it won't set
>>  host->claimed to 0.
> 
> The patch makes sense to me, however I was wondering, if usbredir 
> fails
> to claim (for example) the 3rd interface on the USB device, should
> usbredir do something to release the 1st and 2nd interfaces it
> successfully claimed? Or is it fine to leave the device in a half
> claimed/half unclaimed state as long as host->claimed is 0?
> 
> Christophe
> 
>>  After that, usbredirhost_release run by
>>  usbredirhost_close will detach kernel driver and do a device reset 
>> even some
>>  one is still using the USB device. Thus the second attempt will 
>> success
>>  because device is released already. Some buggy USB device will be 
>> unusable
>>  in such situation and require a replug.
>> 
>>  To reproduce it:
>> 
>>  1. launch a virtual machine and redirect a USB device to it
>>  2. launch another virtual machine and redirect the same USB device 
>> to
>>  it，the spice client will show you a message of "Device is busy" 
>> means the
>>  device is already used by the virtual machine launched in step 1
>>  3. do step 2 again, this time the same USB device will redirect 
>> successfully
>> 
>>  We have tested it on the latest version of remote-viewer and 
>> virt-viewer.
>> 
>>  On Wed, May 9, 2018 at 8:40 AM, Qiu Wenbo <qiuwenbo at kylinos.com.cn> 
>> wrote:
>>  > You can redirect a USB device which is already redirected to 
>> another
>>  > virtual machine on the second attempt.
>>  >
>>  > Signed-off-by: Qiu Wenbo <qiuwenbo at kylinos.com.cn>
>>  > ---
>>  >  usbredirhost/usbredirhost.c | 1 +
>>  >  1 file changed, 1 insertion(+)
>>  >
>>  > diff --git a/usbredirhost/usbredirhost.c 
>> b/usbredirhost/usbredirhost.c
>>  > index 3666227..e96d980 100644
>>  > --- a/usbredirhost/usbredirhost.c
>>  > +++ b/usbredirhost/usbredirhost.c
>>  > @@ -557,6 +557,7 @@ static int usbredirhost_claim(struct 
>> usbredirhost
>>  > *host, int initial_claim)
>>  >                  ERROR("could not claim interface %d 
>> (configuration %d):
>>  > %s",
>>  >                        n, host->config->bConfigurationValue,
>>  >                        libusb_error_name(r));
>>  > +            host->claimed = 0;
>>  >              return libusb_status_or_error_to_redir_status(host, 
>> r);
>>  >          }
>>  >      }
>>  > --
>>  > 2.17.0
>>  >
>>  >
>>  >
>>  > _______________________________________________
>>  > Spice-devel mailing list
>>  > Spice-devel at lists.freedesktop.org
>>  > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> 
>>  _______________________________________________
>>  Spice-devel mailing list
>>  Spice-devel at lists.freedesktop.org
>>  https://lists.freedesktop.org/mailman/listinfo/spice-devel
> 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20180521/64170547/attachment-0001.html>