search for: nbd_aio_pread_structured_callback

...nbdkit) == 0); Side effects in an assert. Nasty. (Maybe you can get away with it if we explicitly #undef NDEBUG at the top of this file, so the test still works if someone does ./configure CFLAGS=-DNDEBUG, but splitting the side effects from the test validation seems wise) > + > + cookie = nbd_aio_pread_structured_callback (nbd, buf, sizeof buf, 0, > + read_cb, NULL, > + completion_cb, NULL, 0); > + assert (read_cb_free == 0); > + assert (completion_cb_free == 0); > + while (!nbd_aio_command_completed (nbd, coo...

...sure, and currently don't free that C struct until nbd_close. Basically, we have: nbd_internal_py_aio_pread_structured data = malloc() nbd_aio_pread_structured(, chunk_wrapper, data) nbd_add_close_callback(data_free, data) nbd_internal_py_aio_pread_structured_callback data = malloc() nbd_aio_pread_structured_callback(, chunk_wrapper, callback_wrapper, data) nbd_add_close_callback(data_free, data) where there is no convenient way for the generator to insert cleanup for the data malloc'd in aio_pread_structured (for aio_pread_structured_callback, it could use the callback_wrapper - but handling the two API...

On 7/17/19 8:00 AM, Richard W.M. Jones wrote: > --- > .gitignore | 1 + > README | 2 + > configure.ac | 9 + > examples/Makefile.am | 22 ++ > examples/glib-main-loop.c | 511 ++++++++++++++++++++++++++++++++++++++ > 5 files changed, 545 insertions(+) Looks good. > > + revents = g_source_query_unix_fd

...ength-4) / 4, &error) == -1) >> if (cmd->error == 0) > > Hmm - no change to the FINISH state, which means you are relying on > command retirement to free chunk/extent instead. As long as that > happens, we should be okay, though. Another thought: if a user calls nbd_aio_pread_structured_callback (..., chunk, &data, callback, &data, ...), where chunk ignores FREE but callback(FREE) calls free(data), is there any problem with the fact that we may end up with a call order of callback(VALID|FREE) before chunk(FREE)? I think we're okay - as long as chunk doesn't dereference dat...

Some callers of nbd_aio_pread_structured_callback or nbd_aio_block_status_callback share the same opaque pointer to both calls, with the intent to only free the pointer on the completion callback. Although our documentation does not explicitly specify the order in which callbacks are made, allowing the callback(FREE) to occur prior to the read(FR...

...s.c +++ b/examples/strict-structured-reads.c @@ -235,9 +235,9 @@ main (int argc, char *argv[]) *r = (struct range) { .first = offset, .last = offset + maxsize, }; *d = (struct data) { .offset = offset, .count = maxsize, .flags = flags, .remaining = r, }; - if (nbd_aio_pread_structured_callback (nbd, buf, sizeof buf, offset, - read_chunk, d, read_verify, d, - flags) == -1) { + if (nbd_aio_pread_structured (nbd, buf, sizeof buf, offset, + read_chunk, d, read_verify, d, +...

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.

...s.c +++ b/examples/strict-structured-reads.c @@ -224,8 +224,8 @@ main (int argc, char *argv[]) *r = (struct range) { .first = offset, .last = offset + maxsize, }; *d = (struct data) { .offset = offset, .count = maxsize, .flags = flags, .remaining = r, }; - if (nbd_aio_pread_structured_callback (nbd, buf, sizeof buf, offset, d, - read_chunk, read_verify, + if (nbd_aio_pread_structured_callback (nbd, buf, sizeof buf, offset, + read_chunk, read_verify, d, flags)...

Mutable (Int n) => int *n This can currently only be used for callback arguments of type int (not for other types, nor for any ordinary function arguments), but it could be implemented more generally in future. --- generator/generator | 75 +++++++++++++++++++++++++++++++++++++-------- 1 file changed, 63 insertions(+), 12 deletions(-) diff --git a/generator/generator b/generator/generator

...} @@ -235,6 +227,7 @@ main (int argc, char *argv[]) *r = (struct range) { .first = offset, .last = offset + maxsize, }; *d = (struct data) { .offset = offset, .count = maxsize, .flags = flags, .remaining = r, }; + /* XXX *d and *r are both leaked. */ if (nbd_aio_pread_structured_callback (nbd, buf, sizeof buf, offset, read_chunk, d, read_verify, d, flags) == -1) { diff --git a/generator/generator b/generator/generator index a0322ee..0187c02 100755 --- a/generator/generator +++ b/generator/genera...

...g_callback (nbd, debug_fn, NULL); + assert (debug_fn_free == 1); + + nbd_close (nbd); + assert (debug_fn_free == 2); + + /* Test command callbacks are freed when the command is retired. */ + nbd = nbd_create (); + assert (nbd); + assert (nbd_connect_command (nbd, nbdkit) == 0); + + cookie = nbd_aio_pread_structured_callback (nbd, buf, sizeof buf, 0, + read_cb, NULL, + completion_cb, NULL, 0); + assert (read_cb_free == 0); + assert (completion_cb_free == 0); + while (!nbd_aio_command_completed (nbd, cookie)) + assert (nbd_po...

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.

...ck' is the best approach or most idiomatic OCaml - but hey, it taught me a lot about OCaml. Writing a 'string * callback list' proved to be a rather interesting exercise] Our C code has a couple of functions that want to take multiple callback functions that share a single opaque data (nbd_aio_pread_structured_callback; nbd_aio_block_status_callback). However, the mapping of this construct to Python passed only the opaque handle of the first function to the second callback wrapper, which means we would invoke the wrong Python Callable; better is tracking a single malloc()d structure containing the Python opaque o...

Posting now that I got something to compile (at the expense of breaking OCaml bindings), but I'm open to ideas on how to improve it. Eric Blake (2): generator: Tweak print_c_arg_list to take alternate first arg RFC: generator: Handle shared callbacks in Python generator/generator | 556 ++++++++++++++++++++++---------------------- 1 file changed, 280 insertions(+), 276 deletions(-) --

...ads.c index 92eb3e6..a50f662 100644 --- a/examples/strict-structured-reads.c +++ b/examples/strict-structured-reads.c @@ -225,7 +225,7 @@ main (int argc, char *argv[]) *d = (struct data) { .offset = offset, .count = maxsize, .flags = flags, .remaining = r, }; if (nbd_aio_pread_structured_callback (nbd, buf, sizeof buf, offset, - read_chunk, read_verify, d, + read_chunk, d, read_verify, d, flags) == -1) { fprintf (stderr, "%s\n", nbd_get_error ());...

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.

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