Re: [Libguestfs] [libnbd PATCH 5/8] states: Wire in a read callback
On 6/20/19 3:34 AM, Richard W.M. Jones wrote:
On Mon, Jun 17, 2019 at 07:07:55PM -0500, Eric Blake wrote:
> + errno = nbd_internal_errno_of_nbd_error (error);
> + set_error (errno, "server reported read failure at offset 0x%"
PRIx64,
> + offset);
> + if (cmd->cb.fn.read (cmd->cb.opaque, cmd->data + (offset -
cmd->offset),
> + 0, offset, LIBNBD_READ_ERROR) == -1)
I understand that in the next commit the pread_callback callback
function is meant to read errno implicitly, and this is why you are
setting it before calling that callback here. However I don't
understand why you didn't just pass this in as an extra (ie. explicit)
parameter? As currently defined this will not work well in non-C
languages, because the generator and indeed those languages know
nothing about errno.
Oh, good point.
With reads, since the server is allowed to send out-of-order chunks;
thus, it is valid for a 1024 read request to result in a server sending
first ERROR_OFFSET at 512, and then OFFSET_DATA covering [0-511]. If the
user is ever going to have a chance to gracefully turn this into a
short-read result (claiming a successful read of 512 bytes instead of an
overall failure), it is essential that we invoke a callback for a later
data chunk even if an earlier error has occurred. So even for a data
chunk, knowing the current error value from all earlier callbacks may be
useful. So I'll definitely respin this to include an explicit error
parameter to all uses of the callback.
For the existing nbd_aio_block_status, our current behavior is that as
soon as a callback fails, we skip calling the callback for any
additional contexts. We may want to change that behavior to match what
I envision for pread, in calling the callback every time even when an
error has already happened. Thoughts?
--
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)