Re: [Libguestfs] [nbdkit PATCH 1/5] filters: Tighter rules on .get_ready/.after_fork

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); }