Re: [Libguestfs] [PATCH libnbd 2/3] lib: Implement closure lifetimes.
On 7/24/19 12:48 PM, Richard W.M. Jones wrote:
> [2] Here, we know we are calling the read callback exactly once,
so we
> could pass DATA|FREE here.
That was actually my first version of the patch. However it runs into
the problem that we need to call the FREE version when a command is
aborted. It's complicated to know if we called cb.fn.read already.
But thinking about this a bit more, maybe we should call VALID|FREE
here and set cmd->cb.fn.read = NULL afterwards.
Indeed, that's a nice trick; it also solves the question for structured
replies (if we pass VALID|FREE when we see NBD_REPLY_FLAG_DONE, then set
the callback to NULL, then we still need to manage calling plain FREE
later on when finally retiring the command, but only if the callback is
still set). I'll fold that in to my followup, as having fewer callback
invocations is always worthwhile (after all, indirect functions got more
expensive thanks to Spectre), even if it is probably not the bottleneck
here.
> This frees the first debug callback when installing a second; but
where
> does nbd_close free the second? Also, do we allow C code to pass NULL
> to uninstall a handler? Should other languages be able to clear out a
> callback?
Yes nbd_close should check and call the callback again with FREE.
I still didn't see that in v2; but we'll get it fixed soon enough.
--
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)