usbredirhost: host should not be marked as claimed on failure

Submitted by Qiu Wenbo on May 9, 2018, 12:40 a.m.

Details

Message ID 20180509004044.15156-1-qiuwenbo@kylinos.com.cn
State New
Headers show
Series "usbredirhost: host should not be marked as claimed on failure" ( rev: 2 ) in Spice

Not browsing as part of any series.

Commit Message

Qiu Wenbo May 9, 2018, 12:40 a.m.
You can redirect a USB device which is already redirected to another
virtual machine on the second attempt.

Signed-off-by: Qiu Wenbo <qiuwenbo@kylinos.com.cn>
---
 usbredirhost/usbredirhost.c | 1 +
 1 file changed, 1 insertion(+)

Patch hide | download patch | download mbox

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);
         }
     }

Comments

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. 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@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@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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
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@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@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@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel

> _______________________________________________
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel