search for: completion_cb

...tests/closure-lifetimes.c +++ b/tests/closure-lifetimes.c @@ -38,50 +38,59 @@ static char *nbdkit_delay[] = "delay-read=10", NULL }; -static unsigned debug_fn_valid; -static unsigned debug_fn_free; -static unsigned read_cb_valid; -static unsigned read_cb_free; -static unsigned completion_cb_valid; -static unsigned completion_cb_free; +static unsigned debug_fn_called; +static unsigned debug_fn_freed; +static unsigned read_cb_called; +static unsigned read_cb_freed; +static unsigned completion_cb_called; +static unsigned completion_cb_freed; static int -debug_fn (unsigned valid_flag,...

...it under the terms of the GNU Lesser General Public @@ -42,6 +42,8 @@ static unsigned debug_fn_called; static unsigned debug_fn_freed; static unsigned read_cb_called; static unsigned read_cb_freed; +static unsigned block_status_cb_called; +static unsigned block_status_cb_freed; static unsigned completion_cb_called; static unsigned completion_cb_freed; @@ -74,6 +76,21 @@ read_cb_free (void *opaque) read_cb_freed++; } +static int +block_status_cb (void *opaque, const char *meta, uint64_t offset, + uint32_t *entries, size_t nr_entries, int *error) +{ + assert (!block_status_cb_fre...

...tests/closure-lifetimes.c +++ b/tests/closure-lifetimes.c @@ -38,50 +38,58 @@ static char *nbdkit_delay[] = "delay-read=10", NULL }; -static unsigned debug_fn_valid; -static unsigned debug_fn_free; -static unsigned read_cb_valid; -static unsigned read_cb_free; -static unsigned completion_cb_valid; -static unsigned completion_cb_free; +static unsigned debug_fn_called; +static unsigned debug_fn_freed; +static unsigned read_cb_called; +static unsigned read_cb_freed; +static unsigned completion_cb_called; +static unsigned completion_cb_freed; static int -debug_fn (unsigned valid_flag,...

As promised in my earlier thread on libnbd completion callback question. Eric Blake (2): generator: Refactor handling of closures in unlocked functions generator: Free closures on failure docs/libnbd.pod | 2 +- generator/C.ml | 48 +++++++++++------ generator/C.mli | 1 + lib/debug.c | 7 +-- lib/opt.c | 31 ++++++-----

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.

...int signal) > +{ Is gcc -Wshadow going to annoy us by this choice of naming? I'd use signum, if we're worried. > @@ -149,6 +150,7 @@ main (int argc, char *argv[]) > read_cb, NULL, > completion_cb, NULL, 0); > if (cookie == -1) NBD_ERROR; > + nbd_kill_command (nbd, 0); This looks a bit funny until I read the docs at [1]. When using kill(2), I'm used to the function call 'kill(pid, 0)' probing for process existence. But on the command line, kill(1) has the behavior of...

...ount, > + uint64_t offset, unsigned status, int *error) > +{ Same here. > + if (valid_flag & LIBNBD_CALLBACK_VALID) > + read_cb_valid++; > + if (valid_flag & LIBNBD_CALLBACK_FREE) > + read_cb_free++; > + return 0; > +} > + > +static int > +completion_cb (unsigned valid_flag, void *opaque, > + int64_t cookie, int *error) > +{ and again > + if (valid_flag & LIBNBD_CALLBACK_VALID) > + completion_cb_valid++; > + if (valid_flag & LIBNBD_CALLBACK_FREE) > + completion_cb_free++; > + return 0; > +}...

...n (int argc, char *argv[]) if (nbd == NULL) NBD_ERROR; if (nbd_connect_command (nbd, nbdkit) == -1) NBD_ERROR; - cookie = nbd_aio_pread_structured_callback (nbd, buf, sizeof buf, 0, - read_cb, NULL, - completion_cb, NULL, 0); + cookie = nbd_aio_pread_structured (nbd, buf, sizeof buf, 0, + read_cb, NULL, + completion_cb, NULL, 0); if (cookie == -1) NBD_ERROR; assert (read_cb_free == 0); assert (completion_cb_free == 0); @@ -143,9...

...;, "-v", "--filter=delay", "null", "size=512", - "delay-read=3", + "delay-read=10", NULL }; static unsigned debug_fn_valid; @@ -134,6 +134,7 @@ main (int argc, char *argv[]) assert (read_cb_free == 1); assert (completion_cb_free == 1); + nbd_kill_command (nbd, 0); nbd_close (nbd); /* Test command callbacks are freed if the handle is closed without @@ -149,6 +150,7 @@ main (int argc, char *argv[]) read_cb, NULL, com...

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

...; res = nbd_aio_pread_callback(h, buf, count, offset, NULL, free_cb, wrap, flags); } else /* in nbd_aio_pread_callback */ { wrap->callback = ...; res = nbd_aio_pread_callback(h, buf, count, offset, completion_cb, free_cb, wrap, flags); } where completion_cb(wrap) calls the OCaml callback (and doesn't get used if there is no OCaml callback), and where free_cb(wrap) gets called at the end of the lifetime for buf, wrap->callback (if any), AND wrap itself, so that we have...

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

This applies on top of the OClosure v2 series posted a few minutes ago. Rich.

...ree == 1); debug_fn_free = 0; @@ -117,8 +117,9 @@ main (int argc, char *argv[]) if (nbd_connect_command (nbd, nbdkit) == -1) NBD_ERROR; cookie = nbd_aio_pread_structured (nbd, buf, sizeof buf, 0, - read_cb, NULL, - completion_cb, NULL, 0); + (nbd_chunk_callback) { .callback = read_cb }, + (nbd_completion_callback) { .callback = completion_cb }, + 0); if (cookie == -1) NBD_ERROR; assert (read_cb_free == 0); a...

...err.value = errno.ENOMEM assert user_data == 42 diff --git a/tests/closure-lifetimes.c b/tests/closure-lifetimes.c index 3ddf4c1..b225d68 100644 --- a/tests/closure-lifetimes.c +++ b/tests/closure-lifetimes.c @@ -73,8 +73,7 @@ read_cb (unsigned valid_flag, void *opaque, } static int -completion_cb (unsigned valid_flag, void *opaque, - int64_t cookie, int *error) +completion_cb (unsigned valid_flag, void *opaque, int *error) { assert (completion_cb_free == 0); diff --git a/tests/server-death.c b/tests/server-death.c index 56e7e51..7854527 100644 --- a/tests/server-death.c...

...nbdkit", "-s", "--exit-with-parent", "-v", + "null", "size=512", NULL }; + +static unsigned debug_fn_valid; +static unsigned debug_fn_free; +static unsigned read_cb_valid; +static unsigned read_cb_free; +static unsigned completion_cb_valid; +static unsigned completion_cb_free; + +static int +debug_fn (unsigned valid_flag, void *opaque, + const char *context, const char *msg) +{ + if (valid_flag & LIBNBD_CALLBACK_VALID) + debug_fn_valid++; + if (valid_flag & LIBNBD_CALLBACK_FREE) + debug_fn_free++; + re...

On 8/12/19 11:08 AM, Richard W.M. Jones wrote: > This adds a C-only semi-private function for freeing various types of > persistent data passed to libnbd. > > There are some similarities with nbd_add_close_callback which we > removed in commit 7f191b150b52ed50098976309a6af883d245fc56. > --- > +=head1 FREE CALLBACKS > + > +B<Note:> The API described in this