search for: unlock_connect

...ection, run a single connection, free the backend, and then try to unlock the connection. Unfortunately the unlock operation has to check the thread model again which fails because the backend has gone away: Program terminated with signal SIGSEGV, Segmentation fault. #0 0x00000000004070ce in unlock_connection () at locks.c:59 59 int thread_model = backend->thread_model (backend); [Current thread is 1 (Thread 0x7fab43243700 (LWP 6676))] (gdb) bt #0 0x00000000004070ce in unlock_connection () at locks.c:59 #1 0x00000000004061b1 in handle_single_connection (sockin=<optimized out>,...

...rkers == 1) nworkers = 0; conn = new_connection (sockin, sockout, nworkers); if (!conn) @@ -287,9 +287,9 @@ handle_single_connection (int sockin, int sockout) { int r; - plugin_lock_connection (); + lock_connection (); r = _handle_single_connection (sockin, sockout); - plugin_unlock_connection (); + unlock_connection (); return r; } @@ -740,12 +740,12 @@ negotiate_handshake (struct connection *conn) { int r; - plugin_lock_request (conn); + lock_request (conn); if (!newstyle) r = _negotiate_handshake_oldstyle (conn); else r = _negotiate_handshake_newstyle...

...rkers == 1) nworkers = 0; conn = new_connection (sockin, sockout, nworkers); if (!conn) @@ -287,9 +287,9 @@ handle_single_connection (int sockin, int sockout) { int r; - plugin_lock_connection (); + lock_connection (); r = _handle_single_connection (sockin, sockout); - plugin_unlock_connection (); + unlock_connection (); return r; } @@ -740,12 +740,12 @@ negotiate_handshake (struct connection *conn) { int r; - plugin_lock_request (conn); + lock_request (conn); if (!newstyle) r = _negotiate_handshake_oldstyle (conn); else r = _negotiate_handshake_newstyle...

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

...odel (void) void lock_connection (void) { - if (thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_CONNECTIONS) { - debug ("acquire connection lock"); + if (thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_CONNECTIONS) pthread_mutex_lock (&connection_lock); - } } void unlock_connection (void) { - if (thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_CONNECTIONS) { - debug ("release connection lock"); + if (thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_CONNECTIONS) pthread_mutex_unlock (&connection_lock); - } } void lock_request (struct connecti...

...lock_connection (void) { - if (thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_CONNECTIONS) - pthread_mutex_lock (&connection_lock); + if (thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_CONNECTIONS && + pthread_mutex_lock (&connection_lock)) + abort (); } void unlock_connection (void) { - if (thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_CONNECTIONS) - pthread_mutex_unlock (&connection_lock); + if (thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_CONNECTIONS && + pthread_mutex_unlock (&connection_lock)) + abort (); } void lock_reque...

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

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.

...an implicit TLS connection. Fixes commit 91023f269d4cea56f573a1aa0d880b12052f6e1e. --- server/locks.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/locks.c b/server/locks.c index 5d54d311..9269c75e 100644 --- a/server/locks.c +++ b/server/locks.c @@ -93,7 +93,7 @@ unlock_connection (void) void lock_request (void) { - GET_CONN; + struct connection *conn = threadlocal_get_conn (); if (thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_ALL_REQUESTS && pthread_mutex_lock (&all_requests_lock)) @@ -110,7 +110,7 @@ lock_request (void) void unlock_requ...

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

...ping things mechanical (with maybe a tweak to the comment) is fine. > } > > static struct backend filter_functions = { > diff --git a/server/locks.c b/server/locks.c > index ef6726d8..d187b422 100644 > --- a/server/locks.c > +++ b/server/locks.c > @@ -91,8 +91,12 @@ unlock_connection (void) > } > > void > -lock_request (struct connection *conn) > +lock_request (void) > { > + struct connection *conn = GET_CONN; > + > + assert (conn != NULL); > + Here's a site where we could exploit the macro guaranteeing a non-null result. -- E...

...ock_connection (void) { - int thread_model = plugin_thread_model (); + int thread_model = backend->thread_model (backend); if (thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_CONNECTIONS) { debug ("acquire connection lock"); @@ -56,7 +56,7 @@ lock_connection (void) void unlock_connection (void) { - int thread_model = plugin_thread_model (); + int thread_model = backend->thread_model (backend); if (thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_CONNECTIONS) { debug ("release connection lock"); @@ -67,7 +67,7 @@ unlock_connection (void) void lock_requ...

...ock_connection (void) { - int thread_model = plugin_thread_model (); + int thread_model = backend->thread_model (backend); if (thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_CONNECTIONS) { debug ("acquire connection lock"); @@ -56,7 +56,7 @@ lock_connection (void) void unlock_connection (void) { - int thread_model = plugin_thread_model (); + int thread_model = backend->thread_model (backend); if (thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_CONNECTIONS) { debug ("release connection lock"); @@ -67,7 +67,7 @@ unlock_connection (void) void lock_requ...

...ock_connection (void) { - int thread_model = plugin_thread_model (); + int thread_model = backend->thread_model (backend); if (thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_CONNECTIONS) { debug ("acquire connection lock"); @@ -56,7 +56,7 @@ lock_connection (void) void unlock_connection (void) { - int thread_model = plugin_thread_model (); + int thread_model = backend->thread_model (backend); if (thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_CONNECTIONS) { debug ("release connection lock"); @@ -67,7 +67,7 @@ unlock_connection (void) void lock_requ...

I do have a question about whether patch 2 is right, or whether I've exposed a bigger problem in the truncate (and possibly other) filter, but the rest seem fairly straightforward. Eric Blake (4): server: Check for pthread lock failures truncate: Factor out reading real_size under mutex plugins: Check for mutex failures filters: Check for mutex failures filters/cache/cache.c

...const char *filename, void *dl, + struct nbdkit_filter *(*filter_init) (void)) + __attribute__((__nonnull__ (1, 3, 4, 5))); /* locks.c */ extern void lock_init_thread_model (void); extern void lock_connection (void); extern void unlock_connection (void); -extern void lock_request (struct connection *conn); -extern void unlock_request (struct connection *conn); +extern void lock_request (struct connection *conn) + __attribute__((__nonnull__ (1))); +extern void unlock_request (struct connection *conn) + __attribute__((__nonnull__ (1)));...

The first three patches are identical to: https://www.redhat.com/archives/libguestfs/2018-January/msg00079.html "[PATCH nbdkit v2 0/3] Refactor plugin_* functions into a backend" The rest of the patches add filters using the new filter API previously described here: https://www.redhat.com/archives/libguestfs/2018-January/msg00073.html This needs a lot more testing -- and tests --

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

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.

A combination of the work that both Rich and I have been doing lately, where filters use only the new API with flags on every command that the client can send over the wire (we can then add support for more flags in nbdkit without having to add new callbacks, as NBD adds more flags upstream). Eric Blake (4): protocol: Split flags from cmd field in requests backend: Pass flags argument through