[Libguestfs] [nbdkit PATCH 1/5] filters: Tighter rules on .get_ready/.after_fork
Eric Blake
eblake at redhat.com
Fri May 7 01:59:36 UTC 2021
Allowing a filter to bypass the plugin's .get_ready or .after_fork
makes no sense, and there are no other arguments to modify.
Furthermore, now that filters have access to open a plugin context
independently of a client connection, .after_fork would be an ideal
place to do so with its existing nbdkit_backend argument (which we
already documented was stable, although this test actually adds tests
for that). Still, we will want to add a counterpart API (in a later
patch) to tear that context down cleanly, where .unload is too late.
But for that to work, we need to ensure that the plugin completes
initialization before the filter is allowed to try to open a context
into the plugin, which means swapping the visitation order to be
inner-to-outer (similar to .prepare).
---
docs/nbdkit-filter.pod | 24 +++++++++++------------
include/nbdkit-filter.h | 7 ++-----
server/filters.c | 26 ++++++-------------------
filters/exitwhen/exitwhen.c | 9 ++++-----
filters/extentlist/extentlist.c | 5 ++---
filters/log/log.c | 9 ++++-----
filters/multi-conn/multi-conn.c | 5 ++---
filters/pause/pause.c | 4 ++--
filters/rate/rate.c | 5 ++---
filters/stats/stats.c | 5 ++---
filters/tls-fallback/tls-fallback.c | 5 ++---
tests/test-layers-filter.c | 12 +++++-------
tests/test-layers.c | 30 ++++++++++++++---------------
13 files changed, 60 insertions(+), 86 deletions(-)
diff --git a/docs/nbdkit-filter.pod b/docs/nbdkit-filter.pod
index 97f32e10..9816f4e8 100644
--- a/docs/nbdkit-filter.pod
+++ b/docs/nbdkit-filter.pod
@@ -127,8 +127,7 @@ which is required.
=head1 NEXT PLUGIN
F<nbdkit-filter.h> defines some function types (C<nbdkit_next_config>,
-C<nbdkit_next_config_complete>, C<nbdkit_next_get_ready>,
-C<nbdkit_next_after_fork>, C<nbdkit_next_preconnect>,
+C<nbdkit_next_config_complete>, C<nbdkit_next_preconnect>,
C<nbdkit_next_list_exports>, C<nbdkit_next_default_export>,
C<nbdkit_next_open>) and a structure called C<struct nbdkit_next_ops>.
These abstract the next plugin or filter in the chain. There is also
@@ -406,28 +405,29 @@ with an error message and return C<-1>.
=head2 C<.get_ready>
- int (*get_ready) (nbdkit_next_get_ready *next, void *nxdata,
- int thread_model);
+ int (*get_ready) (int thread_model);
-This intercepts the plugin C<.get_ready> method and can be used by the
-filter to get ready to serve requests.
+This optional callback is reached if the plugin C<.get_ready> method
+succeeded (if the plugin failed, nbdkit has already exited), and can
+be used by the filter to get ready to serve requests.
The C<thread_model> parameter informs the filter about the final
thread model chosen by nbdkit after considering the results of
C<.thread_model> of all filters in the chain after
-C<.config_complete>. This does not need to be passed on to C<next>,
-as the model can no longer be altered at this point.
+C<.config_complete>.
If there is an error, C<.get_ready> should call C<nbdkit_error> with
an error message and return C<-1>.
=head2 C<.after_fork>
- int (*after_fork) (nbdkit_next_after_fork *next, nbdkit_backend *nxdata);
+ int (*after_fork) (nbdkit_backend *backend);
-This intercepts the plugin C<.after_fork> method and can be used by
-the filter to start background threads (although these have limited
-usefulness since they will not be able to access the plugin).
+This optional callback is reached after the plugin C<.after_fork>
+method has succeeded (if the plugin failed, nbdkit has already
+exited), and can be used by the filter to start background threads.
+The C<backend> parameter is valid until C<.unload>, for creating manual
+contexts into the backend with C<nbdkit_next_context_open>.
If there is an error, C<.after_fork> should call C<nbdkit_error> with
an error message and return C<-1>.
diff --git a/include/nbdkit-filter.h b/include/nbdkit-filter.h
index 86c41dd7..662f52cb 100644
--- a/include/nbdkit-filter.h
+++ b/include/nbdkit-filter.h
@@ -66,8 +66,6 @@ typedef struct nbdkit_next_ops nbdkit_next;
typedef int nbdkit_next_config (nbdkit_backend *nxdata,
const char *key, const char *value);
typedef int nbdkit_next_config_complete (nbdkit_backend *nxdata);
-typedef int nbdkit_next_get_ready (nbdkit_backend *nxdata);
-typedef int nbdkit_next_after_fork (nbdkit_backend *nxdata);
typedef int nbdkit_next_preconnect (nbdkit_backend *nxdata, int readonly);
typedef int nbdkit_next_list_exports (nbdkit_backend *nxdata, int readonly,
struct nbdkit_exports *exports);
@@ -197,9 +195,8 @@ struct nbdkit_filter {
nbdkit_backend *nxdata);
const char *config_help;
int (*thread_model) (void);
- int (*get_ready) (nbdkit_next_get_ready *next, nbdkit_backend *nxdata,
- int thread_model);
- int (*after_fork) (nbdkit_next_after_fork *next, nbdkit_backend *nxdata);
+ int (*get_ready) (int thread_model);
+ int (*after_fork) (nbdkit_backend *backend);
int (*preconnect) (nbdkit_next_preconnect *next, nbdkit_backend *nxdata,
int readonly);
int (*list_exports) (nbdkit_next_list_exports *next, nbdkit_backend *nxdata,
diff --git a/server/filters.c b/server/filters.c
index 31cf5c7e..136fa672 100644
--- a/server/filters.c
+++ b/server/filters.c
@@ -171,33 +171,19 @@ filter_config_complete (struct backend *b)
b->next->config_complete (b->next);
}
-static int
-next_get_ready (struct backend *b)
-{
- b->get_ready (b);
- return 0;
-}
-
static void
filter_get_ready (struct backend *b)
{
struct backend_filter *f = container_of (b, struct backend_filter, backend);
+ b->next->get_ready (b->next); /* exits on failure */
+
debug ("%s: get_ready thread_model=%d", b->name, thread_model);
if (f->filter.get_ready) {
- if (f->filter.get_ready (next_get_ready, b->next, thread_model) == -1)
+ if (f->filter.get_ready (thread_model) == -1)
exit (EXIT_FAILURE);
}
- else
- b->next->get_ready (b->next);
-}
-
-static int
-next_after_fork (struct backend *b)
-{
- b->after_fork (b);
- return 0;
}
static void
@@ -205,14 +191,14 @@ filter_after_fork (struct backend *b)
{
struct backend_filter *f = container_of (b, struct backend_filter, backend);
+ b->next->after_fork (b->next); /* exits on failure */
+
debug ("%s: after_fork", b->name);
if (f->filter.after_fork) {
- if (f->filter.after_fork (next_after_fork, b->next) == -1)
+ if (f->filter.after_fork (b->next) == -1)
exit (EXIT_FAILURE);
}
- else
- b->next->after_fork (b->next);
}
static int
diff --git a/filters/exitwhen/exitwhen.c b/filters/exitwhen/exitwhen.c
index 80836357..543af058 100644
--- a/filters/exitwhen/exitwhen.c
+++ b/filters/exitwhen/exitwhen.c
@@ -436,22 +436,21 @@ exitwhen_config (nbdkit_next_config *next, nbdkit_backend *nxdata,
* then we exit immediately.
*/
static int
-exitwhen_get_ready (nbdkit_next_get_ready *next, nbdkit_backend *nxdata,
- int thread_model)
+exitwhen_get_ready (int thread_model)
{
ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock);
if (check_for_event ())
exit (EXIT_SUCCESS);
- return next (nxdata);
+ return 0;
}
/* After forking, start the background thread. Initially it is
* polling.
*/
static int
-exitwhen_after_fork (nbdkit_next_after_fork *next, nbdkit_backend *nxdata)
+exitwhen_after_fork (nbdkit_backend *nxdata)
{
int err;
pthread_t thread;
@@ -462,7 +461,7 @@ exitwhen_after_fork (nbdkit_next_after_fork *next, nbdkit_backend *nxdata)
nbdkit_error ("pthread_create: %m");
return -1;
}
- return next (nxdata);
+ return 0;
}
static int
diff --git a/filters/extentlist/extentlist.c b/filters/extentlist/extentlist.c
index c609062c..7b7ba2f2 100644
--- a/filters/extentlist/extentlist.c
+++ b/filters/extentlist/extentlist.c
@@ -262,12 +262,11 @@ parse_extentlist (void)
}
static int
-extentlist_get_ready (nbdkit_next_get_ready *next, nbdkit_backend *nxdata,
- int thread_model)
+extentlist_get_ready (int thread_model)
{
parse_extentlist ();
- return next (nxdata);
+ return 0;
}
static int
diff --git a/filters/log/log.c b/filters/log/log.c
index 26358302..acc9d77e 100644
--- a/filters/log/log.c
+++ b/filters/log/log.c
@@ -103,8 +103,7 @@ log_config (nbdkit_next_config *next, nbdkit_backend *nxdata,
/* Open the logfile. */
static int
-log_get_ready (nbdkit_next_get_ready *next, nbdkit_backend *nxdata,
- int thread_model)
+log_get_ready (int thread_model)
{
int fd;
@@ -133,17 +132,17 @@ log_get_ready (nbdkit_next_get_ready *next, nbdkit_backend *nxdata,
saved_pid = getpid ();
print (NULL, "Ready", "thread_model=%d", thread_model);
- return next (nxdata);
+ return 0;
}
static int
-log_after_fork (nbdkit_next_after_fork *next, nbdkit_backend *nxdata)
+log_after_fork (nbdkit_backend *nxdata)
{
/* Only issue this message if we actually fork. */
if (getpid () != saved_pid)
print (NULL, "Fork", "");
- return next (nxdata);
+ return 0;
}
/* List exports. */
diff --git a/filters/multi-conn/multi-conn.c b/filters/multi-conn/multi-conn.c
index 1b1c9d8c..12cc9a8f 100644
--- a/filters/multi-conn/multi-conn.c
+++ b/filters/multi-conn/multi-conn.c
@@ -145,13 +145,12 @@ multi_conn_config (nbdkit_next_config *next, nbdkit_backend *nxdata,
"multi-conn-exportname=<BOOL> true to limit emulation by export name.\n"
static int
-multi_conn_get_ready (nbdkit_next_get_ready *next, nbdkit_backend *nxdata,
- int thread_model)
+multi_conn_get_ready (int thread_model)
{
if (thread_model == NBDKIT_THREAD_MODEL_SERIALIZE_CONNECTIONS &&
mode == AUTO)
mode = DISABLE;
- return next (nxdata);
+ return 0;
}
static void *
diff --git a/filters/pause/pause.c b/filters/pause/pause.c
index 90511b93..e3745397 100644
--- a/filters/pause/pause.c
+++ b/filters/pause/pause.c
@@ -231,7 +231,7 @@ control_socket_thread (void *arg)
/* Start the background thread after fork. */
static int
-pause_after_fork (nbdkit_next_after_fork *next, nbdkit_backend *nxdata)
+pause_after_fork (nbdkit_backend *nxdata)
{
int err;
pthread_t thread;
@@ -242,7 +242,7 @@ pause_after_fork (nbdkit_next_after_fork *next, nbdkit_backend *nxdata)
nbdkit_error ("pthread_create: %m");
return -1;
}
- return next (nxdata);
+ return 0;
}
/* This is called before processing each NBD request. */
diff --git a/filters/rate/rate.c b/filters/rate/rate.c
index 7b07b5bf..6838628f 100644
--- a/filters/rate/rate.c
+++ b/filters/rate/rate.c
@@ -147,14 +147,13 @@ rate_config (nbdkit_next_config *next, nbdkit_backend *nxdata,
}
static int
-rate_get_ready (nbdkit_next_get_ready *next, nbdkit_backend *nxdata,
- int thread_model)
+rate_get_ready (int thread_model)
{
/* Initialize the global buckets. */
bucket_init (&read_bucket, rate, BUCKET_CAPACITY);
bucket_init (&write_bucket, rate, BUCKET_CAPACITY);
- return next (nxdata);
+ return 0;
}
#define rate_config_help \
diff --git a/filters/stats/stats.c b/filters/stats/stats.c
index 7fd043a3..5f44bc37 100644
--- a/filters/stats/stats.c
+++ b/filters/stats/stats.c
@@ -212,8 +212,7 @@ stats_config_complete (nbdkit_next_config_complete *next,
}
static int
-stats_get_ready (nbdkit_next_get_ready *next, nbdkit_backend *nxdata,
- int thread_model)
+stats_get_ready (int thread_model)
{
int fd;
@@ -238,7 +237,7 @@ stats_get_ready (nbdkit_next_get_ready *next, nbdkit_backend *nxdata,
gettimeofday (&start_t, NULL);
- return next (nxdata);
+ return 0;
}
#define stats_config_help \
diff --git a/filters/tls-fallback/tls-fallback.c b/filters/tls-fallback/tls-fallback.c
index ee3e6784..fab9e58b 100644
--- a/filters/tls-fallback/tls-fallback.c
+++ b/filters/tls-fallback/tls-fallback.c
@@ -63,15 +63,14 @@ tls_fallback_config (nbdkit_next_config *next, nbdkit_backend *nxdata,
"tlsreadme=<MESSAGE> Alternative contents for the plaintext dummy export.\n"
int
-tls_fallback_get_ready (nbdkit_next_get_ready *next, nbdkit_backend *nxdata,
- int thread_model)
+tls_fallback_get_ready (int thread_model)
{
if (thread_model == NBDKIT_THREAD_MODEL_SERIALIZE_CONNECTIONS) {
nbdkit_error ("the tls-fallback filter requires parallel connection "
"support");
return -1;
}
- return next (nxdata);
+ return 0;
}
static int
diff --git a/tests/test-layers-filter.c b/tests/test-layers-filter.c
index 2d8fe879..25ecb585 100644
--- a/tests/test-layers-filter.c
+++ b/tests/test-layers-filter.c
@@ -89,20 +89,18 @@ test_layers_filter_thread_model (void)
}
static int
-test_layers_filter_get_ready (nbdkit_next_get_ready *next,
- nbdkit_backend *nxdata, int thread_model)
+test_layers_filter_get_ready (int thread_model)
{
DEBUG_FUNCTION;
- return next (nxdata);
+ return 0;
}
static int
-test_layers_filter_after_fork (nbdkit_next_after_fork *next,
- nbdkit_backend *nxdata)
+test_layers_filter_after_fork (nbdkit_backend *backend)
{
DEBUG_FUNCTION;
- saved_backend = nxdata;
- return next (nxdata);
+ saved_backend = backend;
+ return 0;
}
static int
diff --git a/tests/test-layers.c b/tests/test-layers.c
index cd5d73bc..09edcc0a 100644
--- a/tests/test-layers.c
+++ b/tests/test-layers.c
@@ -1,5 +1,5 @@
/* nbdkit
- * Copyright (C) 2018-2020 Red Hat Inc.
+ * Copyright (C) 2018-2021 Red Hat Inc.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are
@@ -234,28 +234,28 @@ main (int argc, char *argv[])
"filter3: test_layers_filter_thread_model",
NULL);
- /* get_ready methods called in order. */
+ /* get_ready methods called in inner-to-outer order. */
log_verify_seen_in_order
- ("testlayersfilter3: get_ready",
- "filter3: test_layers_filter_get_ready",
- "testlayersfilter2: get_ready",
- "filter2: test_layers_filter_get_ready",
+ ("testlayersplugin: get_ready",
+ "test_layers_plugin_get_ready",
"testlayersfilter1: get_ready",
"filter1: test_layers_filter_get_ready",
- "testlayersplugin: get_ready",
- "test_layers_plugin_get_ready",
+ "testlayersfilter2: get_ready",
+ "filter2: test_layers_filter_get_ready",
+ "testlayersfilter3: get_ready",
+ "filter3: test_layers_filter_get_ready",
NULL);
- /* after_fork methods called in order. */
+ /* after_fork methods called in inner-to-outer order. */
log_verify_seen_in_order
- ("testlayersfilter3: after_fork",
- "filter3: test_layers_filter_after_fork",
- "testlayersfilter2: after_fork",
- "filter2: test_layers_filter_after_fork",
+ ("testlayersplugin: after_fork",
+ "test_layers_plugin_after_fork",
"testlayersfilter1: after_fork",
"filter1: test_layers_filter_after_fork",
- "testlayersplugin: after_fork",
- "test_layers_plugin_after_fork",
+ "testlayersfilter2: after_fork",
+ "filter2: test_layers_filter_after_fork",
+ "testlayersfilter3: after_fork",
+ "filter3: test_layers_filter_after_fork",
NULL);
/* preconnect methods called in outer-to-inner order, complete
--
2.31.1
More information about the Libguestfs
mailing list