Re: [Xen-devel] [PATCH V8 3/7] libxl: add pvusb API

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

>>> On 11/17/2015 at 02:06 AM, in message
<22090.6954.35639.703183@xxxxxxxxxxxxxxxxxxxxxxxx>, Ian Jackson
<Ian.Jackson@xxxxxxxxxxxxx> wrote: 
> Thanks for your attention to my earlier mail.  I'll delete all the 
> comments where we agree :-). 
>  
> > > > > > +/* bind/unbind usb device interface */   
> > > > > > +static int unbind_usb_intf(libxl__gc *gc, char *intf, char 
> > > > > > **drvpath)   
>  
> > > > > > +{   
> > > ...  
> > > > > > +        dp = libxl__zalloc(gc, PATH_MAX);   
> > > > > > +        dp = realpath(spath, dp);   
> > > > >    
> > > > > Why is this call to realpath needed ?  
> > > >   
> > > > In sysfs, /driver sometimes is a link, since we need to save the 
> > > > original  
>  
> > > > driver to xenstore, so need to get the realpath of the driver.  
> > >   
> > > I mean, why is the path with all the symlinks in it not suitable for  
> > > storage and later use ?  
> >  
> > The symlink might be "../../../../../../bus/usb/drivers/btusb", we 
> > couldn't save that to xenstore for later usage. 
>  
> So the real reason is simply that it is a relative path and we need an 
> absolute one, because a relative path is not useful ? 
>  
> OK, thanks for the explanation.  (I'm not sure whether this is 
> unobvious enough to warrant a comment, perhaps 
>  /* sysfs can produce relative paths */ 
> or something.) 
>  
> > > > > I think you probably do things in the wrong order here.  You should   
> > > > > write the old driver info to xenstore first, so that if the bind to   
> > > > > usbback fails, you have the old driver info.   
> > > >   
> > > > Perhaps not. Once we finished adding entries to xenstore, pvusb  
> > > > frontend/backend drivers will detect the change and do probe work, if  
> > > > USB device is still not bound to USBBACK, there might be error.  
> > >   
> > > What I mean is this:  
> > >   
> > > Is it not possible to write the original path to xenstore before doing  
> > > the unbind ?  Otherwise it seems like there could be error paths where  
> > > the original path is not recorded, the xenstore write fails, and then  
> > > the information about how to rebind to the original driver has been  
> > > lost.  
> >  
> > I see.  
>  
> Right.  Do you think this warrants a change to the code ? 
>  
> > > Does writing USBBACK_INFO_PATH"/%s/%s/driver_path" really trigger  
> > > usbback ?  
> >  
> > No, writing driver_path info to xenstore won't trigger usbback. Writing 
> > frontend/backend info will. 
>  
> Jolly good. 
>  
> > > > If we merge libxl__device_usb_add and do_usb_add, then it's cleaner.  
> > >   
> > > See my comment below.  You've explained the distinction to my  
> > > satisfaction.  
> > >   
> > > But, to solve the duplication of the controller info acquisition,  
> > > perhaps you could have do_usb_add take the controller info as a  
> > > paramaeter ? 
> >  
> > Yes, could be. Only do_usb_add and do_usb_remove parameters are not 
> > symmetrical.  
>  
> I don't think that matters.  (If it did it would be better to add an 
> unused parameter to the remove function.) 

Take all. Will update codes.

>  
> Regards, 
> Ian. 
>  
> _______________________________________________ 
> Xen-devel mailing list 
> Xen-devel@xxxxxxxxxxxxx 
> http://lists.xen.org/xen-devel 
>  
>  



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