Re: [Xen-devel] [PATCH v5 04/10] libxl: convert libxl_device_disk_add to an asyn op

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Roger Pau Monne writes ("Re: [PATCH v5 04/10] libxl: convert 
libxl_device_disk_add to an asyn op"):
> Ian Jackson wrote:
> > And you're using `internal' here to mean `internal to libxl'.  I think
> > that's clear from the name (which has `__').  Whereas on first reading
> > it sounds like you mean `internal to the device adding machinery' -
> > but `libxl__device_disk_add' is the main entrypoint to that
> > machinery.
> 
> I've used 'internal' here to reflex the difference between 
> libxl_device_disk_add and libxl__device_disk_add, but probably leads to 
> confusion.

Yes.

> Well the order is the following:
> 
> libxl__device_{disk/nic}_add (device type dependant function) -> 
> libxl__initiate_device_add (device type independent).

Yes.

> I'm really bad about this naming thing. But the fact is that 
> libxl__initiate_device_add is not a good name, since when we call this 
> function the device is already added (to xenstore), this merely waits 
> for it to reach the desired state and performs the necessary actions to 
> "add" it to the guest (call hotplug scripts). Maybe "connect" is a more 
> appropriate name than "add".

Perhaps so.

> I liked the nomenclature because it follows the same style as 
> libxl__initiate_device_remove, and I think (and I might be wrong) it 
> makes things easier to understand.

But  libxl__initiate_device_remove  _is_ the function which actually
initiates a device removal.  It isn't an internal helper for a
family of  libxl__device_FOO_remove  functions.

If it were symmetrical,  libxl__initate_device_add  would be the
single entrypoint and it would call something kind-specific as a helper.
That's what the comments and names seem to suggest to me, apart from
the explicit statement that it's the other way around.

Hence my feeling that it needs to be clarified and perhaps this
function renamed.

Ian.

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