[Qemu-devel] [PATCH] Re-attach usb device to kernel while usb_host_open fails

[Qemu-devel] [PATCH] Re-attach usb device to kernel while usb_host_open fails

[Lin Ma]

Signed-off-by: Lin Ma <lma@suse.com>
---
 hw/usb/host-libusb.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
index 10f4735..7258c4d 100644
--- a/hw/usb/host-libusb.c
+++ b/hw/usb/host-libusb.c
@@ -888,6 +888,11 @@ static int usb_host_open(USBHostDevice *s, libusb_device *dev)
 fail:
     trace_usb_host_open_failure(bus_num, addr);
     if (s->dh != NULL) {
+        qemu_remove_exit_notifier(&s->exit);
+        QTAILQ_REMOVE(&hostdevs, s, next);
+        usb_host_release_interfaces(s);
+        libusb_reset_device(s->dh);
+        usb_host_attach_kernel(s);
         libusb_close(s->dh);
         s->dh = NULL;
         s->dev = NULL;
-- 
2.1.4

Re: [Qemu-devel] [PATCH] Re-attach usb device to kernel while usb_host_open fails

[Gonglei]

On 2015/6/23 12:24, Lin Ma wrote:
> Signed-off-by: Lin Ma <lma@suse.com>
> ---
>  hw/usb/host-libusb.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
> index 10f4735..7258c4d 100644
> --- a/hw/usb/host-libusb.c
> +++ b/hw/usb/host-libusb.c
> @@ -888,6 +888,11 @@ static int usb_host_open(USBHostDevice *s, libusb_device *dev)
>  fail:
>      trace_usb_host_open_failure(bus_num, addr);
>      if (s->dh != NULL) {
> +        qemu_remove_exit_notifier(&s->exit);
> +        QTAILQ_REMOVE(&hostdevs, s, next);

This change will cause a regression. For example, if an usb device's
(assume that it's usb1.0 device) speed does not match the ehci adapter,
then then invoking usb_host_open failed. if somebody changes the usb
device to an usb 2.0 device, it will not auto check because the device has
removed from the global hostdevs list.

So I think we don't need  do the above operations.

Regards,
-Gonglei

> +        usb_host_release_interfaces(s);
> +        libusb_reset_device(s->dh);
> +        usb_host_attach_kernel(s);
>          libusb_close(s->dh);
>          s->dh = NULL;
>          s->dev = NULL;
> 

Re: [Qemu-devel] [PATCH] Re-attach usb device to kernel while usb_host_open fails

[Lin Ma]

在 2015年06月23日 16:29, Gonglei 写道:
> On 2015/6/23 12:24, Lin Ma wrote:
>> Signed-off-by: Lin Ma <lma@suse.com>
>> ---
>>   hw/usb/host-libusb.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
>> index 10f4735..7258c4d 100644
>> --- a/hw/usb/host-libusb.c
>> +++ b/hw/usb/host-libusb.c
>> @@ -888,6 +888,11 @@ static int usb_host_open(USBHostDevice *s, libusb_device *dev)
>>   fail:
>>       trace_usb_host_open_failure(bus_num, addr);
>>       if (s->dh != NULL) {
>> +        qemu_remove_exit_notifier(&s->exit);
>> +        QTAILQ_REMOVE(&hostdevs, s, next);
> This change will cause a regression. For example, if an usb device's
> (assume that it's usb1.0 device) speed does not match the ehci adapter,
> then then invoking usb_host_open failed. if somebody changes the usb
> device to an usb 2.0 device, it will not auto check because the device has
> removed from the global hostdevs list.
>
> So I think we don't need  do the above operations.
In this situation, in my opinion, User should perform device_add to add 
the device again, Then the device will be added
to hostdevs list and can be auto checked again, right?

In addition, The usb_host_open will be performed 3 times until 
's->errcount >= 3' while invoking usb_host_open failed.
If the usb device isn't removed from hostdevs list, It will be detached 
& re-attached 3 times.


Thanks,
Lin
>> +        usb_host_release_interfaces(s);
>> +        libusb_reset_device(s->dh);
>> +        usb_host_attach_kernel(s);
>>           libusb_close(s->dh);
>>           s->dh = NULL;
>>           s->dev = NULL;
>>
>
>

Re: [Qemu-devel] [PATCH] Re-attach usb device to kernel while usb_host_open fails

[Gonglei]

On 2015/6/23 22:49, Lin Ma wrote:
> 
> 在 2015年06月23日 16:29, Gonglei 写道:
>> On 2015/6/23 12:24, Lin Ma wrote:
>>> Signed-off-by: Lin Ma <lma@suse.com>
>>> ---
>>>   hw/usb/host-libusb.c | 5 +++++
>>>   1 file changed, 5 insertions(+)
>>>
>>> diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
>>> index 10f4735..7258c4d 100644
>>> --- a/hw/usb/host-libusb.c
>>> +++ b/hw/usb/host-libusb.c
>>> @@ -888,6 +888,11 @@ static int usb_host_open(USBHostDevice *s, libusb_device *dev)
>>>   fail:
>>>       trace_usb_host_open_failure(bus_num, addr);
>>>       if (s->dh != NULL) {
>>> +        qemu_remove_exit_notifier(&s->exit);
>>> +        QTAILQ_REMOVE(&hostdevs, s, next);
>> This change will cause a regression. For example, if an usb device's
>> (assume that it's usb1.0 device) speed does not match the ehci adapter,
>> then then invoking usb_host_open failed. if somebody changes the usb
>> device to an usb 2.0 device, it will not auto check because the device has
>> removed from the global hostdevs list.
>>
>> So I think we don't need  do the above operations.
> In this situation, in my opinion, User should perform device_add to add the device again, Then the device will be added
> to hostdevs list and can be auto checked again, right?
> 
No, that's a regression if you ask users call device_add again.
The purpose that we use bus/port to identify one usb device is
can handle this scenario. After passing through one usb device by
usb/port, the guest can auto check the device though you changed a usb
device in the same physical port.

> In addition, The usb_host_open will be performed 3 times until 's->errcount >= 3' while invoking usb_host_open failed.
> If the usb device isn't removed from hostdevs list, It will be detached & re-attached 3 times.
> 
Yes, that's no problem as a protecting logic.

Regards,
-Gonglei
> 
> Thanks,
> Lin
>>> +        usb_host_release_interfaces(s);
>>> +        libusb_reset_device(s->dh);
>>> +        usb_host_attach_kernel(s);
>>>           libusb_close(s->dh);
>>>           s->dh = NULL;
>>>           s->dev = NULL;
>>>
>>
>>
>