[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



Chun Yan Liu writes ("Re: [Xen-devel] [PATCH V4 3/7] libxl: add pvusb API"):
> <21881.47707.526863.158586@xxxxxxxxxxxxxxxxxxxxxxxx>, Ian Jackson
> <Ian.Jackson@xxxxxxxxxxxxx> wrote: 
> > Chunyan Liu writes ("[Xen-devel] [PATCH V4 3/7] libxl: add pvusb API"): 

> > > +int libxl_device_usbctrl_getinfo(libxl_ctx *ctx, uint32_t domid, 
> > > +                                libxl_device_usbctrl *usbctrl, 
> > > +                                libxl_usbctrlinfo *usbctrlinfo) 
> > > +                                LIBXL_EXTERNAL_CALLERS_ONLY; 
> >  
> > Why is this function marked LIBXL_EXTERNAL_CALLERS_ONLY ? 
> 
> Currently libxl itself won't call it. Exposed for toolstack usage. Will
> remove that if it's not proper.

LIBXL_EXTERNAL_CALLERS_ONLY is a special macro which _prevents_ anyone
from adding calls to the function from inside libxl.  It should be
used where adding such an internal caller would be a mistake, not
simply where there happen not to be any internal callers right now.

> > > +/* AO operation to connect a PVUSB controller. 
> > > + * Adding a PVUSB controller will add a 'vusb' device entry in xenstore, 
> > > + * and will wait for device connection. 
> >  
> > In this context I think "will wait for device connection" is 
> > misleading.  What you mean is that the vusb is available for adding 
> > devices to, but won't have any yet.  Nothing in libxl is "waiting". 
> 
> Here I mean libxl_wait_for_device_connection. Since it adds a new
> device entry to xenstore, it needs to wait for a moment for
> frontend/backend connection.

Oh, I see.  I don't think you should mention that here.  That part of
the implementation, not the interface.

AFAICT libxl__device_usbctrl_add works just like all the other
libxl__device_*_add functions, in that it adds the device and when
completed it calls aodev->callback.  There is no need to mention this
explicitly in a comment - indeed, mentioning it for this device type
but not for others would lead the reader to wonder what was different.

Thanks,
Ian.

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.