similar to: [PATCH libnbd 0/7] Add free callbacks and remove valid_flag.

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

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

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

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

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

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

On Mon, Jul 22, 2019 at 10:08:25AM +0100, Richard W.M. Jones wrote: > On Sat, Jul 20, 2019 at 07:38:45AM +0100, Richard W.M. Jones wrote: > > More thoughts on callbacks, etc. following on from: > > https://www.redhat.com/archives/libguestfs/2019-July/thread.html#00184 > > > > Closure lifetimes > > ----------------- Here's a possibly better idea which still

More thoughts on callbacks, etc. following on from: https://www.redhat.com/archives/libguestfs/2019-July/thread.html#00184 Closure lifetimes ----------------- Closures could have a lifetime if we had a little bit of support from the C library. We would generate (from C only): nbd_set_free_<fn>_<closure> (nbd, free_closure); which calls free_closure (user_data) as soon as the

The freeing feature can now be done by associating a free callback with a closure's user_data, so having the valid_flag is no longer necessary and it can be completely removed. This mostly reverts commit 2d9b98e96772e282d51dafac07f16387dadc8afa. --- TODO | 2 - docs/libnbd.pod | 64 ++-------------- examples/glib-main-loop.c |

On 7/24/19 11:54 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

On 7/25/19 8:07 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

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. --- generator/generator | 57 ++++++++++++++++++++ lib/Makefile.am | 1 + lib/free.c | 129 ++++++++++++++++++++++++++++++++++++++++++++ lib/handle.c

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

This is a combination of these two earlier series: https://www.redhat.com/archives/libguestfs/2019-August/msg00235.html https://www.redhat.com/archives/libguestfs/2019-August/msg00240.html plus changes to allow .callback = NULL / .free != NULL, and to reduce the complexity of freeing callbacks. Although it's rather long there's nothing complex here. We might consider squashing some

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.

For example nbd_set_debug takes a callback function. Previously this was defined explicitly inside the function parameters. This commit defines a new public typedef: typedef int (*nbd_debug_callback) (unsigned valid_flag, void *user_data, const char *context, const char *msg); and then uses the typedef like this: extern int nbd_set_debug_callback

Mechanical change: Wherever we call any closure with the LIBNBD_CALLBACK_FREE function, we also call nbd_internal_free_callback with the closure's user_data. This allows calls to associate a free callback with any closure via its user_data pointer. --- generator/states-reply-simple.c | 1 + generator/states-reply-structured.c | 24 ++++++++++++++++++------ generator/states-reply.c

On Mon, Aug 12, 2019 at 11:29:10AM -0500, Eric Blake wrote: > On 8/12/19 11:08 AM, Richard W.M. Jones wrote: > > + typedef void (*nbd_free_callback) (void *ptr, void *user_data); > > + int nbd_add_free_callback (struct nbd_handle *h, > > + void *ptr, > > + nbd_free_callback cb, > > +