similar to: [PATCH nbdkit] server: Remove useless thread local sockaddr.

It is never a wise idea to pass stack-allocated storage to another thread during pthread_create() unless you can guarantee that the new thread will complete prior to the calling thread returning and ending the lifetime of that storage. We were violating this, with the result in a SEGV in the detached child thread during threadlocal_set_sockaddr() with parameters pointing into thread_data which

These patches should be ready to go in now; I will also post my work-in-progress for enabling full parallel handling that depends on these, but with that series, I was still getting crashes or hangs with test-socket-activation (I think I've nailed all the crashes I've seen, but the hang is rather insidious; see my other email

We can't guarantee what storage duration the caller's request for a thread name has; and in fact, if the caller uses plugin_name() for their thread name, then the moment .unload is called, our threadlocal storage is pointing to la-la-land and we get a nice SEGV while trying to print any debug message. So copy the user's string instead. Signed-off-by: Eric Blake

If the plugin .pread method did not fill the buffer with data then the contents of the heap could be leaked back to the client. To avoid this create a thread-local data buffer which is initialized to zero and expanded (with zeroes) as required. This buffer is shared between pread and pwrite which makes the code a little bit simpler. Also this may improve locality by reusing the same memory for

Unlike the equivalent change on the client side which caused a dramatic performance improvement, there is no noticable difference from this patch in my testing. --- server/sockets.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/server/sockets.c b/server/sockets.c index 2c71970..b25405c 100644 --- a/server/sockets.c +++ b/server/sockets.c @@ -37,13 +37,15 @@

Annotate some function parameters with attribute((nonnull)). Only do this for internal headers where we are sure that we will be using sufficiently recent GCC or Clang. For the public header files (ie. include/nbdkit-*.h) it may be that people building out of tree plugins are using old GCC which had problems, or even other compilers that don't support this extension at all. Libvirt has an

--- lib/Makefile.am | 1 + server/internal.h | 7 +++++ server/main.c | 5 +++ server/nbdkit.syms | 5 --- server/plugins.c | 2 +- server/public.c | 21 +++---------- server/quit.c | 2 +- lib/lib.h | 15 +++++++++ lib/init.c | 15 +++++++++ lib/slow.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++ lib/libnbdkit.syms | 5 +++ 11 files

For GCC only, define unlikely() macro. Use it on error paths to move code out of the hot path. In the server only, use the debug() macro (don't call nbdkit_debug directly). This macro checks the verbose flag and moves the call to nbdkit_debug out of the hot path. --- server/connections.c | 11 ++++++----- server/internal.h | 17 ++++++++++++++++- server/plugins.c | 2 +-

On Haiku tests/test-socket-activation failed with: nbdkit: locks.c:96:lock_request: conn != NULL called from server/sockets.c: accept_connection in the fallback path which does: lock_request (); thread_data->sock = set_cloexec (accept (listen_sock, NULL, NULL)); unlock_request () Because there is no current connection in this thread this code fails. However it should be possible to

The plugin interface was impliticly using the last errno value as the source for any error code sent over the wire to the client. This is okay for C, but in other language bindings, it gets rather awkward when you can't guarantee what errno will even be set to by the time the binding glue has finished executing. The solution is to expose an explicit utility function for setting the preferred

I think I understand now that libnbdkit.so won't break the ABI for existing plugins. Does it require that plugins for newer nbdkit use -lnbdkit (which would be a source API break) or would it still be possible to compile without this? I guess as long as plugins do not start using -no-undefined then it would still work, so it wouldn't be a source API break. I had a look into how we might

Works essentially just like calling getpeername(2), because that's how it is implemented. --- TODO | 6 ++++++ docs/nbdkit-plugin.pod | 23 +++++++++++++++++++++++ include/nbdkit-common.h | 2 ++ server/nbdkit.syms | 1 + server/public.c | 21 +++++++++++++++++++++ 5 files changed, 53 insertions(+) diff --git a/TODO b/TODO index 04def3c..d2cf0ae 100644 ---

On 4/28/20 11:45 AM, Richard W.M. Jones wrote: > On Haiku tests/test-socket-activation failed with: > > nbdkit: locks.c:96:lock_request: conn != NULL > > called from server/sockets.c: accept_connection > in the fallback path which does: > lock_request (); > thread_data->sock = set_cloexec (accept (listen_sock, NULL, NULL)); > unlock_request () > >

This fixes the long-standing crash on close when nbdkit exits. I did try first to fix threads so we're using a proper thread pool, but that's difficult to implement. So this does the minimal change needed to fix the crash instead. There are still two segfaults that happen during running the test suite. One is deliberately caused (tests/test-captive.sh). The other appears to be an

GCC is concerned that if we never go round the loop then fd will be uninitialized. By asserting that getaddrinfo set result != NULL we can avoid this. nbd.c: In function ‘nbd_open_handle’: nbd.c:974:5: error: ‘fd’ may be used uninitialized in this function [-Werror=maybe-uninitialized] 974 | close (fd); | ^~~~~~~~~~ nbd.c:954:7: note: ‘fd’ was declared here 954 | int fd;

New nbdkit_peer_pid, nbdkit_peer_uid and nbdkit_peer_gid calls can be used on Linux (only) to read the peer PID, UID and GID from clients connected over a Unix domain socket. This can be used in the preconnect phase to add additional filtering. One use for this is to add an extra layer of authentication for local connections. A subsequent commit will enhance the now misnamed nbdkit-ip-filter to

Version 1 was here: https://www.redhat.com/archives/libguestfs/2019-April/msg00144.html Version 2 makes a couple of much larger changes: The OCaml patch changes the API of the pread method so it matches what other language bindings are already doing, ie. get the language plugin to return a newly allocated buffer, check it is long enough, copy out the data. The server patch implements a

There's enough here to need a review; some of it probably needs backporting to stable-1.12. This probably breaks tests on Haiku or other platforms that have not been as on-the-ball about atomic CLOEXEC; feel free to report issues that arise, and I'll help come up with workarounds (even if we end up leaving a rare fd leak on less-capable systems). Meanwhile, I'm still working on my

--- .../reflection/nbdkit-reflection-plugin.pod | 23 ++++- plugins/reflection/reflection.c | 88 +++++++++++++++++++ tests/Makefile.am | 2 + tests/test-reflection-address.sh | 63 +++++++++++++ 4 files changed, 174 insertions(+), 2 deletions(-) diff --git a/plugins/reflection/nbdkit-reflection-plugin.pod

From: Yifan Gu <gyf304@gmail.com> This patch adds in indirection for API functions, as PE DLL loader does not resolve undefined symbols. This patch only includes header changes. --- include/nbdkit-common.h | 133 +++++++++++++++++++++++++++++++++++++++- include/nbdkit-compat.h | 97 +++++++++++++++++++++++++++++ include/nbdkit-filter.h | 39 ++++++++++++ include/nbdkit-plugin.h | 27