Re: [Xen-devel] [PATCH V13 3/5] libxl: add pvusb API

[Juergen Gross <jgross@xxxxxxxx>]

On 04/02/16 02:53, Chun Yan Liu wrote:
> 
> 
>>>> On 2/3/2016 at 10:33 PM, in message <56B20FCC.3010308@xxxxxxxxxx>, George
> Dunlap <george.dunlap@xxxxxxxxxx> wrote: 
>> On 02/02/16 18:11, Ian Jackson wrote: 
>>> George Dunlap writes ("Re: [Xen-devel] [PATCH V13 3/5] libxl: add pvusb  
>> API"): 
>>>> There are effectively four states a device can be in, from the 
>>>> 'assignment' point of view: 
>>>>
>>>> 1. Assigned to the normal Linux device driver(s) for the USB device 
>>>>
>>>> 2. Assigned to no driver 
>>>>
>>>> 3. Assigned to usbback, but not yet assigned to any guest 
>>>>
>>>> 4. Assigned to a guest 
>>>  
>>> Thanks for your clear explanation (of which I will snip much). 
>>>  
>>>> Additionally, each USB "device" has one or more "interfaces".  To 
>>>> assign a "device" to usbback in the sysfs case means assigning all of 
>>>> the "interfaces".  The code seems to assume that different interfaces 
>>>> from the same device can be assigned to different drivers. 
>>>  
>>> It is indeed the case that in principle a single USB device with 
>>> multiple interfaces can be assigned to multiple different drivers. 
>>>  
>>>> Regarding Ian's comments: 
>>>>
>>>> Since "assigned to the guest" and "listed under the pvusb xenstore 
>>>> node" are the same thing, is it even *possible* to (safely) unassign 
>>>> the device from usbback before removing the xenstore nodes? 
>>>  
>>> It might be possible to remove some of the xenstore nodes but leave 
>>> others present, so that usbback detaches, but enough information 
>>> remains for libxl to know that Xen still `owns' the device. 
>>>  
>>> But, surely usbback needs to cope with the notion that the device 
>>> might disappear.  USB devices can disappear at any time. 
>>  
>> That's true. But if the USB device has actually disappeared, the device 
>> is in fact "safe" from usbback.  I think I might consider state 2 safe 
>> to go to, but I definitely wouldn't consider state 1 safe with the 
>> xenstore nodes still in place. 
>>  
>>>  
>>>> It's not clear to me under what conditions 3->2 might fail, 
>>>  
>>> As an example, someone might press ^C on `xl usb-detach'. 
>>  
>> *grumble* 
>>  
>>>> or what could be done about it if it did fail. 
>>>  
>>> The most obvious reason for it failing is that something somewhere 
>>> still held onto the device.  (For umounting filesystems, and detaching 
>>> block devices, this happens a lot.)  So if the detach from usbback 
>>> fails would probably be possible to simply retry it. 
>>  
>> I'm not sure what this means in the context of moving from 3 (assigned 
>> to usbback) to 2 (unassigned).  usbback will definitely not use the 
>> device to mount something in dom0; and I'm pretty sure has no visibility 
>> as to whether it's being used as a filesystem in the domU. 
>>  
>> (Moving from 1 -> 2 of course would be subject to this sort of thing, 
>> but that's a different issue.) 
>>  
>>>> Regarding 2->1, again it's not clear that there's anything libxl can 
>>>> do.  Reloading the driver's module might reset it enough to pick up 
>>>> the (now unassigned) USB devices again; other than that, rebooting is 
>>>> probably the best option. 
>>>  
>>> I think re-attempting the bind may work.  USB devices can be flaky. 
>>>  
>>>> It's also not clear to me, if some functions succeed in being 
>>>> reassigned and others fail, whether it's best to just try to assign as 
>>>> many as we can, or better to go back and un-assign all the ones that 
>>>> succeeded. 
>>>  
>>> Unless explicitly requested, I don't think the system should create 
>>> situations some interfaces are assigned to host drivers and some to 
>>> usbback. 
>>  
>> I was talking here about whether it would be better to have some 
>> interfaces assigned to the original drivers and some interfaces left 
>> unassigned, or to try to leave all interfaces unassigned if any of the 
>> assignments fail. 
>>  
>> I agree, it would be better to try to avoid the possibility of having 
>> some interfaces bound to usbback and some interfaces bound to the 
>> original drivers. 
>>  
>>> And, I'm a fan of `crash-only software': in general, if a failure 
>>> occurs, the situation should just be left as-is.  The intermediate 
>>> state needs to be visible and rectifiable. 
>>>  
>>> This approach to software state machines avoids bugs where the system 
>>> gets into `impossible' states, recovery from which is impossible 
>>> because the designers didn't anticipate them. 
>>>  
>>> It would be tolerable if the recovery sometimes involves `lsusb' and 
>>> echoing things into sysfs, but it would be better if not. 
>>  
>> Right -- so what about this.  When removing a USB device: 
>>  
>> * First modify the pvusb xenstore nodes such that 1) it's safe to 
>> attempt removing the interfaces from usbback, but 2) they still show up 
>> in usb-list.  (This may be a noop.) 
>>  
>> * Attempt to remove all interfaces from usbback; if any of these fail, 
>> stop and report an error.  Possible recovery options: 
>>  - Re-try the usb_remove command 
>>  - Re-load usbback (obviously disruptive to other VMs) 
>>  - Reboot the host 
>>  - Manually try unbinding with sysfs 
>>  
>> * Remove the remaining pvusb xenstore nodes.  If this fails, stop and 
>> report the error.  Possible recovery options: 
>>  - Re-try the usb_remove command 
>>  
>> = REBIND OPTION 1: Do the best we can without extra commands 
>>  
>> * Attempt to re-bind the interfaces to the original drivers, as recorded 
>> in the libxl usb xenstore nodes.  If any of these fail, report an error 
>> but continue to try the rest of the interfaces, and remove the xenstore 
>> nodes containing information about the original drivers.  Possible 
>> recovery options for the user: 
>>  - Reload the original drivers 
>>  - Reboot the host 
>>  - Manually try rebinding with sysfs 
>>  
>> = REBIND OPTION 2: Include a recovery command, usb-retry-rebind 
>>  
>> * Attempt to re-bind the interfaces to the original drivers, as recorded 
>> in the libxl usb xenstore nodes.  If any of these fail, stop immediately 
>> and report an error; do not remove the xenstore nodes containing the 
>> original drivers of any interfaces that failed to rebind.  Possible 
>> recovery options for the user: 
>>  - Run 'xl usbdev-retry-rebind', which will just execute this step again 
>>  - Unload and reload original host drivers 
>>  - Reboot the host 
>>  
>> Both of these: 
>>  
>> 1. Will avoid the state of some interfaces bound to usbback, some 
>> rebound to the original drivers 
>>  
>> 2. Give the user a convenient way to re-try unbinding from usbback it failed 
>>  
>> 3. Give the user out-of-xl ways to attempt to recover the state other 
>> than messing around with sysfs. 
>>  
>> Rebind option 2 will give the user a convenient way to retry rebinding 
>> to the original driver via xl if that step failed. 
>>  
>> I'm inclined to suggest rebind option #1 for now, just to keep things 
>> simple. 
>>  
>> Thoughts? 
>>  
>> Chunyan, would the first half of that (removing from usbback before 
>> removing the pvusb xenstore nodes) actually work? 
> 
> From testing, yes, it works. But I am not sure if it is safe though. Juergen
> should be very clear about that according to usbback internal codes.
> 
> Juergen, could you help to confirm that?

I think it should work.

TBH, I don't think the error handling of driver binding/unbinding should
be made too intelligent. In the long run I hope to have a qemu-based
pvUSB backend. And in this case driver binding/unbinding will be handled
completely by qemu/libusb.

Whether we want to keep the kernel based pvUSB-backend support in xl
at the end should be discussed later. I'll either add the qemu based
variant or I'll replace the kernel variant with the qemu one as soon
as the qemu backend has been accepted.


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel