Re: [Libguestfs] [libnbd PATCH] docs: Clarify how callbacks should handle errors

> Some of the high-level commands (L<nbd_pread_structured(3)>, > -L<nbd_block_status(3)>) involve the use of a callback function invoked by > -the state machine at appropriate points in the server's reply before > -the overall command is complete. These callback functions, along with > -all of the completion callbacks, include a parameter C<error> > -containing the value of any error detected so far; if the callback

> -function fails, it should assign back into C<error> and return C<-1> > -to change the resulting error of the overall command. Assignments > -into C<error> are ignored for any other return value; similarly, > -assigning C<0> into C<error> does not have an effect. > +L<nbd_block_status(3)>) involve the use of a callback function invoked > +by the state machine at appropriate points in the server's reply > +before the overall command is complete. These callback functions, > +along with all of the completion callbacks, include a parameter > +C<error> containing the value of any error detected so far. If a Error is a pointer to the value

> +callback function fails and wants to change the resulting error of the > +overall command visible later in the API sequence, it should assign > +back into C<error> and return C<-1>. Assignments into C<error> are > +ignored for any other return value; similarly, assigning C<0> into > +C<error> does not have an effect. But maybe the text about assigning into it is good enough to make this clear. > + > +Note that a mid-command callback might never be reached, such as if > +libnbd detects that the command was invalid to send (see > +L<nbd_set_strict_mode(3)>) or if the server reports a failure. It is > +safe for a mid-command callback to ignore non-zero C<error>: all the > +other parameters to the mid-command callback will still be valid > +(corresponding to the current portion of the server's reply), and the > +overall command will still fail (at the completion callback or > +L<nbd_aio_command_completed(3)> for an asynchronous command, or as the > +result of the overall synchronous command). Returing C<-1> from a > +mid-command callback does not prevent that callback from being reached > +again, if the server sends more mid-command replies that warrant > +another use of that callback. A mid-command callback may be reached > +more times than expected if the server is non-compliant. > + > +On the other hand, if a completion callback is supplied (only possible > +with asynchronous commands), it will always be reached exactly once, > +and the completion callback must not ignore the value of C<error>. In > +particular, the content of a buffer passed to L<nbd_aio_pread(3)> or > +L<nbd_aio_pread_structured(3)> is unspecified if C<error> is set on > +entry to the completion callback. It is recommended that if your code > +uses automatic command retirement, then the completion function should > +return C<1> on all control paths, even when handling errors (note that > +with automatic retirement, assigning into C<error> is pointless as > +there is no later API to see that value). I think we miss a warning here, that if you don't use automatic retirement, you *must* call nbd_aio_command_completed() to retire the command, otherwise commands will leak until you close the handle.

The text should make it clear that the caller need to chose how to use the library, either use callbacks or use completion checks. Can we make this simpler by preventing usage of completion checks if you define a completion callback? This will eliminate the auto retire concept - if you define a callback, commands always retires.