search for: connection_lock

...diff --git a/server/locks.c b/server/locks.c index f4d6497..d70baf2 100644 --- a/server/locks.c +++ b/server/locks.c @@ -55,49 +55,59 @@ lock_init_thread_model (void) void 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_lo...

...RT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT + * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH 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_THREA...

...RT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT + * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH 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_THREA...

...+ * callback should always be called. + */ + if (!quit) { + if (conn->handle) + plugin_close (conn); + } free (conn); } 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,1...

...c/locks.c index 62b2dd0..bd8fd99 100644 --- a/src/locks.c +++ b/src/locks.c @@ -38,15 +38,24 @@ #include "internal.h" +/* Note 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...

...RE); > } > > -/* Handle the thread model. */ > -void > -plugin_lock_connection (void) > -{ > - if (plugin._thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_CONNECTIONS) { > - debug ("%s: acquire connection lock", filename); > - pthread_mutex_lock (&connection_lock); > - } > -} but the old code did not (and just blindly assumed that _thread_model is always valid). Can that new assertion ever fire during racy unload sequences? Otherwise, looks right to me. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualiza...

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

...-56,53 +56,39 @@ lock_init_thread_model (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); - } } v...

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

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

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.

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

The upstream protocol recently promoted NBD_CMD_WRITE_ZEROES from experimental to a documented extension. Exposing support for this allows plugin writers to create sparse files when driven by a client that knows how to use the extension; meanwhile, even if a plugin does not support this extension, the server benefits from less network traffic from the client. Eric Blake (5): protocol: Support

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

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

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.