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

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



Ian Jackson wrote:
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.

What about libxl__initiate_device_connection?

and thanks for reviewing the code :).

_______________________________________________
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®.