[PATCH nbdkit] locks: Cache the plugin thread model.
by Richard W.M. Jones
Commit 876d715e17a59bd25df53be34b942afd96e52da9
("Refactor plugin_* functions into a backend struct.") causes hidden
crashes in the test suite (which unfortunately don't cause tests to
fail).
These all come from handle_single_connection where we may lock the
connection, 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>,
sockout=<optimized out>) at connections.c:317
#2 0x00000000004087d3 in start_thread (datav=0x229fd00) at sockets.c:262
#3 0x00007fab44e525e1 in start_thread () from /lib64/libpthread.so.0
#4 0x00007fab44b8717f in clone () from /lib64/libc.so.6
Since the thread model cannot be changed after a plugin is loaded,
just cache it in src/locks.c after the backend is created.
Note that I don't believe commit 876d715e17a59bd25df53be34b942afd96e52da9
caused this breakage. I think it only worked by accident before
because it would still have been possible that
handle_single_connection was reading the plugin._thread_model field
after dlclose.
---
src/internal.h | 1 +
src/locks.c | 19 +++++++++++--------
src/main.c | 1 +
3 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/src/internal.h b/src/internal.h
index 28b1aaf..8047b3b 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -181,6 +181,7 @@ struct backend {
extern struct backend *plugin_register (const char *_filename, void *_dl, struct nbdkit_plugin *(*plugin_init) (void));
/* 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);
diff --git a/src/locks.c b/src/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);
+ thread_model = backend->thread_model (backend);
+}
+void
+lock_connection (void)
+{
if (thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_CONNECTIONS) {
debug ("acquire connection lock");
pthread_mutex_lock (&connection_lock);
@@ -56,8 +65,6 @@ lock_connection (void)
void
unlock_connection (void)
{
- int thread_model = backend->thread_model (backend);
-
if (thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_CONNECTIONS) {
debug ("release connection lock");
pthread_mutex_unlock (&connection_lock);
@@ -67,8 +74,6 @@ unlock_connection (void)
void
lock_request (struct connection *conn)
{
- int thread_model = backend->thread_model (backend);
-
if (thread_model <= NBDKIT_THREAD_MODEL_SERIALIZE_ALL_REQUESTS) {
debug ("acquire global request lock");
pthread_mutex_lock (&all_requests_lock);
@@ -86,8 +91,6 @@ lock_request (struct connection *conn)
void
unlock_request (struct connection *conn)
{
- int thread_model = backend->thread_model (backend);
-
debug ("release unload prevention lock");
pthread_rwlock_unlock (&unload_prevention_lock);
diff --git a/src/main.c b/src/main.c
index b3e6bad..90d464a 100644
--- a/src/main.c
+++ b/src/main.c
@@ -497,6 +497,7 @@ main (int argc, char *argv[])
}
backend = open_plugin_so (filename, short_name);
+ lock_init_thread_model ();
if (help) {
usage ();
--
2.15.1
6 years, 9 months
[nbdkit] Proposed (new) filter API
by Richard W.M. Jones
Here's my second attempt at a filter API.
As before, .config and .config_complete can only call the
next->.config or next->.config_complete methods in the chain
respectively.
The change is with the connected functions (get_size, pread, pwrite,
etc.). Here we pass a struct of next functions allowing the filter to
call any functions on the plugin, so for example a write can turn into
read + write (and vice versa although that might not be a good idea).
Also .open and .close cannot be intercepted by the filter. They're
just used to allow the filter to allocate a handle.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW
6 years, 9 months
[PATCH] configure: Don't define _FORTIFY_SOURCE.
by Richard W.M. Jones
We routinely test the upstream code by running everything under
valgrind, and in any case _FORTIFY_SOURCE is usually defined by
downstream Linux distros and we can leave the optimization vs safety
decision to them.
See this bug: https://bugs.gentoo.org/640494
---
m4/guestfs-c.m4 | 6 ------
1 file changed, 6 deletions(-)
diff --git a/m4/guestfs-c.m4 b/m4/guestfs-c.m4
index 932b6de73..3e8642675 100644
--- a/m4/guestfs-c.m4
+++ b/m4/guestfs-c.m4
@@ -119,12 +119,6 @@ AC_SUBST([NO_UM_CFLAGS])
AC_DEFINE([lint], [1], [Define to 1 if the compiler is checking for lint.])
AC_DEFINE([GNULIB_PORTCHECK], [1], [Enable some gnulib portability checks.])
-AH_VERBATIM([FORTIFY_SOURCE],[
-/* Enable compile-time and run-time bounds-checking, and some warnings. */
-#if __OPTIMIZE__ && (! defined (_FORTIFY_SOURCE) || _FORTIFY_SOURCE < 2)
-# undef _FORTIFY_SOURCE
-# define _FORTIFY_SOURCE 2
-#endif])
AC_C_PROTOTYPES
test "x$U" != "x" && AC_MSG_ERROR([Compiler not ANSI compliant])
--
2.13.2
6 years, 9 months
[nbdkit PATCH 0/7] Initial implementation of FUA flag passthrough
by Eric Blake
Tested via:
term1$ qemu-nbd -k $PWD/sock -t -f raw -x foo junk --trace=nbd_\*
term2$ ./nbdkit -f -v -e bar nbd socket=$PWD/sock export=foo
term3$ qemu-io -t none -f raw nbd://localhost:10809/bar --trace=nbd_\*
and checking the traces to see that 'w 0 1' vs. 'w -f 0 1' was able
to influence whether the FUA flag showed up at the server in term1.
Still to go: figure out how to expose the flags through the language
bindings
Patch 7 is RFC; Rich and I debated on IRC whether we want it (the
code in plugin_register() sounds like it tries hard to support
newer plugins with older nbdkit), or whether we should just drop
the patch and state that a user either manually supplies back-compat
variants (tedious boilerplate for the user to supply, especially
since newer nbdkit will never call the older functions) or just
document that newer plugins combined with old nbdkit is not
guaranteed to work.
Eric Blake (7):
connections: Ignore FUA flag on read/flush
protocol: Split flags from cmd field in requests
plugins: Move FUA fallback to plugins
plugins: Add callbacks for FUA semantics
plugins: Implement FUA support
nbd: Wire up FUA flag passthrough
RFC: plugins: Add back-compat for new plugin with old nbdkit
docs/nbdkit-plugin.pod | 77 +++++++++++++++++++++++++++++++++++++--
docs/nbdkit.pod | 8 +++--
include/nbdkit-plugin.h | 34 +++++++++++++++++-
plugins/nbd/nbd.c | 64 ++++++++++++++++++++-------------
src/connections.c | 46 +++++++++++++-----------
src/internal.h | 9 ++---
src/plugins.c | 96 +++++++++++++++++++++++++++++++++++++++----------
src/protocol.h | 10 +++---
8 files changed, 266 insertions(+), 78 deletions(-)
--
2.14.3
6 years, 9 months
[PATCH v3 0/3] copy GPT attributes
by Cédric Bosdonnat
Hi all,
Here is v3 of the series, taking Richard's comments in account.
Cédric Bosdonnat (3):
daemon: make sgdisk_info_extract_uuid_field more generic
New APIs: part_set_gpt_attributes and part_get_gpt_attributes
resize: copy GPT partition flags
daemon/parted.ml | 45 +++++++++++++++++++++++++++++++++++----------
daemon/parted.mli | 2 ++
generator/actions_core.ml | 37 +++++++++++++++++++++++++++++++++++++
generator/proc_nr.ml | 2 ++
lib/MAX_PROC_NR | 2 +-
resize/resize.ml | 15 ++++++++++++---
6 files changed, 89 insertions(+), 14 deletions(-)
--
2.15.1
6 years, 9 months
[PATCH v2 0/3] copying gpt attributes
by Cédric Bosdonnat
Hi all,
Here is the latest version of the series addressing Pino's comments.
Cédric Bosdonnat (3):
daemon: make sgdisk_info_extract_uuid_field more generic
New APIs: part_set_gpt_attributes and part_get_gpt_attributes
resize: copy GPT partition flags
daemon/parted.ml | 45 +++++++++++++++++++++++++++++++++++----------
daemon/parted.mli | 3 +++
generator/actions_core.ml | 37 +++++++++++++++++++++++++++++++++++++
generator/proc_nr.ml | 2 ++
lib/MAX_PROC_NR | 2 +-
resize/resize.ml | 15 ++++++++++++---
6 files changed, 90 insertions(+), 14 deletions(-)
--
2.15.1
6 years, 9 months
