Re: [Libguestfs] [libnbd PATCH v2 3/5] docs: Clarify how callbacks should handle errors
On 02/03/22 21:25, Eric Blake wrote: > Recent patches have demonstrated confusion on the order in which > callbacks are reached, when it is safe or dangerous to ignore *error, > and what a completion callback should do when auto-retirement is in > use. Add wording to make it more obvious that: > > - callbacks are reached in the following order: mid-command callback > (0, 1, or many times, if supplied), completion callback (exactly > once, if supplied), mid-command free (exactly once, if supplied), > completion free (exactly once, if supplied) > - returning -1 from a mid-command callback does does not prevent > future callbacks > - ignoring *error in a mid-command callback is safe > - completion callbacks are reached unconditionally, and must NOT ignore > *error > - if the user chooses to use auto-retirement instead of manual calls to > nbd_aio_command_completed, the completion callback should return 1 even > on error cases to avoid complicating command cleanup > - the contents of buf after nbd_pread and friends is undefined on > error (at present, if the user did not pre-initialize the buffer, > there are some code paths in libnbd that leave it uninitialized) > --- > docs/libnbd.pod | 69 +--- > generator/API.ml | 24 - > 2 files changed, 71 insertions(+), 22 deletions(-) > > diff --git a/docs/libnbd.pod b/docs/libnbd.pod > index eb8038b0..15bdf0a8 100644 > --- a/docs/libnbd.pod > +++ b/docs/libnbd.pod > @@ -829,8 +829,12 @@ This can be used to free associated C. For > example: > NBD_NULL_COMPLETION, > 0); > > -will call L on C after the last time that the > -S> function is called. > +will call L once on C after the point where it is > +known that the S> function can no longer be > +called, regardless of how many times C was actually called. If > +both a mid-command and completion callback are supplied, the functions > +will be reached in this order: mid-function callbacks, completion > +callback, mid-function free, and finally completion free. > > The free function is only accessible in the C API as it is not needed > in garbage collected programming languages. > @@ -858,27 +862,60 @@ same nbd object, as it would cause deadlock. > > =head2 Completion callbacks > > -All of the low-level commands have a completion callback variant that > -registers a callback function used right before the command is marked > -complete. > +All of the asychronous commands have an optional completion callback > +function that is used right before the command is marked complete, > +after any mid-command callbacks have finished, and before any free > +functions. > > When the completion callback returns C<1>, the command is > automatically retired (there is no need to call > -L); for any other return value, the command > -still needs to be retired. > +L); for any other return value, the > +command still needs to be manually retired (otherwise, the command > +will tie up resources until L is eventually reached). > > =head2 Callbacks with C parameter > > Some of the high-level commands (L, > -L) 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 > -containing the value of any error detected so far; if the callback > -function fails, it should assign back into C and return C<-1> > -to change the resulting error of the overall command. Assignments > -into C are ignored for any other return value; similarly, > -assigning C<0> into C does not have an effect. > +L) 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 which is a pointer containing the value of any error detected > +so far. If a 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 and return C<-1> in the > +C API. Assignments into C are ignored for any other return > +value; similarly, assigning C<0> into C does not have an > +effect. In other language bindings, reporting callback errors is > +generally done by raising an exception rather than by return value. > + > +Note that a mid-command callback might never be reached, such as if > +libnbd detects that the command was invalid to send (see > +L) or if the server reports a failure that > +concludes the command. It is safe for a mid-command callback to > +ignore non-zero C: 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
Re: [Libguestfs] [libnbd PATCH v2 3/5] docs: Clarify how callbacks should handle errors
()On Thu, Feb 3, 2022 at 10:28 PM Eric Blake wrote: > > Recent patches have demonstrated confusion on the order in which > callbacks are reached, when it is safe or dangerous to ignore *error, > and what a completion callback should do when auto-retirement is in > use. Add wording to make it more obvious that: > > - callbacks are reached in the following order: mid-command callback > (0, 1, or many times, if supplied), completion callback (exactly > once, if supplied), mid-command free (exactly once, if supplied), > completion free (exactly once, if supplied) > - returning -1 from a mid-command callback does does not prevent > future callbacks > - ignoring *error in a mid-command callback is safe > - completion callbacks are reached unconditionally, and must NOT ignore > *error > - if the user chooses to use auto-retirement instead of manual calls to > nbd_aio_command_completed, the completion callback should return 1 even > on error cases to avoid complicating command cleanup > - the contents of buf after nbd_pread and friends is undefined on > error (at present, if the user did not pre-initialize the buffer, > there are some code paths in libnbd that leave it uninitialized) > --- > docs/libnbd.pod | 69 +--- > generator/API.ml | 24 - > 2 files changed, 71 insertions(+), 22 deletions(-) > > diff --git a/docs/libnbd.pod b/docs/libnbd.pod > index eb8038b0..15bdf0a8 100644 > --- a/docs/libnbd.pod > +++ b/docs/libnbd.pod > @@ -829,8 +829,12 @@ This can be used to free associated C. For > example: > NBD_NULL_COMPLETION, > 0); > > -will call L on C after the last time that the > -S> function is called. > +will call L once on C after the point where it is > +known that the S> function can no longer be > +called, regardless of how many times C was actually called. If > +both a mid-command and completion callback are supplied, the functions > +will be reached in this order: mid-function callbacks, completion > +callback, mid-function free, and finally completion free. > > The free function is only accessible in the C API as it is not needed > in garbage collected programming languages. > @@ -858,27 +862,60 @@ same nbd object, as it would cause deadlock. > > =head2 Completion callbacks > > -All of the low-level commands have a completion callback variant that > -registers a callback function used right before the command is marked > -complete. > +All of the asychronous commands have an optional completion callback > +function that is used right before the command is marked complete, > +after any mid-command callbacks have finished, and before any free > +functions. > > When the completion callback returns C<1>, the command is > automatically retired (there is no need to call > -L); for any other return value, the command > -still needs to be retired. > +L); for any other return value, the > +command still needs to be manually retired (otherwise, the command > +will tie up resources until L is eventually reached). > > =head2 Callbacks with C parameter > > Some of the high-level commands (L, > -L) 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 > -containing the value of any error detected so far; if the callback > -function fails, it should assign back into C and return C<-1> > -to change the resulting error of the overall command. Assignments > -into C are ignored for any other return value; similarly, > -assigning C<0> into C does not have an effect. > +L) 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 which is a pointer containing the value of any error detected > +so far. If a 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 and return C<-1> in the > +C API. Assignments into C are ignored for any other return > +value; similarly, assigning C<0> into C does not have an > +effect. In other language bindings, reporting callback errors is > +generally done by raising an exception rather than by return value. > + > +Note that a mid-command callback might never be reached, such as if > +libnbd detects that the command was invalid to send (see > +L) or if the server reports a failure that > +concludes the command. It is safe for a mid-command callback to > +ignore non-zero C: 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
[Libguestfs] [libnbd PATCH v2 3/5] docs: Clarify how callbacks should handle errors
Recent patches have demonstrated confusion on the order in which callbacks are reached, when it is safe or dangerous to ignore *error, and what a completion callback should do when auto-retirement is in use. Add wording to make it more obvious that: - callbacks are reached in the following order: mid-command callback (0, 1, or many times, if supplied), completion callback (exactly once, if supplied), mid-command free (exactly once, if supplied), completion free (exactly once, if supplied) - returning -1 from a mid-command callback does does not prevent future callbacks - ignoring *error in a mid-command callback is safe - completion callbacks are reached unconditionally, and must NOT ignore *error - if the user chooses to use auto-retirement instead of manual calls to nbd_aio_command_completed, the completion callback should return 1 even on error cases to avoid complicating command cleanup - the contents of buf after nbd_pread and friends is undefined on error (at present, if the user did not pre-initialize the buffer, there are some code paths in libnbd that leave it uninitialized) --- docs/libnbd.pod | 69 +--- generator/API.ml | 24 - 2 files changed, 71 insertions(+), 22 deletions(-) diff --git a/docs/libnbd.pod b/docs/libnbd.pod index eb8038b0..15bdf0a8 100644 --- a/docs/libnbd.pod +++ b/docs/libnbd.pod @@ -829,8 +829,12 @@ This can be used to free associated C. For example: NBD_NULL_COMPLETION, 0); -will call L on C after the last time that the -S> function is called. +will call L once on C after the point where it is +known that the S> function can no longer be +called, regardless of how many times C was actually called. If +both a mid-command and completion callback are supplied, the functions +will be reached in this order: mid-function callbacks, completion +callback, mid-function free, and finally completion free. The free function is only accessible in the C API as it is not needed in garbage collected programming languages. @@ -858,27 +862,60 @@ same nbd object, as it would cause deadlock. =head2 Completion callbacks -All of the low-level commands have a completion callback variant that -registers a callback function used right before the command is marked -complete. +All of the asychronous commands have an optional completion callback +function that is used right before the command is marked complete, +after any mid-command callbacks have finished, and before any free +functions. When the completion callback returns C<1>, the command is automatically retired (there is no need to call -L); for any other return value, the command -still needs to be retired. +L); for any other return value, the +command still needs to be manually retired (otherwise, the command +will tie up resources until L is eventually reached). =head2 Callbacks with C parameter Some of the high-level commands (L, -L) 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 -containing the value of any error detected so far; if the callback -function fails, it should assign back into C and return C<-1> -to change the resulting error of the overall command. Assignments -into C are ignored for any other return value; similarly, -assigning C<0> into C does not have an effect. +L) 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 which is a pointer containing the value of any error detected +so far. If a 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 and return C<-1> in the +C API. Assignments into C are ignored for any other return +value; similarly, assigning C<0> into C does not have an +effect. In other language bindings, reporting callback errors is +generally done by raising an exception rather than by return value. + +Note that a mid-command callback might never be reached, such as if +libnbd detects that the command was invalid to send (see +L) or if the server reports a failure that +concludes the command. It is safe for a mid-command callback to +ignore non-zero C: 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 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
