similar to: [PATCH libnbd v2 00/10] Callbacks and OCaml and Python persistent buffers.

In this patch series we use the newly introduced free callback on the completion function to dererence the OCaml NBD.Buffer. I will make the same kind of change for Python later in a separate series. The completion function is always called at the C level, even if the OCaml program didn't use the optional argument. That's because the free callback doesn't run otherwise. There is a

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

Basically the same as this patch series, but for Python: https://www.redhat.com/archives/libguestfs/2019-August/msg00235.html plus adding the 590 asynch test at the end. Rich.

Patches 1-4 are basically uncontroversial, straightforward refactoring and IMHO we should just push them. Possibly 1-3 should be squashed together, but I posted them separately so they are easier to review. Patches 5 and 6 together implement OClosure. Patch 5 adds the feature and is simple to understand. Patch 6 changes the Closure completion callbacks into OClosure, but because it doesn't

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 ++++++-----

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

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.

I'm posting this now, as I'm at the end of a workday and I got things working for manual experimentation. Still to do: - write interop tests for qemu-nbd and nbdkit (including my proposed patch addition of qemu-nbd -A to show qemu:allocation-depth) - figure out if we can make 'nbdinfo --map' use the new API to automatically select all contexts advertised by the server Eric Blake

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.

In v2: - now based on my proposal to add LIBNBD_SHUTDOWN_IMMEDIATE - four flags instead of two: STRICT_FLAGS is new (patch 4), and STRICT_BOUNDS is separate from STRICT_ZERO_SIZE (patch 5) - various refactorings for more shared code and less duplication Eric Blake (5): api: Add xxx_MASK constant for each Flags type generator: Refactor filtering of accepted OFlags api: Add

When we introduced valid_flags, there was an incentive to do as few callbacks as possible, favoring cb(VALID|FREE) calls over the sequence cb(VALID);cb(FREE). To make it work, we set .callback=NULL after an early free, so that the later check during retirement didn't free again. But now that our .free callback is distinct from our other callbacks, there is no longer an advantage to bundling

v1 was here: https://www.redhat.com/archives/libguestfs/2019-August/msg00168.html I pushed uncontroversial patches 1-4 v2: - The implementation of OClosure (new patch 1) in Python is fixed. - Patch 2 (old patch 5) is unchanged. - I added a new API for removing debug callbacks. I think this approach has some advantages over using OClosure. - I didn't yet do any work on changing the

On 8/13/19 5:06 AM, Richard W.M. Jones wrote: > An optional Closure parameter, but otherwise works the same way as > Closure. > @@ -3778,6 +3777,7 @@ let generate_lib_api_c () = > ) args; > List.iter ( > function > + | OClosure { cbname } -> pr ", %s_callback ? \"<fun>\" : \"NULL\"" cbname Well, it also permits a

In v2: ack'ed preliminary patches have been pushed, and I've added a lot of testsuite coverage as well as putting the new API to use in nbdinfo. Eric Blake (2): api: Add nbd_opt_list_meta_context info: List available meta-contexts lib/internal.h | 1 + generator/API.ml | 84 ++++++++-

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

Inspection of the generated code showed several places where we did not release references on all error paths. In particular, switching things to always jump to an out: label, even when the underlying C function cannot fail, makes it easier to clean up when the user passes wrong types to a function call. One of the easiest triggers without this patch was this one-liner: $ nbdsh -c

As before, but this one has working Python bindings. OCaml still TBD. Rich.

These are some simplifications to the generator. They don't probably make much sense on their own, but they are preparatory to better handling of enums, and or'd lists of flags. Rich.

We have defined the concept of a "null callback" meaning one where the .callback field = NULL. The first two new macros just test this property, and the third one sets a callback to null. This change is neutral refactoring. --- generator/generator | 4 ++-- generator/states-reply-simple.c | 2 +- generator/states-reply-structured.c | 11 ++++++-----

On 7/24/19 7:17 AM, Richard W.M. Jones wrote: > Previously closures had a crude flag which tells if they are > persistent or transient. Transient closures (flag = false) last for > the lifetime of the currently called libnbd function. Persistent > closures had an indefinite lifetime which could last for as long as > the handle. In language bindings handling persistent closures