Re: [Libguestfs] [libnbd] notify API changes (was: Re: [libnbd PATCH 5/6] api: Add new nbd_aio_FOO_notify functions)
On 7/15/19 6:06 AM, Richard W.M. Jones wrote:
On Sat, Jun 29, 2019 at 08:28:28AM -0500, Eric Blake wrote:
> As mentioned in the previous patch, there are situations where an aio
> client wants instant notification when a given command is complete,
> rather than having to maintain a separate data structure to track all
> in-flight commands and then iterate over that structure to learn which
> commands are complete. It's also desirable when writing a server
> validation program (such as for checking structured reads for
> compliance) to be able to clean up the associated opaque data and have
> a final chance to change the overall command status.
>
> Introduce new nbd_aio_FOO_notify functions for each command. Rewire
> the existing nbd_aio_FOO to forward to the new command. (Perhaps the
> generator could reduce some of the boilerplate duplication, if a later
> patch wants to refactor this).
I'm writing some code now using these new nbd_aio_<CMD>_notify
functions, and I temporarily confused myself because these are similar
to nbd_notify_read and nbd_notify_write (the functions used to signal
to the state machine that the socket is ready for reading/writing).
I wonder if we should rename something here. My suggestions are
either of the following or both:
(I) Rename nbd_notify_read / nbd_notify_write to nbd_ready_to_read /
nbd_ready_to_write.
I'm still wondering if we want to add an nbd_notify_error for the I/O
thread to inform libnbd about a POLLERR situation (as otherwise the
libnbd state machine may not notice the error until much later). I
don't think nbd_read_to_error() sounds good, so keeping the name
nbd_notify_FOO for notifying the state machine about a particular change
in the fd seems best.
(II) Rename nbd_aio_<CMD>_notify to nbd_aio_<CMD>_callback.
which leaves this as our only sane option to rename. The bikeshedding
rename sounds fine to me; it's a fairly simple change; it does bump
API/ABI and will require another release, but we're still at the point
where we can do it.
I've also noticed that we have 'struct nbd_handle' and also refer to all
of our nbd_aio_FOO() results as command handles; that might get
confusing. My though there is to rename the latter to 'cookie' or 'id'
or any other term you can think of which would fit (which is more of a
doc change, and not an actual API change).
What do you think?
Do you want me to go ahead and rename nbd_aio_FOO_notify to
nbd_aio_FOO_callback?
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 488 bytes)