search for: next_

On Wed, Feb 12, 2020 at 09:38:29AM -0600, Eric Blake wrote: > +#ifdef NBDKIT_INTERNAL > +/* Opaque type encapsulating all information needed for calling into > + * the next filter or plugin. > + */ > +typedef struct backend backend; > +#else > +typedef void backend; > +#endif Is it better to call it something with an nbdkit_* prefix? Although not advisable, there can be

...t; * a stable ‘void *nxdata’ in the filter API. > */ > -struct b_conn { > +struct b_h { > struct backend *b; > - struct connection *conn; > void *handle; > }; > > @@ -186,22 +186,22 @@ filter_config_complete (struct backend *b) > static int > next_preconnect (void *nxdata, int readonly) > { > - struct b_conn *b_conn = nxdata; > - return b_conn->b->preconnect (b_conn->b, b_conn->conn, readonly); > + struct b_h *b_h = nxdata; > + return b_h->b->preconnect (b_h->b, readonly); > } None of the next_*...

With our recent cleanups to nxdata, the only remaining difference between functions like backend_open() and next_open() was the signature (one used void*, the other struct backend *); the API is compatible. All of our filters are in-tree, and we don't promise API/ABI stability, but it is still a lot of files to touch, so the simplest solution to avoid the redundant hop through wrapper functions is to chan...

...+- > src/Makefile.am | 1 + > src/filters.c | 606 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > src/internal.h | 23 ++- > src/main.c | 114 +++++++++-- > src/plugins.c | 11 +- > 7 files changed, 772 insertions(+), 21 deletions(-) > > +/* The next_functions structure contains pointers to backend > + * functions. However because these functions are all expecting a > + * backend and a connection, we cannot call them directly, but must > + * write some next_* functions that unpack the two parameters from a > + * single ‘void *nxdata...

...m/archives/libguestfs/2020-February/msg00092.html > --- > server/filters.c | 217 ++++++++++++++++------------------------------- > 1 file changed, 73 insertions(+), 144 deletions(-) > > @@ -216,201 +205,181 @@ plugin_magic_config_key (struct backend *b) > static int > next_open (void *nxdata, int readonly) > { > - struct b_h *b_h = nxdata; > + struct backend *b_next = nxdata; > > - return backend_open (b_h->b, readonly); > + return backend_open (b_next, readonly); > } With this change, 'next_open' and '(int (*)(void *, i...

...in .open, and serves as - * a stable ‘void *nxdata’ in the filter API. - */ -struct b_h { - struct backend *b; - void *handle; -}; - /* Note this frees the whole chain. */ static void filter_free (struct backend *b) @@ -186,20 +176,19 @@ filter_config_complete (struct backend *b) static int next_preconnect (void *nxdata, int readonly) { - struct b_h *b_h = nxdata; - return b_h->b->preconnect (b_h->b, readonly); + struct backend *b_next = nxdata; + return b_next->preconnect (b_next, readonly); } static int filter_preconnect (struct backend *b, int readonly) { struct...

It's confusing to use the same terminology for a single backend as for the linked list of backends. In particular it's often not clear if we're calling the next backend or the whole chain of backends. --- server/internal.h | 14 ++++++++++-- server/connections.c | 20 ++++++++--------- server/locks.c | 2 +- server/main.c

The third patch is a large but mechanical change which gets rid of passing around struct connection * entirely within the server, preferring instead to reference the connection through thread-local storage. I hope this is a gateway to simplifying other parts of the code. Rich.

...quot;\n"); + printf ("%s\n", f->filter.config_help); + } +} + +static void +filter_dump_fields (struct backend *b) +{ + struct backend_filter *f = container_of (b, struct backend_filter, backend); + + f->backend.next->dump_fields (f->backend.next); +} + +static int +next_config (void *nxdata, const char *key, const char *value) +{ + struct backend *b = nxdata; + b->config (b, key, value); + return 0; +} + +static void +filter_config (struct backend *b, const char *key, const char *value) +{ + struct backend_filter *f = container_of (b, struct backend_filter,...

.../nbdkit-filter.pod new file mode 100644 index 0000000..75157ef --- /dev/null +++ b/docs/nbdkit-filter.pod @@ -0,0 +1,501 @@ +=encoding utf8 + +=head1 NAME + +nbdkit-filter - How to write nbdkit filters + +=head1 SYNOPSIS + + #include <nbdkit-filter.h> + + static int + myfilter_config (nbdkit_next_config *next, void *nxdata, + const char *key, const char *value) + { + if (strcmp (key, "myparameter") == 0) { + // ... + return 0; + } + else { + // pass through to next filter or plugin + return next (nxdata, key, value); + } + } + + static stru...

.../nbdkit-filter.pod new file mode 100644 index 0000000..cec60c6 --- /dev/null +++ b/docs/nbdkit-filter.pod @@ -0,0 +1,510 @@ +=encoding utf8 + +=head1 NAME + +nbdkit-filter - How to write nbdkit filters + +=head1 SYNOPSIS + + #include <nbdkit-filter.h> + + static int + myfilter_config (nbdkit_next_config *next, void *nxdata, + const char *key, const char *value) + { + if (strcmp (key, "myparameter") == 0) { + // ... + return 0; + } + else { + // pass through to next filter or plugin + return next (nxdata, key, value); + } + } + + static stru...

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

...truct connection *conn = GET_CONN; struct b_conn_handle *h = &conn->handles[b->i]; controlpath_debug ("%s: open readonly=%d", b->name, readonly); @@ -166,12 +167,12 @@ backend_open (struct backend *b, struct connection *conn, int readonly) /* Most filters will call next_open first, resulting in * inner-to-outer ordering. */ - h->handle = b->open (b, conn, readonly); + h->handle = b->open (b, readonly); controlpath_debug ("%s: open returned handle %p", b->name, h->handle); if (h->handle == NULL) { if (b->i) /*...

v1 was here: https://www.redhat.com/archives/libguestfs/2020-February/msg00081.html v2 replaces struct connection *conn = GET_CONN; with GET_CONN; which sets conn implicitly and asserts that it is non-NULL. If we actually want to test if conn is non-NULL or behave differently, then you must use threadlocal_get_conn() instead, and some existing uses do that. Rich.

This is still tentative and needs a lot of work, but: - partition filter works, supporting MBR & GPT - prepare and finalize methods fixed - open method can now be changed (allowing readonly flag to be modified) - thread_model can be limited I believe I made most of the changes which were previously suggested in email. I think the only one I didn't was preventing inclusion of both

...expose them to plugins (only internally in the backend). However for filters they make more sense since they give you a place to prepare and finalize. They're necessary also for filters because filter.open could not call the next open method, save the (plugin) handle, then construct an nbdkit_next_ops object, and call pread. It _has_ to be split into two operations. This patch series is incomplete in at least three ways: - We still need a way for filter.open to call down to the next open function, so it can adjust the readonly flag (for copyonwrite filter, TBD). - Partition filter...

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