similar to: [PATCH libnbd] api: Rename nbd_kill_command -> nbd_kill_subprocess.

Reverts commit 387cbe67c3db27e8a61117fedb6e7fad76e409ef. --- generator/generator | 18 +++++++++++++++++- lib/handle.c | 28 +++++++++++++++++++++++++++- tests/closure-lifetimes.c | 4 +++- 3 files changed, 47 insertions(+), 3 deletions(-) diff --git a/generator/generator b/generator/generator index 2cd83f1..25e4aa5 100755 --- a/generator/generator +++ b/generator/generator

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 |

We can easily demonstrate a memory leak when repeatedly calling nbd_aio_pread from nbdsh in the wrong state. True, that isn't something a sane client would normally do, but it's worth fixing. The culprit: when nbd_aio_pread returns a cookie, we guarantee to clean up the closure, but if we fail early and never scheduled the command, nothing ever cleans up the closure. We could document

Change the way that we deal with freeing closures in language bindings: * The valid_flag is removed (mostly reverting commit 2d9b98e96772e282d51dafac07f16387dadc8afa). * An extra ‘free’ parameter is added to all callback structures. This is called by the library whenever the closure won't be called again (so the user_data can be freed). This is analogous to valid_flag ==

On Thu, Aug 15, 2019 at 07:01:49AM -0500, Eric Blake wrote: > We still had another potential API change to squeeze into 0.9.8: > Right now, the API is overloading 'command': > nbd_connect_command/nbd_kill_command > vs. > nbd_aio_command_completed/nbd_aio_peek_command_completed > > Keeping nbd_connect_command may be okay (it takes a command line to > create a

On 7/25/19 2:15 PM, Richard W.M. Jones wrote: > Reverts commit 387cbe67c3db27e8a61117fedb6e7fad76e409ef. > --- > generator/generator | 18 +++++++++++++++++- > lib/handle.c | 28 +++++++++++++++++++++++++++- > tests/closure-lifetimes.c | 4 +++- > 3 files changed, 47 insertions(+), 3 deletions(-) tests/server-death.c can be simplified (no longer has to use

On 8/15/19 4:56 AM, Richard W.M. Jones wrote: > --- > generator/generator | 25 +++++++++++++++++++++++++ > 1 file changed, 25 insertions(+) > > diff --git a/generator/generator b/generator/generator > index 1252bdb..13cb0b9 100755 > --- a/generator/generator > +++ b/generator/generator > @@ -3121,6 +3121,31 @@ let () = > failwithf "%s: first_version

On 7/25/19 4:11 PM, Richard W.M. Jones wrote: > On Thu, Jul 25, 2019 at 03:19:23PM -0500, Eric Blake wrote: >> 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 >> sending SIGTERM by default when you

On Thu, Jul 25, 2019 at 03:19:23PM -0500, Eric Blake wrote: > 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 > sending SIGTERM by default when you omit a signal number (and not > serving as a process existence

On 8/13/19 5:36 PM, Richard W.M. Jones wrote: > Change the way that we deal with freeing closures in language > bindings: > > * The valid_flag is removed (mostly reverting > commit 2d9b98e96772e282d51dafac07f16387dadc8afa). > > * An extra ‘free’ parameter is added to all callback structures. This > is called by the library whenever the closure won't be called

On Fri, Sep 11, 2020 at 09:31:11AM -0500, Eric Blake wrote: > As mentioned in commits 176fc4ea and 609c25f0, our original plan in > adding a flags argument to nbd_shutdown was to let us specify > different behaviors at the libnbd level, rather than NBD protocol > flags (for that, the user has nbd_aio_disconnect). But when we later > parameterized OFlags to accept various bitmasks

This is my proposal for fixing the documentation to match practice (namely, that completion.callback is not invoked in the cases where the aio call itself reports errors); we could instead try to go the other direction and tweak the generator to guarantee that both completion.callback and completion.free are reached no matter what, but that felt more invasive to me. Eric Blake (2): api: Tighten

‘make clean && make’ was not rebuilding the docs/*.3 files. The reason is obscure: - docs/Makefile has rules: MANS = $(man_MANS) all: all-am all-am: Makefile $(MANS) - sinclude docs/Makefile.inc happened long after MANS is defined, so MANS held the earlier version of $(man_MANS) without the api-built man pages listed. This was confirmed by looking at the output of

As mentioned in commits 176fc4ea and 609c25f0, our original plan in adding a flags argument to nbd_shutdown was to let us specify different behaviors at the libnbd level, rather than NBD protocol flags (for that, the user has nbd_aio_disconnect). But when we later parameterized OFlags to accept various bitmasks (commit f891340b), we failed to mark nbd_shutdown as using a different bitmask than

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

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

On 9/26/19 11:40 AM, Richard W.M. Jones wrote: > If the user calls nbd_kill_subprocess, we shouldn't kill the process > again when we close the handle (since the process has likely gone and > we might be killing a different process). > --- > lib/handle.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/lib/handle.c b/lib/handle.c > index

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.

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 was wasteful as we needed to register a "close callback" to

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