search for: libnbd_callback_valid

...her languages you can use closures to achieve the same outcome. -=head2 Callback lifetimes - -All callbacks have an C<unsigned valid_flag> parameter which is used -to help with the lifetime of the callback. C<valid_flag> contains the -I<logical or> of: - -=over 4 - -=item C<LIBNBD_CALLBACK_VALID> - -The callback parameters are valid and this is a normal callback. - -=item C<LIBNBD_CALLBACK_FREE> - -This is the last time the library will call this function. Any data -associated with the callback can be freed. - -=item other bits - -Other bits in C<valid_flag> should be ignor...

...ings we can remove roots / decrement reference counts at the right place to free the closure, without waiting for the handle to be closed. The solution to this is to introduce the concept of a closure lifetime. The callback is called with an extra valid_flag parameter which is a bitmap containing LIBNBD_CALLBACK_VALID and/or LIBNBD_CALLBACK_FREE. LIBNBD_CALLBACK_VALID corresponds to a normal call of the callback function by the library. After the library has finished with the callback (declaring that this callback will never be needed or called again), it is called once more with valid_flag == LIBNBD_CALLBACK_...

...ings we can remove roots / decrement reference counts at the right place to free the closure, without waiting for the handle to be closed. The solution to this is to introduce the concept of a closure lifetime. The callback is called with an extra valid_flag parameter which is a bitmap containing LIBNBD_CALLBACK_VALID and/or LIBNBD_CALLBACK_FREE. LIBNBD_CALLBACK_VALID corresponds to a normal call of the callback function by the library. After the library has finished with the callback (declaring that this callback will never be needed or called again), it is called once more with valid_flag == LIBNBD_CALLBACK_...

I think I've addressed everything that was raised in review. Some of the highlights: - Callbacks should be freed reliably along all exit paths. - There's a simple test of closure lifetimes. - I've tried to use VALID|FREE in all the places where I'm confident that it's safe and correct to do. There may be more places. Note this is an optimization and shouldn't

...lback. C<valid_flag> contains the -I<logical or> of: +You can associate a free function with callbacks. Libnbd will call +this function when the callback will not be called again by libnbd. -=over 4 +This can be used to free associated C<user_data>. For example: -=item C<LIBNBD_CALLBACK_VALID> + void *my_data = malloc (...); + + nbd_aio_pread_structured (nbd, buf, sizeof buf, offset, + (nbd_chunk_callback) { .callback = my_fn, + .user_data = my_data, + .free = free }, + NBD_NULL...

...ire before the original API has returned). -If the completion callback returns C<1>, the command is automatically -retired (there is no need to call C<nbd_aio_command_completed>); for -any other return value, the command still needs to be retired. +When C<valid_flag> includes C<LIBNBD_CALLBACK_VALID>, and the +completion callback returns C<1>, the command is automatically retired +(there is no need to call C<nbd_aio_command_completed>); for any other +return value, the command still needs to be retired. =head2 Callbacks with C<int *error> parameter @@ -515,8 +516,9 @@ a...

...ings we can remove roots / decrement reference counts at the right place to free the closure, without waiting for the handle to be closed. The solution to this is to introduce the concept of a closure lifetime. The callback is called with an extra valid_flag parameter which is a bitmap containing LIBNBD_CALLBACK_VALID and/or LIBNBD_CALLBACK_FREE. LIBNBD_CALLBACK_VALID corresponds to a normal call of the callback function by the library. After the library has finished with the callback (declaring that this callback will never be needed or called again), it is called once more with valid_flag == LIBNBD_CALLBACK_...

