[Libguestfs] [libnbd] More thoughts on callbacks and more

[Eric Blake]

On 7/22/19 6:50 AM, Richard W.M. Jones wrote:
> On Mon, Jul 22, 2019 at 10:08:25AM +0100, Richard W.M. Jones wrote:
>> On Sat, Jul 20, 2019 at 07:38:45AM +0100, Richard W.M. Jones wrote:
>>> More thoughts on callbacks, etc. following on from:
>>> https://www.redhat.com/archives/libguestfs/2019-July/thread.html#00184
>>>
>>> Closure lifetimes
>>> -----------------
> 
> Here's a possibly better idea which still changes the API a bit but
> not as invasively.
> 
> We overload the callback so that it can either be a callback function
> or a "free function".  They are distinguished by an extra flag
> argument passed to the callback:
> 
>   extern int nbd_set_debug_callback (
>         struct nbd_handle *h,
>         int (*debug_fn) (int valid_flag,   // <-- note
>                          void *user_data,
> 	                 const char *context, const char *msg),
>         void *user_data);

Would the 'valid_flag' be presented to non-C bindings, or is it only
needed for C code? At any rate, the idea makes sense as a lighter-weight
way for libnbd to always inform the callback about the last invocation.
For nbd_aio_pread_structured and nbd_aio_block_status, the chunk/extent
callback can either match server state (if the server replies replies
with NBD_REPLY_FLAG_DONE on an OFFSET_DATA/BLOCK_STATUS reply, we could
set VALID|FREE; if the server defers NBD_REPLY_FLAG_DONE to an
NBD_REPLY_TYPE_NONE packet then the two flags will definitely not be set
at the same time), or we could always defer the FREE flag until the
overall command is ready to retire; for nbd_aio_FOO_callback, it may
indeed be easier to set VALID|FREE on the single use of the callback at
the time it is ready to retire.  But this idea does make it possible for
libnbd to inform the callback about its last expected use.

> 
> The extra ‘valid_flag’ contains one or both of LIBNBD_CALLBACK_VALID
> and LIBNBD_CALLBACK_FREE.  Callback code would look like:
> 
>   int my_debug_fn (int valid_flag, void *user_data,
>                    const char *context, const char *msg)
>   {
>     if (valid_flag & LIBNBD_CALLBACK_VALID) {
>       // This is the normal callback as before.
>       printf ("debug msg = %s\n", msg);
>     }
> 
>     if (valid_flag & LIBNBD_CALLBACK_FREE) {
>       // In this case only user_data is valid.  The other parameters
>       // may be unspecified (unless VALID was also set).
>       free (user_data);
>     }
>   }

I also like the idea of letting the callback handle both flags in a
single call where sensible, for fewer indirect function calls.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20190722/2693400f/attachment.sig>