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 <cy...@suse.com> 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@lists.xen.org http://lists.xen.org/xen-devel