...> close callbacks. Our mails are crossing; I had typed this up (so I'm sending now) even though I already see that you've tweaked things in v2. I've got two threads of thoughts below. > > (Note it is also possible for the library to call the callback with > valid_flag == LIBNBD_CALLBACK_VALID|LIBNBD_CALLBACK_FREE, meaning it's the > last valid call.) [1] In my previous reply, I assumed that the nbd_aio_FOO_callback function would use this paradigm... > +++ b/examples/strict-structured-reads.c > @@ -127,11 +131,14 @@ read_chunk (void *opaque, const void *bufv, size_t cou...

...-reply-structured.c index 2ef8d20..20b0e9e 100644 --- a/generator/states-reply-structured.c +++ b/generator/states-reply-structured.c @@ -18,6 +18,19 @@ /* State machine for parsing structured replies from the server. */ +static unsigned +valid_flags (struct nbd_handle *h) +{ + unsigned valid = LIBNBD_CALLBACK_VALID; + uint16_t flags = be16toh (h->sbuf.sr.structured_reply.flags); + + if (flags & NBD_REPLY_FLAG_DONE) + valid |= LIBNBD_CALLBACK_FREE; + return valid; +} + +/*----- End of prologue. -----*/ + /* STATE MACHINE */ { REPLY.STRUCTURED_REPLY.START: /* We've only read the simple_re...

...; decrement reference counts at the right place to free the closure, > without waiting for the handle to be closed. > > The solution to this is to introduce the concept of a closure > lifetime. The callback is called with an extra valid_flag parameter > which is a bitmap containing LIBNBD_CALLBACK_VALID and/or > LIBNBD_CALLBACK_FREE. LIBNBD_CALLBACK_VALID corresponds to a normal > call of the callback function by the library. After the library has > finished with the callback (declaring that this callback will never be > needed or called again), it is called once more with > valid...

This implements most of what I wrote here: https://www.redhat.com/archives/libguestfs/2019-July/msg00213.html

Patches 1 & 2 are rather complex, but the end result is that we pass closures + user_data + free function in single struct parameters as I described previously in this email: https://www.redhat.com/archives/libguestfs/2019-August/msg00210.html Patch 3 adds a convenient FREE_CALLBACK macro which seems a worthwhile simplification if you buy into 1 & 2. Patch 4 adds another macro which is

As proposed here: https://www.redhat.com/archives/libguestfs/2019-August/msg00130.html I didn't actually read Eric's replies to that yet because I've been concentrating on writing these patches all day. Anyway here they are and I'll look at what Eric said about the proposal next. Rich.

...extern int nbd_set_debug_callback ( struct nbd_handle *h, int (*debug_fn) (int valid_flag, // <-- note void *user_data, const char *context, const char *msg), void *user_data); The extra ‘valid_flag’ contains one or both of LIBNBD_CALLBACK_VALID and LIBNBD_CALLBACK_FREE. Callback code would look like: int my_debug_fn (int valid_flag, void *user_data, const char *context, const char *msg) { if (valid_flag & LIBNBD_CALLBACK_VALID) { // This is the normal callback as before. printf ("debug msg...

v1 was here: https://www.redhat.com/archives/libguestfs/2019-July/thread.html#00231 The changes address everything that Eric picked up in his review of the first two patches. I have also added two more patches (4 and 5) which respectively fix docs and change int status -> unsigned status, as discussed. Passes make, check, check-valgrind. Rich.

...ls are crossing; I had typed this up (so I'm sending now) even > though I already see that you've tweaked things in v2. I've got two > threads of thoughts below. > > > > > (Note it is also possible for the library to call the callback with > > valid_flag == LIBNBD_CALLBACK_VALID|LIBNBD_CALLBACK_FREE, meaning it's the > > last valid call.) > > [1] In my previous reply, I assumed that the nbd_aio_FOO_callback > function would use this paradigm... > > > > +++ b/examples/strict-structured-reads.c > > > @@ -127,11 +131,14 @@ read_chun...

...ator/states-reply-simple.c +++ b/generator/states-reply-simple.c @@ -60,16 +60,16 @@ case 0: /* guaranteed by START */ assert (cmd); - if (cmd->cb.fn.read) { + if (cmd->cb.fn.chunk) { int error = 0; assert (cmd->error == 0); - if (cmd->cb.fn.read (LIBNBD_CALLBACK_VALID|LIBNBD_CALLBACK_FREE, - cmd->cb.fn_user_data, - cmd->data, cmd->count, - cmd->offset, LIBNBD_READ_DATA, &error) == -1) + if (cmd->cb.fn.chunk (LIBNBD_CALLBACK_VALID|LIBNBD_CALLBACK_FREE, +...

...associate a free function with callbacks. Libnbd will call maybe 'an optional free function' > +this function when the callback will not be called again by libnbd. > > -=over 4 > +This can be used to free associated C<user_data>. For example: > > -=item C<LIBNBD_CALLBACK_VALID> > + void *my_data = malloc (...); > + > + nbd_aio_pread_structured (nbd, buf, sizeof buf, offset, > + (nbd_chunk_callback) { .callback = my_fn, > + .user_data = my_data, > + .free = fre...

...re; for nbd_aio_FOO_callback, it may indeed be easier to set VALID|FREE on the single use of the callback at the time it is ready to retire. But this idea does make it possible for libnbd to inform the callback about its last expected use. > > The extra ‘valid_flag’ contains one or both of LIBNBD_CALLBACK_VALID > and LIBNBD_CALLBACK_FREE. Callback code would look like: > > int my_debug_fn (int valid_flag, void *user_data, > const char *context, const char *msg) > { > if (valid_flag & LIBNBD_CALLBACK_VALID) { > // This is the normal callback as be...

...>> * current error rather than any earlier one. If the callback fails >> * without setting errno, then use the server's error below. >> */ >> - if (cmd->cb.fn.read (cmd->cb.fn_user_data, >> + if (cmd->cb.fn.read (LIBNBD_CALLBACK_VALID, cmd->cb.fn_user_data, >> cmd->data + (offset - cmd->offset), >> 0, offset, LIBNBD_READ_ERROR, &scratch) == -1) >> if (cmd->error == 0) > > We could still optimize this file based on NBD_...