[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH V8 3/7] libxl: add pvusb API
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.) Regards, Ian. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |