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

[George Dunlap <George.Dunlap@eu.citrix.com>]

On Tue, Jan 26, 2016 at 7:43 AM, Chun Yan Liu <cyliu@suse.com> wrote:
>
>
>>>> On 1/20/2016 at 12:56 PM, in message
> <569F83F5020000660009E6AC@relay2.provo.novell.com>, "Chun Yan Liu"
> <cyliu@suse.com> wrote:
>
>>
>>>>> On 1/19/2016 at 11:48 PM, in message
>> <22174.23240.402164.635740@mariner.uk.xensource.com>, Ian Jackson
>> <Ian.Jackson@eu.citrix.com> wrote:
>> > Chunyan Liu writes ("[PATCH V13 3/5] libxl: add pvusb API"):
>> > > Add pvusb APIs, including:
>> > >  - attach/detach (create/destroy) virtual usb controller.
>> > >  - attach/detach usb device
>> > >  - list usb controller and usb devices
>> > >  - some other helper functions
>> >
>> >
>> > Thanks.  This is making progress but I'm afraid we're not quite there
>> > yet.
>> >
>> >
>> > > +static int usbback_dev_unassign(libxl__gc *gc, const char *busid)
>> > > +{
>> > ...
>> > > +    /* Till here, USB device has been unbound from USBBACK and
>> > > +     * removed from xenstore, usb list couldn't show it anymore,
>> > > +     * so no matter removimg driver path successfully or not,
>> > > +     * we will report operation success.
>> > > +     */
>> >
>> > I'm still unconvinced by this and this may mean that the code in this
>> > function is in the wrong order.  Earlier we had this exchange:
>> >
>> > > > Ought this function to really report success if these calls fail ?
>> > >
>> > > I think so. Till here, the USB device has already been unbound from
>> > > usbback and removed from xenstore. usb-list cannot list it any more.
>> >
>> > The problem is that I think that if this function fails, it can leave
>> >  - debris in xenstore (the usbback path)
>> Yes, it's true.
>>
>> >  - the interface bound to the wrong driver
>> No, it won't be bound to 'wrong' driver, only maybe not bound to any driver
>> (Already unbound from usbback, but failed to rebound to its original
>> driver).
>> In this case, we would report warning: failed to rebind to driver xxx.
>>
>> > And then there is no way for the user to get libxl to re-attempt the
>> > operation, or clean up.  Am I right ?
>>
>> Yes. No way to re-attempt usbdev-detach or cleanup driver path in
>> xenstore. But won't affect next time usbdev-attach the same device.
>>
>> >
>> > One way to avoid this kind of problem is to deal with the xenstore
>> > path last.  That way the device will still appear as attached to the
>> > domain.
>>
>> I'm afraid if the side effect is acceptable? In my testing, some USB
>> bluetooth
>> device always fails to rebind to 'btusb' driver after it's unbound
>> from 'usbback'. In this case, we can't detach it from the domain then.
>
> Ian J., any opinion on this? If it's still thought to be better, I'll update patch.

I think Ian may be waiting for me to reply and express an opinion; but
unfortunately that will have to wait until next week. :-(

 -George