search for: unload_prevention_lock

...- if (thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_REQUESTS) - pthread_mutex_lock (&conn->request_lock); + if (thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_REQUESTS && + pthread_mutex_lock (&conn->request_lock)) + abort (); - pthread_rwlock_rdlock (&unload_prevention_lock); + if (pthread_rwlock_rdlock (&unload_prevention_lock)) + abort (); } void unlock_request (struct connection *conn) { - pthread_rwlock_unlock (&unload_prevention_lock); + if (pthread_rwlock_unlock (&unload_prevention_lock)) + abort (); - if (thread_model <= NBDKIT_TH...

...DAMAGE. + */ + +#include <config.h> + +#include <stdio.h> +#include <stdlib.h> + +#include "internal.h" + +static pthread_mutex_t connection_lock = PTHREAD_MUTEX_INITIALIZER; +static pthread_mutex_t all_requests_lock = PTHREAD_MUTEX_INITIALIZER; +static pthread_rwlock_t unload_prevention_lock = PTHREAD_RWLOCK_INITIALIZER; + +void +lock_connection (void) +{ + int thread_model = plugin_thread_model (); + + if (thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_CONNECTIONS) { + debug ("acquire connection lock"); + pthread_mutex_lock (&connection_lock); + } +} + +void +...

...DAMAGE. + */ + +#include <config.h> + +#include <stdio.h> +#include <stdlib.h> + +#include "internal.h" + +static pthread_mutex_t connection_lock = PTHREAD_MUTEX_INITIALIZER; +static pthread_mutex_t all_requests_lock = PTHREAD_MUTEX_INITIALIZER; +static pthread_rwlock_t unload_prevention_lock = PTHREAD_RWLOCK_INITIALIZER; + +void +lock_connection (void) +{ + int thread_model = plugin_thread_model (); + + if (thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_CONNECTIONS) { + debug ("acquire connection lock"); + pthread_mutex_lock (&connection_lock); + } +} + +void +...

...} diff --git a/src/plugins.c b/src/plugins.c index d2d0b39..f5056d9 100644 --- a/src/plugins.c +++ b/src/plugins.c @@ -48,6 +48,7 @@ static pthread_mutex_t connection_lock = PTHREAD_MUTEX_INITIALIZER; static pthread_mutex_t all_requests_lock = PTHREAD_MUTEX_INITIALIZER; +static pthread_rwlock_t unload_prevention_lock = PTHREAD_RWLOCK_INITIALIZER; /* Maximum read or write request that we will handle. */ #define MAX_REQUEST_SIZE (64 * 1024 * 1024) @@ -155,6 +156,11 @@ void plugin_cleanup (void) { if (dl) { + /* Acquiring this lock prevents any plugin callbacks from running + * simultaneously. +...

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

This fixes the race conditions for me, using the test described here: https://www.redhat.com/archives/libguestfs/2017-September/msg00226.html Rich.

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.

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

...LIZE_REQUESTS) { - debug ("acquire per-connection request lock"); + if (thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_REQUESTS) pthread_mutex_lock (connection_get_request_lock (conn)); - } - debug ("acquire unload prevention lock"); pthread_rwlock_rdlock (&unload_prevention_lock); } void unlock_request (struct connection *conn) { - debug ("release unload prevention lock"); pthread_rwlock_unlock (&unload_prevention_lock); - if (thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_REQUESTS) { - debug ("release per-connection request lock")...

...that the plugin's thread model cannot change after being + * loaded, so caching it here is safe. + */ +static int thread_model; + static pthread_mutex_t connection_lock = PTHREAD_MUTEX_INITIALIZER; static pthread_mutex_t all_requests_lock = PTHREAD_MUTEX_INITIALIZER; static pthread_rwlock_t unload_prevention_lock = PTHREAD_RWLOCK_INITIALIZER; void -lock_connection (void) +lock_init_thread_model (void) { - int thread_model = backend->thread_model (backend); + thread_model = backend->thread_model (backend); +} +void +lock_connection (void) +{ if (thread_model <= NBDKIT_THREAD_MODEL_SERIALI...

...f (thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_ALL_REQUESTS && pthread_mutex_lock (&all_requests_lock)) @@ -110,7 +110,7 @@ lock_request (void) void unlock_request () { - GET_CONN; + struct connection *conn = threadlocal_get_conn (); if (pthread_rwlock_unlock (&unload_prevention_lock)) abort (); -- 2.25.0

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

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

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

...*plugin_register (const char *_filename, void *_dl, struct nbdkit_plugin *(*plugin_init) (void)); /* locks.c */ extern void lock_connection (void); diff --git a/src/locks.c b/src/locks.c index 6021356..62b2dd0 100644 --- a/src/locks.c +++ b/src/locks.c @@ -45,7 +45,7 @@ static pthread_rwlock_t unload_prevention_lock = PTHREAD_RWLOCK_INITIALIZER; void lock_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...

...*plugin_register (const char *_filename, void *_dl, struct nbdkit_plugin *(*plugin_init) (void)); /* locks.c */ extern void lock_connection (void); diff --git a/src/locks.c b/src/locks.c index 6021356..62b2dd0 100644 --- a/src/locks.c +++ b/src/locks.c @@ -45,7 +45,7 @@ static pthread_rwlock_t unload_prevention_lock = PTHREAD_RWLOCK_INITIALIZER; void lock_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...

...*plugin_register (const char *_filename, void *_dl, struct nbdkit_plugin *(*plugin_init) (void)); /* locks.c */ extern void lock_connection (void); diff --git a/src/locks.c b/src/locks.c index 6021356..62b2dd0 100644 --- a/src/locks.c +++ b/src/locks.c @@ -45,7 +45,7 @@ static pthread_rwlock_t unload_prevention_lock = PTHREAD_RWLOCK_INITIALIZER; void lock_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...

This patch isn't complete (patch 6/6 isn't finished) so it's just for discussion, although it does compile and run. This introduces to nbdkit a concept of "filters" which can be placed in front of plugins to modify their behaviour. Some examples where you might use filters: * Serve a subset of the data, such as (offset, range) or a single partition from a disk image.

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.