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

["Chun Yan Liu" <cyliu@xxxxxxxx>]

>>> On 3/3/2016 at 05:20 PM, in message
<CAFLBxZaOYB0PSxxJi_mF3G6yie80oh2X_O3tJahTH1XJMJjEkA@xxxxxxxxxxxxxx>, George
Dunlap <George.Dunlap@xxxxxxxxxxxxx> wrote: 
> On Thu, Mar 3, 2016 at 3:11 AM, Chun Yan Liu <cyliu@xxxxxxxx> wrote: 
>>>>> On 3/3/2016 at 02:32 AM, in message <56D731B1.60009@xxxxxxxxxx>, George 
>>>>> Dunlap 
> > <george.dunlap@xxxxxxxxxx> wrote: 
> >> On 01/03/16 08:09, Chunyan Liu wrote: 
> >> > +static int usbdev_rebind(libxl__gc *gc, const char *busid) 
> >> > +{ 
> >> > +    char **intfs = NULL; 
> >> > +    char *usbdev_encode = NULL; 
> >> > +    char *path = NULL; 
> >> > +    int i, num = 0; 
> >> > +    int rc; 
> >> > + 
> >> > +    rc = usbdev_get_all_interfaces(gc, busid, &intfs, &num); 
> >> > +    if (rc) goto out; 
> >> > + 
> >> > +    usbdev_encode = usb_interface_xenstore_encode(gc, busid); 
> >> > + 
> >> > +    for (i = 0; i < num; i++) { 
> >> > +        char *intf = intfs[i]; 
> >> > +        char *usbintf_encode = NULL; 
> >> > +        const char *drvpath; 
> >> > + 
> >> > +        /* rebind USB interface to its originial driver */ 
> >> > +        usbintf_encode = usb_interface_xenstore_encode(gc, intf); 
> >> > +        path = GCSPRINTF(USBBACK_INFO_PATH "/%s/%s/driver_path", 
> >> > +                         usbdev_encode, usbintf_encode); 
> >> > +        rc = libxl__xs_read_checked(gc, XBT_NULL, path, &drvpath); 
> >> > +        if (rc) goto out; 
> >> > + 
> >> > +        if (drvpath) { 
> >> > +            rc = bind_usbintf(gc, intf, drvpath); 
> >> > +            if (rc) { 
> >> > +                LOGE(ERROR, "Couldn't rebind %s to %s", intf, drvpath); 
> >> > +                goto out; 
> >> > +            } 
> >> > +        } 
> >> > +    } 
> >> > + 
> >> > +    path = GCSPRINTF(USBBACK_INFO_PATH "/%s", usbdev_encode); 
> >> > +    rc = libxl__xs_rm_checked(gc, XBT_NULL, path); 
> >> > + 
> >> > +out: 
> >> 
> >> So it looks like if one of the re-binds fails, then it stops where it is 
> >> and leaves the USBBACK re-bind info in xenstore.  In that case it's not 
> >> clear to me how that information would ever be removed. 
> >> 
> >> I think until such time as we have a command to re-attempt the re-bind, 
> >>  if there's an error in the actual rebind, it should just break out of 
> >> the for loop, and remove the re-bind nodes, and document a way to let 
> >> the user try to clean things up. 
> > 
> > Just according to last time discussion about how to handle the rebind 
> > failure, seems Ian preferred to add a xl command to deal with rebind 
> > in future, based on that need, I think driver_path info should be kept 
> > in xenstore then. Without that need, I agree it's best to remove 
> > xenstore nodes. So, keep or remove? 
>  
> Well when we have such a command, then yes, we'll need to keep the 
> nodes for it to use and delete.  But at the moment, we have no such 
> command, so these nodes will just sit around cluttering up the libxl 
> xenstore space, and nothing will delete them (unless a user manually 
> runs xenstore-rm). 
>  
> So I think for now we should delete them on failure; and at some point 
> later, when someone implements a recovery command, then they should 
> change this code to not delete the xenstore nodes (and also change the 
> log messages to point to that command). 

OK. Got it. Will update.

-  Chunyan

>  
>  -George 
>  
>  


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