Re: [Xen-devel] [PATCH V14 4/6] libxl: add pvusb API

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

George Dunlap writes ("Re: [Xen-devel] [PATCH V14 4/6] libxl: add pvusb API"):
> On Fri, Feb 19, 2016 at 10:39 AM, Chunyan Liu <cyliu@xxxxxxxx> wrote:
> > +  [...]
> 
> So I see below that you're calling this before removing things from
> xenstore, so that if any of these fail, the user can still call "xl
> usb-remove" to retry.
> 
> Unfortunately, since you reassign interfaces to the original driver
> before all interfaces are de-assigned from usbback, you can end up in
> a situation where the device is partially re-plugged into the original
> drivers, partially still assigned to pciback.
> 
> I think this whole process should be three steps:
> 
> 1. Unassign all interfaces from usbback, stopping and returning an
> error as soon as one attempt fails
> 
> 2. Remove the pvusb xenstore nodes (stopping and returning an error if it 
> fails)
> 
> 3. Attempt to re-assign all interfaces to the original drivers,
> stopping and returning an error as soon as one attempt fails.

This seems like a good plan to me.

(Making 3 after 2 re-attemptable would mean that the original driver
information needs to be saved in a different location in xenstore to
the pvusb control nodes, but that is not a problem.)

> * If one of the re-assignments fail, then the user will have to reload
> the drivers, reboot, or mess around with sysfs to fix things.
> *However*, this avoids a scenario where a user is completely unable to
> remove a device from a domain because of a buggy driver.

Right.

> I realize this falls short of the "crash-only" design IanJ suggested
> we try to do, but I think that in this case the work required to do
> such a design would be a lot more work than the benefit it gives us.

I think what you have above is indeed crash-only.  You can tell by all
the "if any error occurs, stop immediately".

The only wrinkle is that the obvious implementation reads the old
bindings from xenstore between 1 and 2, deletes the information from
xenstore in 2, and uses that information in step 3, which is cheating
(and leads to the sysfs-wrangling-required state).  But that is quite
easy to avoid:
  - record the old driver bindings somewhere in xenstore (keyed by
    the physical host device, not the guest domain)
  - provide a libxl call and corresponding xl command which attempts
    to rebind

But, FAOD, I do not want to block this patch until such a thing is
implemented.  I think it is sufficient to document the existence of
the awkward state, and the likely workarounds.

Ian.

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