On 8/13/19 2:33 AM, Richard W.M. Jones wrote:
On Mon, Aug 12, 2019 at 11:00:01PM +0100, Richard W.M. Jones wrote:
On Mon, Aug 12, 2019 at 01:53:56PM -0500, Eric Blake wrote:
We then have the design question of whether to make an OClosure type, where C has two functions nbd_aio_pread and nbd_aio_pread_callback for convenience, but where other languages have only a single nbd.aio_pread where the callback parameter is optional (the Closure type for pread_structured chunk and for block_status extent will still be mandatory; it is only the completion callback that is currently causing us twice the API because we are treating it as pseudo-optional). Or maybe we just require C clients of nbd_aio_pread to always provide parameters for callbacks, but document that the completion callback and free callback pointers may be NULL.
I'll try to see what OClosure (and then getting rid of the _callback variants, adding to the non-callback variants OClosure) will look like tomorrow.
I should say that while OClosure has the possibility of solving the freeing problem for buffers and closures attached to AIO commands, and may also be a good idea to get rid of the *_callback variants, it doesn't solve the problem in general.
In particular there's no way to free the debug function closure, which means it would be leaked in language bindings.
Not if we keep our current 2-tuple per Closure/OClosure. But the moment we switch the C bindings to have a 3-tuple per Closure, the debug function becomes: nbd_set_debug_callback (h, debug_cb, free_cb, user_data); with libnbd calling debug_cb(user_data) as often as desired, and free_cb(user_data) exactly once when installing a different callback or reaching nbd_close(). The language bindings then increment the refcount on the language callback and pack it into a malloc'd C struct during nbd_set_debug_callback; then debug_cb(user_data) grabs the language object out of the C struct for calling back into the language, and free_cb(user_data) decrements the refcount on the language object and then free()s the C struct. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org