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).