similar to: [PATCH libnbd] lib: Kill subprocess in nbd_close.

$ time nbdsh -c 'h.connect_command (["nbdkit", "-s", "null", "size=512", "--filter=delay", "delay-read=10"]); b = nbd.aio_buffer(1); h.aio_pread (b, 0); del (h)' real 0m10.499s user 0m0.065s sys 0m0.023s With this patch the elapsed time is near instantaneous. --- lib/handle.c | 4 +++- 1 file changed, 3 insertions(+), 1

On 7/25/19 12:43 PM, Richard W.M. Jones wrote: > $ time nbdsh -c 'h.connect_command (["nbdkit", "-s", "null", "size=512", "--filter=delay", "delay-read=10"]); b = nbd.aio_buffer(1); h.aio_pread (b, 0); del (h)' > real 0m10.499s > user 0m0.065s > sys 0m0.023s > > With this patch the elapsed time is near

This doesn't yet work. However it does make me more convinced than ever that we really need to sort out persistent buffer lifetimes in the library (similar to what we did for closures). Rich.

This applies on top of patches 1 & 2 here (instead of patch 3): https://www.redhat.com/archives/libguestfs/2019-May/msg00206.html https://www.redhat.com/archives/libguestfs/2019-May/msg00207.html Rich.

RFC because this is an API break, but we haven't declared stable API yet. If we like it, I'm working on using libnbd to implement the nbdkit-nbd plugin; knowing whether it is API version 0.1 or 0.2 will be useful. I also dabbled with allowing optional parameters in python, although my OCaml is weak enough that there may be cleaner ways to approach that. Eric Blake (4): api: Add flags

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 2af25fe..5ad818e 100644 --- a/lib/handle.c +++ b/lib/handle.c @@ -315,6 +315,8 @@

v1 was here: https://www.redhat.com/archives/libguestfs/2019-August/msg00103.html In v2 I've made several changes: - Fix Python callbacks so if they don't return something which is int-like, we assume they mean to return 0. - Add nbd.Buffer free() method. Read commit message in patch 2 to see what this is about. - Fixed the asynch copy test to deal with the unbelievably

Create a class for AIO buffers. This is a mostly neutral code refactoring, but we add a convenient function for getting the size of the buffer (same as previous commit for OCaml). --- generator/generator | 111 +++++++++++++++++------------ python/handle.c | 26 +++++-- python/t/500-aio-pread.py | 4 +- python/t/505-aio-pread-callback.py | 10 +--

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 | 46 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/generator/generator b/generator/generator

By using a special type we can more naturally express flags in different programming languages. For example OCaml will prefer an optional argument containing a list of flags, defaulting to the empty list: pread [...] ?(flags = []) --- generator/generator | 66 +++++++++++++++++++++++++----------- python/t/400-pread.py | 2 +- python/t/410-pwrite.py | 2 +-

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

This doesn't include Eric's new APIs, but if you push those then I can rebase this one on top. Rich.

v1 was posted here: https://www.redhat.com/archives/libguestfs/2019-September/thread.html#00337 v2: - Drop the first patch. - Hopefully fix the multiple issues with fork-safety and general behaviour on error paths. Note this requires execvpe for which there seems to be no equivalent on FreeBSD, except some kind of tedious path parsing (but can we assign to environ?) Rich.

On 7/25/19 3:56 AM, Martin Kletzander wrote: > On Thu, Jul 25, 2019 at 09:41:01AM +0100, Richard W.M. Jones wrote: >> On Wed, Jul 24, 2019 at 03:14:28PM -0500, Eric Blake wrote: >>> Closing the handle in one thread while another thread is locked causes >>> undefined behavior (as closing does not obtain a lock, and can cause >>> use-after-free or other nasty

V3 was here: <http://mid.mail-archive.com/20230215141158.2426855-1-lersek at redhat.com>. See the Notes section on each patch for the v4 updates. The series is nearly ready for merging: every patch has at least one R-b tag, except "socket activation: avoid manipulating the sign bit". The series builds, and passes "make check" and "make check-valgrind", at

On Wed, Jul 24, 2019 at 03:14:28PM -0500, Eric Blake wrote: > Closing the handle in one thread while another thread is locked causes > undefined behavior (as closing does not obtain a lock, and can cause > use-after-free or other nasty problems to the thread that does own the > lock). However, it is not sensible to try and obtain a lock in > nbd_close, as POSIX says that it is also

Patch 1/6 was posted before and I didn't change it: https://www.redhat.com/archives/libguestfs/2019-May/thread.html#00134 That doesn't necessarily mean I shouldn't change it, I'm posting it again because the other patches depend on it. The main change in this series is we add three new API functions: nbd_aio_is_created - connection has just been created

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

Closing the handle in one thread while another thread is locked causes undefined behavior (as closing does not obtain a lock, and can cause use-after-free or other nasty problems to the thread that does own the lock). However, it is not sensible to try and obtain a lock in nbd_close, as POSIX says that it is also undefined for any other thread to wait on a mutex that has already been destroyed.

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