Re: [Libguestfs] [libnbd PATCH v2 2/5] states: Wire in a read callback
On 6/25/19 4:13 AM, Richard W.M. Jones wrote:
On Thu, Jun 20, 2019 at 08:45:05PM -0500, Eric Blake wrote:
> If any callback fails, and if no prior error was set, then the
> callback's failure becomes the failure reason for the overall
> read. (Note that this is different from the block status callback,
> which for now quits using the callback on subsequent chunks if an
> earlier chunk failed - we may decide to change one or the other for
> consistency before the API is stable.)
>
> Nothing actually passes a callback function yet, so for now this is no
> functional change; but this will make it possible for the next patch
> to add an 'nbd_aio_pread_structred' API.
I don't think non-C callbacks will be able to reliably set errno, but
that's something we can solve in future. Therefore ACK.
Perhaps we could change the API to:
cmd->cb.fn.read (cmd->cb.opaque, cmd->data, cmd->count, cmd->offset,
&error, LIBNBD_READ_XXX)
where the caller is not only passed the current error, but also can
store into that variable to affect the return error (rather than the
return error being implied from whatever is left in errno).
Of course, that necessitates passing a pointer to an integer to the
callback, rather than a bare integer, so it requires a bit more surgery
to the generator. And it may still not be the best interface for other
language bindings - for example, I'm not sure how python would represent
an in/out pointer to integer. In nbdkit, the solution was that we had a
utility function nbdkit_set_error() which was more easily bound to other
languages; the various plugins then declare whether they can reliably
set errno, or whether errno is unreliable and nbdkit_set_error() should
be the sole source of requested error status instead. So the similar
interface here is that callbacks are always passed just a bare integer
on input, and then call a utility function to request a specific error
on output (where we then have a way to decide whether to use the value
in errno, or just a hard-coded default, if the utility function was not
called).
As we are free to change API at a later date up until we declare stable
API, I'm going ahead and pushing this one as-is.
--
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)