similar to: [PATCH nbdkit] server: Fix parameters of lock_request, unlock_request

On Haiku which doesn't have pipe2 we were using the fallback path. However a previous change to remove the conn parameter of lock_request and unlock_request had not been made to this code and so it failed to compile. Fixes: commit 91023f269d4cea56f573a1aa0d880b12052f6e1e --- server/connections.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git

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 previous incomplete patch was here: https://www.redhat.com/archives/libguestfs/2019-September/msg00049.html based on earlier discussion here: https://www.redhat.com/archives/libguestfs/2019-September/msg00047.html In v2: - The previous patch was incomplete. This version completes it by adding tests and extending nbdkit-sh-plugin. - nbdkit_export_name now returns NULL for error,

We noticed while writing various libnbd tests that when the delay filter is in use, there are scenarios where we had to resort to SIGKILL to get rid of nbdkit, because it was non-responsive to SIGINT. I'm still trying to figure out the best way to add testsuite coverage of this, but already proved to myself that it works from the command line, under two scenarios that both used to cause long

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 () > >

v1 was here: https://www.redhat.com/archives/libguestfs/2020-February/msg00081.html v2 replaces struct connection *conn = GET_CONN; with GET_CONN; which sets conn implicitly and asserts that it is non-NULL. If we actually want to test if conn is non-NULL or behave differently, then you must use threadlocal_get_conn() instead, and some existing uses do that. Rich.

The third patch is a large but mechanical change which gets rid of passing around struct connection * entirely within the server, preferring instead to reference the connection through thread-local storage. I hope this is a gateway to simplifying other parts of the code. Rich.

I'm fairly sure that these bugs which appear in the Python plugin: https://bugzilla.redhat.com/show_bug.cgi?id=1566516 https://bugzilla.redhat.com/show_bug.cgi?id=1566522 are really bugs in the SERIALIZE_ALL_REQUESTS thread model. See the first patch for the full explanation. The second patch is a fix for a race condition which is probably nudged into being by the first patch. Now this

v1 -> v2: - Fixed everything mentioned in the review. Rich.

When I added .can_FOO caching in 1.16, I missed the case that the sh plugin itself was calling .can_flush twice in some situations (in order to default .can_fua). Then right after, I regressed it to call .can_zero twice (in order to default .can_fast_zero). I also missed that .thread_model could use better caching, because at the time, I did not add testsuite coverage. Fix that now. Eric Blake

Somewhat invasive but mostly mechanical change to how plugins are called. This patch is in preparation for adding a second backend subtype for filters. Rich.

The messages are not really useful to us, but they do bloat the debugging output of virt-v2v massively: nbdkit: python[1]: debug: acquire global request lock nbdkit: python[1]: debug: acquire per-connection request lock nbdkit: python[1]: debug: acquire unload prevention lock nbdkit: python[1]: debug: pwrite count=2097152 offset=4628414464 fua=0 nbdkit: python[1]: debug: release unload prevention

This is a major rewrite compared to my v1 series, where I've tried a lot harder to ensure that we still accommodate building on Haiku (although I have not actually yet fired up a Haiku VM to try it for myself). I also managed to make the sh plugin fully parallel, on capable platforms. See also my question on patch 10 on whether I've picked the best naming convention. Eric Blake (17):

On platforms that lack atomic CLOEXEC, it's simpler to just always force serialization (no client thread will be executing when nbdkit itself is creating a new file descriptor) than to audit which plugins actually care about not getting any leaked fds. We have one final function that we need to use for atomic CLOEXEC; the next patch will actually put accept4() to use. Maybe this penalization

Haiku unfortunately lacks pipe2, so we have to plan for a fallback at each site that uses it. This also makes a good time to audit all existing users of pipe, to see if they should be using pipe2. The tests fork() but don't fail because of fd leaks; and the nbd plugin doesn't fork() but was merely using pipe2 for convenience over multiple fcntl calls. However, the server's quit_fd

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

The first one is the nastiest - it is an assertion failure caused by a spec-compliant client and introduced by our security fix that was released in 1.14.1. Eric Blake (4): server: Fix regression for NBD_OPT_INFO before NBD_OPT_GO server: Fix back-to-back SET_META_CONTEXT server: Forbid NUL in export and context names server: Fix OPT_GO on different export than SET_META_CONTEXT

These are a couple of patches in preparation for the Block Status implementation. While the patches (especially the second one) are very large they are really just elementary code motion. Rich.

v1 was here: https://www.redhat.com/archives/libguestfs/2019-January/msg00008.html In v2 I have provided two patches: The first patch extends attribute((nonnull)) to most internal functions, but not to the external API. The second patch uses a macro so that attribute((format)) is only used in the public API on GCC or Clang. At least in theory these headers could be used by a C compiler which

Technically, as long as our thread model is SERIALIZE_ALL_REQUESTS, we don't have to be very careful about atomic CLOEXEC on any of the pipes we create for communication with the child. However, the next patch wants to promote sh plugin to parallel when possible, which requires the use of pipe2 to avoid fd leaks. Also, add an assert to ensure that we avoid dup2(n, n) (which would fail to