On 7/30/19 11:28 AM, Richard W.M. Jones wrote: > On Tue, Jul 30, 2019 at 11:21:25AM -0500, Eric Blake wrote: >> On 7/30/19 10:36 AM, Richard W.M. Jones wrote: >>> +When the command completes, C<callback> >>> will be invoked as described in L<libnbd(3)/Completion callbacks>. >>> + >> >> Do we need wording anywhere (probably in the .pod, so we only state it >> once) that mentions that the callback routine is not invoked if >> nbd_aio_FOO_callback returns -1? > > I guess it's implicit from the fact that returning -1 indicates an > error. > > In fact the relationship ought to be stronger than that - the command > should run if and only if nbd_aio_FOO_callback returns >= 0. (That's > not actually true at the moment because an error can happen after > we've enqueued the command.)
Eww, you're right. We return -1 if nbd_internal_run queued the command but encountered an error moving the state machine along; but that is much trickier for the user to recover from (if they aren't using the _callback form, then they NEED to know the cookie id to eventually retire the command during nbd_aio_command_completed). Maybe we should guarantee that we return the cookie in that case, even though a failure was detected? I don't think it is a good idea to change the API from 'int64 nbd_aio_pread()' to 'int nbd_aio_pread(int64 *cookie)' where *cookie gets assigned on successful queue whether or not the overall command returns 0 or -1. In the long term, if nbd_internal_run() fails, then the next nbd_* call that tries to manipulate the state machine will see that the state machine is in the DEAD state; learning that the state machine failed during nbd_aio_pread probably doesn't buy us much benefit. So I'm leaning towards just ignoring nbd_internal_run failure, and guaranteeing that we return > 0 if we queue the command, no matter whether we then encounter a state machine error after that point. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Libguestfs mailing list [email protected] https://www.redhat.com/mailman/listinfo/libguestfs
