On Sat, Feb 22, 2020 at 05:11:01AM -0600, Eric Blake
wrote:> On 2/22/20 4:37 AM, Richard W.M. Jones wrote:
> >Another thing I've been thinking about for some time is splitting
> >.config_complete into .config_complete + .get_ready (new name TBD).
> >At the moment .config_complete is both the place where we finish
> >processing config, and also the last chance we get to set things up
> >before the server forks. Of course such a change would not be
> >material — .get_ready would always be run immediately after
> >.config_complete — but it cleans up the API a little.
>
> How would it play with --dump-config? Right now, we allow that to
> run after .config but before .config_complete, precisely so that
> language bindings can accept the first .config to determine their
> script name, and then consult the script for any further config
> help.
I don't believe it affects it at all. Here are a couple of
patches which show the general idea and pass all the tests.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine. Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/
--Khiu2s0Zk5P3S+XT
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
filename="0001-server-Check-parameters-in-main.patch"
>From aaee5182e0578f0bab0a0b09bce1a2410eda93db Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Sat, 22 Feb 2020 10:54:56 +0000
Subject: [PATCH 1/2] server: Check parameters in main ().
Unclear why we were checking these parameters in start_serving, except
because that's historically how it was always done.
Also move some serving-related calls into start_serving.
While this slightly changes the order of operations in some corner
cases, it's essentially neutral code refactoring.
---
server/main.c | 46 +++++++++++++++++++++++-----------------------
1 file changed, 23 insertions(+), 23 deletions(-)
diff --git a/server/main.c b/server/main.c
index 8aa8b497..3bc59781 100644
--- a/server/main.c
+++ b/server/main.c
@@ -671,10 +671,25 @@ main (int argc, char *argv[])
/* Select the correct thread model based on config. */
lock_init_thread_model ();
- set_up_quit_pipe ();
-#if !ENABLE_LIBFUZZER
- set_up_signals ();
-#endif
+ /* If the user has mixed up -p/--run/-s/-U/--vsock options, then
+ * give an error.
+ *
+ * XXX Actually the server could easily be extended to handle both
+ * TCP/IP and Unix sockets, or even multiple TCP/IP ports.
+ */
+ if ((port && unixsocket) ||
+ (port && listen_stdin) ||
+ (unixsocket && listen_stdin) ||
+ (listen_stdin && run) ||
+ (vsock && unixsocket) ||
+ (vsock && listen_stdin) ||
+ (vsock && run)) {
+ fprintf (stderr,
+ "%s: -p, --run, -s, -U or --vsock options cannot be
used"
+ "in this combination\n",
+ program_name);
+ exit (EXIT_FAILURE);
+ }
start_serving ();
@@ -847,25 +862,10 @@ start_serving (void)
size_t nr_socks;
size_t i;
- /* If the user has mixed up -p/--run/-s/-U/--vsock options, then
- * give an error.
- *
- * XXX Actually the server could easily be extended to handle both
- * TCP/IP and Unix sockets, or even multiple TCP/IP ports.
- */
- if ((port && unixsocket) ||
- (port && listen_stdin) ||
- (unixsocket && listen_stdin) ||
- (listen_stdin && run) ||
- (vsock && unixsocket) ||
- (vsock && listen_stdin) ||
- (vsock && run)) {
- fprintf (stderr,
- "%s: -p, --run, -s, -U or --vsock options cannot be
used"
- "in this combination\n",
- program_name);
- exit (EXIT_FAILURE);
- }
+ set_up_quit_pipe ();
+#if !ENABLE_LIBFUZZER
+ set_up_signals ();
+#endif
/* Lock the process into memory if requested. */
if (swap) {
--
2.25.0
--Khiu2s0Zk5P3S+XT
Content-Type: text/plain; charset=utf-8
Content-Disposition: attachment;
filename="0002-server-Add-.get_ready-callback.patch"
Content-Transfer-Encoding: 8bit
>From c2a7423fec01dea9e4fb7318f9498e0984345fd1 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Sat, 22 Feb 2020 11:01:22 +0000
Subject: [PATCH 2/2] server: Add .get_ready callback.
---
docs/nbdkit-filter.pod | 38 +++++++++++++++++++++++++-------------
docs/nbdkit-plugin.pod | 20 ++++++++++++++++----
include/nbdkit-filter.h | 2 ++
include/nbdkit-plugin.h | 2 ++
server/internal.h | 1 +
server/filters.c | 23 +++++++++++++++++++++++
server/main.c | 5 +++++
server/plugins.c | 16 ++++++++++++++++
tests/test-layers-filter.c | 9 +++++++++
tests/test-layers-plugin.c | 8 ++++++++
tests/test-layers.c | 12 ++++++++++++
11 files changed, 119 insertions(+), 17 deletions(-)
diff --git a/docs/nbdkit-filter.pod b/docs/nbdkit-filter.pod
index 4105b8b7..a9dffb56 100644
--- a/docs/nbdkit-filter.pod
+++ b/docs/nbdkit-filter.pod
@@ -127,22 +127,24 @@ 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_preconnect>,
-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
-an opaque pointer C<nxdata> which must be passed along when calling
-these functions. The value of C<nxdata> passed to C<.open> has a
-stable lifetime that lasts to the corresponding C<.close>, with all
-intermediate functions (such as C<.pread>) receiving the same value
-for convenience; the only exceptions where C<nxdata> is not reused are
-C<.config>, C<.config_complete>, and C<.preconnect>, which
are called
-outside the lifetime of a connection.
+C<nbdkit_next_config_complete>, C<nbdkit_next_get_ready>,
+C<nbdkit_next_preconnect>, 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 an opaque pointer C<nxdata> which must be
+passed along when calling these functions. The value of C<nxdata>
+passed to C<.open> has a stable lifetime that lasts to the
+corresponding C<.close>, with all intermediate functions (such as
+C<.pread>) receiving the same value for convenience; the only
+exceptions where C<nxdata> is not reused are C<.config>,
+C<.config_complete>, C<.get_ready>, and C<.preconnect>, which
are
+called outside the lifetime of a connection.
=head2 Next config, open and close
-The filter’s C<.config>, C<.config_complete> and C<.open>
methods may
-only call the next C<.config>, C<.config_complete> and
C<.open> method
-in the chain (optionally for C<.config>).
+The filter’s C<.config>, C<.config_complete>, C<.get_ready>
and
+C<.open> methods may only call the next C<.config>,
+C<.config_complete>, C<.get_ready> and C<.open> method in the
chain
+(optionally for C<.config>).
The filter’s C<.close> method is called when an old connection closed,
and this has no C<next> parameter because it cannot be
@@ -292,6 +294,16 @@ filter doesn't slow down other filters or plugins.
If there is an error, C<.thread_model> should call C<nbdkit_error>
with an error message and return C<-1>.
+=head2 C<.get_ready>
+
+ int (*get_ready) (nbdkit_next_get_ready *next, void *nxdata);
+
+This intercepts the plugin C<.get_ready> method and can be used by the
+filter to get ready to serve requests.
+
+If there is an error, C<.get_ready> should call C<nbdkit_error>
with
+an error message and return C<-1>.
+
=head2 C<.preconnect>
int (*preconnect) (nbdkit_next_preconnect *next, void *nxdata,
diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod
index 41bffb7f..77f1a098 100644
--- a/docs/nbdkit-plugin.pod
+++ b/docs/nbdkit-plugin.pod
@@ -443,6 +443,18 @@ are silently ignored.
If there is an error, C<.thread_model> should call C<nbdkit_error>
with an error message and return C<-1>.
+=head2 C<.get_ready>
+
+ int get_ready (void);
+
+This optional callback is called before the server starts serving. It
+is called before the server forks or changes directory. It is the
+last chance to do any global preparation that is needed to serve
+connections.
+
+If there is an error, C<.get_ready> should call C<nbdkit_error>
with
+an error message and return C<-1>.
+
=head2 C<.preconnect>
int preconnect (int readonly);
@@ -1182,8 +1194,8 @@ absolute path: if it is relative, then all this function
does is
prepend the current working directory to the path, with no extra
checks.
-Note that this function works I<only> when used in the C<.config>,
and
-C<.config_complete> callbacks.
+Note that this function works I<only> when used in the C<.config>,
+C<.config_complete> and C<.get_ready> callbacks.
If conversion was not possible, this calls C<nbdkit_error> and returns
C<NULL>. Note that this function does not check that the file exists.
@@ -1199,8 +1211,8 @@ absolute path, resolving symlinks. Under the hood it uses
the
C<realpath> function, and thus it fails if the path does not exist,
or it is not possible to access to any of the components of the path.
-Note that this function works I<only> when used in the C<.config>,
and
-C<.config_complete> callbacks.
+Note that this function works I<only> when used in the C<.config>,
+C<.config_complete> and C<.get_ready> callbacks.
If the path resolution was not possible, this calls C<nbdkit_error>
and returns C<NULL>.
diff --git a/include/nbdkit-filter.h b/include/nbdkit-filter.h
index a44c689b..ca58e496 100644
--- a/include/nbdkit-filter.h
+++ b/include/nbdkit-filter.h
@@ -62,6 +62,7 @@ typedef void nbdkit_backend;
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_preconnect (nbdkit_backend *nxdata, int readonly);
typedef int nbdkit_next_open (nbdkit_backend *nxdata, int readonly);
@@ -143,6 +144,7 @@ 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 (*preconnect) (nbdkit_next_preconnect *next, nbdkit_backend *nxdata,
int readonly);
diff --git a/include/nbdkit-plugin.h b/include/nbdkit-plugin.h
index b4ecf658..7e06a4b1 100644
--- a/include/nbdkit-plugin.h
+++ b/include/nbdkit-plugin.h
@@ -136,6 +136,8 @@ struct nbdkit_plugin {
int (*can_fast_zero) (void *handle);
int (*preconnect) (int readonly);
+
+ int (*get_ready) (void);
};
extern void nbdkit_set_error (int err);
diff --git a/server/internal.h b/server/internal.h
index eaec31ba..d8a589f2 100644
--- a/server/internal.h
+++ b/server/internal.h
@@ -344,6 +344,7 @@ struct backend {
void (*config) (struct backend *, const char *key, const char *value);
void (*config_complete) (struct backend *);
const char *(*magic_config_key) (struct backend *);
+ void (*get_ready) (struct backend *);
int (*preconnect) (struct backend *, int readonly);
void *(*open) (struct backend *, int readonly);
int (*prepare) (struct backend *, void *handle, int readonly);
diff --git a/server/filters.c b/server/filters.c
index 8985ebeb..f0371066 100644
--- a/server/filters.c
+++ b/server/filters.c
@@ -171,6 +171,28 @@ 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);
+
+ debug ("%s: get_ready", b->name);
+
+ if (f->filter.get_ready) {
+ if (f->filter.get_ready (next_get_ready, b->next) == -1)
+ exit (EXIT_FAILURE);
+ }
+ else
+ b->next->get_ready (b->next);
+}
+
static int
filter_preconnect (struct backend *b, int readonly)
{
@@ -493,6 +515,7 @@ static struct backend filter_functions = {
.config = filter_config,
.config_complete = filter_config_complete,
.magic_config_key = plugin_magic_config_key,
+ .get_ready = filter_get_ready,
.preconnect = filter_preconnect,
.open = filter_open,
.prepare = filter_prepare,
diff --git a/server/main.c b/server/main.c
index 3bc59781..b3d8e5bf 100644
--- a/server/main.c
+++ b/server/main.c
@@ -691,6 +691,11 @@ main (int argc, char *argv[])
exit (EXIT_FAILURE);
}
+ /* Tell the plugin that we are about to start serving. This must be
+ * called before we change user, fork, or open any sockets.
+ */
+ top->get_ready (top);
+
start_serving ();
top->free (top);
diff --git a/server/plugins.c b/server/plugins.c
index 16b4099b..78ed6723 100644
--- a/server/plugins.c
+++ b/server/plugins.c
@@ -158,6 +158,7 @@ plugin_dump_fields (struct backend *b)
HAS (config);
HAS (config_complete);
HAS (config_help);
+ HAS (get_ready);
HAS (preconnect);
HAS (open);
HAS (close);
@@ -234,6 +235,20 @@ plugin_magic_config_key (struct backend *b)
return p->plugin.magic_config_key;
}
+static void
+plugin_get_ready (struct backend *b)
+{
+ struct backend_plugin *p = container_of (b, struct backend_plugin, backend);
+
+ debug ("%s: get_ready", b->name);
+
+ if (!p->plugin.get_ready)
+ return;
+
+ if (p->plugin.get_ready () == -1)
+ exit (EXIT_FAILURE);
+}
+
static int
plugin_preconnect (struct backend *b, int readonly)
{
@@ -670,6 +685,7 @@ static struct backend plugin_functions = {
.config = plugin_config,
.config_complete = plugin_config_complete,
.magic_config_key = plugin_magic_config_key,
+ .get_ready = plugin_get_ready,
.preconnect = plugin_preconnect,
.open = plugin_open,
.prepare = plugin_prepare,
diff --git a/tests/test-layers-filter.c b/tests/test-layers-filter.c
index 44f62c6e..53427d2a 100644
--- a/tests/test-layers-filter.c
+++ b/tests/test-layers-filter.c
@@ -81,6 +81,14 @@ test_layers_filter_config_complete
(nbdkit_next_config_complete *next,
#define test_layers_filter_config_help \
"test_layers_" layer "_config_help"
+static int
+test_layers_filter_get_ready (nbdkit_next_get_ready *next,
+ void *nxdata)
+{
+ DEBUG_FUNCTION;
+ return next (nxdata);
+}
+
static int
test_layers_filter_preconnect (nbdkit_next_preconnect *next,
void *nxdata, int readonly)
@@ -349,6 +357,7 @@ static struct nbdkit_filter filter = {
.config = test_layers_filter_config,
.config_complete = test_layers_filter_config_complete,
.config_help = test_layers_filter_config_help,
+ .get_ready = test_layers_filter_get_ready,
.preconnect = test_layers_filter_preconnect,
.open = test_layers_filter_open,
.close = test_layers_filter_close,
diff --git a/tests/test-layers-plugin.c b/tests/test-layers-plugin.c
index 10cc6efe..8858bede 100644
--- a/tests/test-layers-plugin.c
+++ b/tests/test-layers-plugin.c
@@ -72,6 +72,13 @@ test_layers_plugin_config_complete (void)
#define test_layers_plugin_config_help
"test_layers_plugin_config_help"
+static int
+test_layers_plugin_get_ready (void)
+{
+ DEBUG_FUNCTION;
+ return 0;
+}
+
static int
test_layers_plugin_preconnect (int readonly)
{
@@ -231,6 +238,7 @@ static struct nbdkit_plugin plugin = {
.config = test_layers_plugin_config,
.config_complete = test_layers_plugin_config_complete,
.config_help = test_layers_plugin_config_help,
+ .get_ready = test_layers_plugin_get_ready,
.preconnect = test_layers_plugin_preconnect,
.open = test_layers_plugin_open,
.close = test_layers_plugin_close,
diff --git a/tests/test-layers.c b/tests/test-layers.c
index fafe68c4..33ae5a75 100644
--- a/tests/test-layers.c
+++ b/tests/test-layers.c
@@ -288,6 +288,18 @@ main (int argc, char *argv[])
"test_layers_plugin_config_complete",
NULL);
+ /* get_ready methods called in order. */
+ log_verify_seen_in_order
+ ("testlayersfilter3: get_ready",
+ "filter3: test_layers_filter_get_ready",
+ "testlayersfilter2: get_ready",
+ "filter2: test_layers_filter_get_ready",
+ "testlayersfilter1: get_ready",
+ "filter1: test_layers_filter_get_ready",
+ "testlayersplugin: get_ready",
+ "test_layers_plugin_get_ready",
+ NULL);
+
/* preconnect methods called in outer-to-inner order, complete
* in inner-to-outer order.
*/
--
2.25.0
--Khiu2s0Zk5P3S+XT--