Libguestfs - Jan 2018 - Re: [nbdkit] Proposed (new) filter API

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

--X78YbkCBd9ye7Cvs
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="nbdkit-filter.h"

/* nbdkit
 * Copyright (C) 2013-2017 Red Hat Inc.
 * All rights reserved.
 *
 * Redistribution and use in source and binary forms, with or without
 * modification, are permitted provided that the following conditions are
 * met:
 *
 * * Redistributions of source code must retain the above copyright
 * notice, this list of conditions and the following disclaimer.
 *
 * * Redistributions in binary form must reproduce the above copyright
 * notice, this list of conditions and the following disclaimer in the
 * documentation and/or other materials provided with the distribution.
 *
 * * Neither the name of Red Hat nor the names of its contributors may be
 * used to endorse or promote products derived from this software without
 * specific prior written permission.
 *
 * THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS
IS'' AND
 * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
 * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
 * PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR
 * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
 * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
 * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
 * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
 * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
 * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
 * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 * SUCH DAMAGE.
 */

/* See nbdkit-filter(3) for documentation and how to write a filter. */

#ifndef NBDKIT_FILTER_H
#define NBDKIT_FILTER_H

/* This header also defines some useful functions like nbdkit_debug
 * and nbdkit_parse_size which are appropriate for filters to use.
 */
#include <nbdkit-plugin.h>

#ifdef __cplusplus
extern "C" {
#endif

#define NBDKIT_FILTER_API_VERSION 1

typedef int (*nbdkit_next_config) (void *nxdata,
                                   const char *key, const char *value);
typedef int (*nbdkit_next_config_complete) (void *nxdata);

struct nbdkit_next {
  int64_t (*get_size) (void *nxdata);

  int (*can_write) (void *nxdata);
  int (*can_flush) (void *nxdata);
  int (*is_rotational) (void *nxdata);
  int (*can_trim) (void *nxdata);

  int (*pread) (void *nxdata, void *buf, uint32_t count, uint64_t offset);
  int (*pwrite) (void *nxdata,
                 const void *buf, uint32_t count, uint64_t offset);
  int (*flush) (void *nxdata);
  int (*trim) (void *nxdata, uint32_t count, uint64_t offset);
  int (*zero) (void *nxdata, uint32_t count, uint64_t offset, int may_trim);
};

struct nbdkit_filter {
  /* Do not set these fields directly; use NBDKIT_REGISTER_FILTER.
   * They exist so that we can support filters compiled against
   * one version of the header with a runtime compiled against a
   * different version with more (or fewer) fields.
   */
  uint64_t _struct_size;
  int _api_version;

  /* New fields will only be added at the end of the struct. */
  const char *name;
  const char *longname;
  const char *version;
  const char *description;

  void (*load) (void);
  void (*unload) (void);

  int (*config) (nbdkit_next_config *next, void *nxdata,
                 const char *key, const char *value);
  int (*config_complete) (nbdkit_next_config_complete *next, void *nxdata);
  const char *config_help;

  void * (*open) (int readonly);
  void (*close) (void *handle);

  int64_t (*get_size) (struct nbdkit_next *next, void *nxdata,
                       void *handle);

  int (*can_write) (struct nbdkit_next *next, void *nxdata,
                    void *handle);
  int (*can_flush) (struct nbdkit_next *next, void *nxdata,
                    void *handle);
  int (*is_rotational) (struct nbdkit_next *next,
                        void *nxdata,
                        void *handle);
  int (*can_trim) (struct nbdkit_next *next, void *nxdata,
                   void *handle);

  int (*pread) (struct nbdkit_next *next, void *nxdata,
                void *handle, void *buf, uint32_t count, uint64_t offset);
  int (*pwrite) (struct nbdkit_next *next, void *nxdata,
                 void *handle,
                 const void *buf, uint32_t count, uint64_t offset);
  int (*flush) (struct nbdkit_next *next, void *nxdata,
                void *handle);
  int (*trim) (struct nbdkit_next *next, void *nxdata,
               void *handle, uint32_t count, uint64_t offset);
  int (*zero) (struct nbdkit_next *next, void *nxdata,
               void *handle, uint32_t count, uint64_t offset, int may_trim);
};

#ifndef NBDKIT_CXX_LANG_C
#ifdef __cplusplus
#define NBDKIT_CXX_LANG_C extern "C"
#else
#define NBDKIT_CXX_LANG_C /* nothing */
#endif
#endif

#define NBDKIT_REGISTER_FILTER(filter)                                  \
  NBDKIT_CXX_LANG_C                                                     \
  struct nbdkit_filter *                                                \
  filter_init (void)                                                    \
  {                                                                     \
    (filter)._struct_size = sizeof (filter);                            \
    (filter)._api_version = NBDKIT_API_VERSION;                         \
    return &(filter);                                                   \
  }

#ifdef __cplusplus
}
#endif

#endif /* NBDKIT_FILTER_H */

--X78YbkCBd9ye7Cvs
Content-Type: text/x-pod; charset=utf-8
Content-Disposition: attachment; filename="nbdkit-filter.pod"
Content-Transfer-Encoding: 8bit

=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 struct nbdkit_filter filter = {
   .name              = "filter",
   .config            = myfilter_config,
   /* etc */
 };
 
 NBDKIT_REGISTER_FILTER(filter)

When this has been compiled to a shared library, do:

 nbdkit [--args ...] --filter=./myfilter.so plugin [key=value ...]

When debugging, use the I<-fv> options:

 nbdkit -fv --filter=./myfilter.so plugin [key=value ...]

=head1 DESCRIPTION

One or more nbdkit filters can be placed in front of an nbdkit plugin
to modify the behaviour of the plugin.  This manual page describes how
to create an nbdkit filter.

Filters can be used for example to limit requests to an offset/limit,
add copy-on-write support, or inject delays or errors (for testing).

Different filters can be stacked:

     NBD                   
  client  filter1  filter2     plugin 
 request                   

Each filter intercepts plugin functions (see L<nbdkit-plugin(3)>) and
can call the next filter or plugin in the chain, modifying parameters,
calling before the filter function, in the middle or after.  Filters
may even short-cut the chain.  As an example, to process its own
parameters the filter can intercept the C<.config> method:

 static int
 myfilter_config (nbdkit_next_config *next, void *nxdata,
                  const char *key, const char *value)
 {
   if (strcmp (key, "myparameter") == 0) {
     // ...
     // here you would handle this key, value
     // ...
     return 0;
   }
   else {
     // pass through to next filter or plugin
     return next (nxdata, key, value);
   }
 }
 
 static struct nbdkit_filter filter = {
   // ...
   .config            = myfilter_config,
   // ...
 };

The call to C<next (nxdata, ...)> calls the C<.config> method of the
next filter or plugin in the chain.  In the example above any
instances of C<myparameter=...> on the command line would not be seen
by the plugin.

To see example filters, take a look at the source of nbdkit, in the
C<filters> directory.

Filters must be written in C and must be fully thread safe.

=head1 C<nbdkit-filter.h>

All filters should start by including this header file:

 #include <nbdkit-filter.h>

=head1 C<struct nbdkit_filter>

All filters must define and register one C<struct nbdkit_filter>,
which contains the name of the filter and pointers to plugin methods
that the filter wants to intercept.

 static struct nbdkit_filter filter = {
   .name              = "filter",
   .longname          = "My Filter",
   .description       = "This is my great filter for nbdkit",
   .config            = myfilter_config,
   /* etc */
 };
 
 NBDKIT_REGISTER_FILTER(filter)

The C<.name> field is the name of the filter.  This is the only field
which is required.

=head1 NEXT PLUGIN

F<nbdkit-filter.h> defines two function types
(C<nbdkit_next_config>,
C<nbdkit_next_config_complete>) and a structure called C<struct
nbdkit_next>.  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 filters C<.config> and C<.config_complete> methods may only call
the next C<.config> or C<.config_complete> method in the chain
(optionally).

The filters C<.open> and C<.close> methods are called when a new
connection is opened or an old connection closed, and these have no
C<next> parameter.

The filters other methods like C<.get_size>, C<.pread> etc  always
called in the context of a connection  are passed a pointer to
C<struct nbdkit_next> which contains a subset of the plugin methods
that can be called during a connection.  It is possible for a filter
to issue (for example) extra read calls in response to a single
C<.pwrite> call.

You can modify parameters when you call the C<next> function.  However
be careful when modifying strings because for some methods
(eg. C<.config>) the plugin may save the string pointer that you pass
along.  So you may have to ensure that the string is not freed for the
lifetime of the server.

Note that if your filter registers a callback but in that callback it
doesn't call the C<next> function then the corresponding method in the
plugin will never be called.

=head1 CALLBACKS

C<struct nbdkit_filter> has some static fields describing the filter
and optional callback functions which can be used to intercept plugin
methods.

=head2 C<.name>

 const char *name;

This field (a string) is required, and B<must> contain only ASCII
alphanumeric characters and be unique amongst all filters.

=head2 C<.version>

 const char *version;

Filters may optionally set a version string which is displayed in help
and debugging output.

=head2 C<.longname>

 const char *longname;

An optional free text name of the filter.  This field is used in error
messages.

=head2 C<.description>

 const char *description;

An optional multi-line description of the filter.

=head2 C<.load>

 void load (void);

This is called once just after the filter is loaded into memory.  You
can use this to perform any global initialization needed by the
filter.

=head2 C<.unload>

 void unload (void);

This may be called once just before the filter is unloaded from
memory.  Note that it's not guaranteed that C<.unload> will always be
called (eg. the server might be killed or segfault), so you should try
to make the filter as robust as possible by not requiring cleanup.
See also L<nbdkit-plugin(3)/SHUTDOWN>.

=head2 C<.config>

 int (*config) (nbdkit_next_config *next, void *nxdata,
                const char *key, const char *value);

This intercepts the plugin C<.config> method and can be used by the
filter to parse its own command line parameters.  You should try to
make sure that command line parameter keys that the filter uses do not
conflict with ones that could be used by a plugin.

If there is an error, C<.config> should call C<nbdkit_error> with an
error message and return C<-1>.

=head2 C<.config_complete>

 int (*config_complete) (nbdkit_next_config_complete *next, void *nxdata);

This intercepts the plugin C<.config_complete> method and can be used
to ensure that all parameters needed by the filter were supplied on
the command line.

If there is an error, C<.config_complete> should call
C<nbdkit_error>
with an error message and return C<-1>.

=head2 C<.config_help>

 const char *config_help;

This optional multi-line help message should summarize any
C<key=value> parameters that it takes.  It does I<not> need to
repeat
what already appears in C<.description>.

If the filter doesn't take any config parameters you should probably
omit this.

=head2 C<.open>

 void * (*open) (int readonly);

This is called when a new client connection is opened and can be used
to allocate any per-connection data structures needed by the filter.
The handle (which is not the same as the plugin handle) is passed back
to other filter callbacks and could be freed in the C<.close>
callback.

Note that the handle is completely opaque to nbdkit, but it must not
be NULL.

If there is an error, C<.open> should call C<nbdkit_error> with an
error message and return C<NULL>.

=head2 C<.close>

 void (*close) (void *handle);

This is called when the client closes the connection.  It should clean
up any per-connection resources used by the filter.

=head2 C<.get_size>

 int64_t (*get_size) (struct nbdkit_next *next, void *nxdata,
                      void *handle);

This intercepts the plugin C<.get_size> method and can be used to read
or modify the apparent size of the block device that the NBD client
will see.

The returned size must be E<ge> 0.  If there is an error,
C<.get_size>
should call C<nbdkit_error> with an error message and return C<-1>.

=head2 C<.can_write>

=head2 C<.can_flush>

=head2 C<.is_rotational>

=head2 C<.can_trim>

 int (*can_write) (struct nbdkit_next *next, void *nxdata,
                   void *handle);
 int (*can_flush) (struct nbdkit_next *next, void *nxdata,
                   void *handle);
 int (*is_rotational) (struct nbdkit_next *next,
                       void *nxdata,
                       void *handle);
 int (*can_trim) (struct nbdkit_next *next, void *nxdata,
                  void *handle);

These intercept the corresponding plugin methods.

If there is an error, the callback should call C<nbdkit_error> with an
error message and return C<-1>.

=head2 C<.pread>

 int (*pread) (struct nbdkit_next *next, void *nxdata,
               void *handle, void *buf, uint32_t count, uint64_t offset);

This intercepts the plugin C<.pread> method and can be used to read or
modify data read by the plugin.

If there is an error (including a short read which couldn't be
recovered from), C<.pread> should call C<nbdkit_error> with an error
message B<and> set C<errno>, then return C<-1>.

=head2 C<.pwrite>

 int (*pwrite) (struct nbdkit_next *next, void *nxdata,
                void *handle,
                const void *buf, uint32_t count, uint64_t offset);

This intercepts the plugin C<.pwrite> method and can be used to modify
data written by the plugin.

If there is an error (including a short write which couldn't be
recovered from), C<.pwrite> should call C<nbdkit_error> with an
error
message B<and> set C<errno>, then return C<-1>.

=head2 C<.flush>

 int (*flush) (struct nbdkit_next *next, void *nxdata,
               void *handle);

This intercepts the plugin C<.flush> method and can be used to modify
flush requests.

If there is an error, C<.flush> should call C<nbdkit_error> with an
error message B<and> set C<errno>, then return C<-1>.

=head2 C<.trim>

 int (*trim) (struct nbdkit_next *next, void *nxdata,
              void *handle, uint32_t count, uint64_t offset);

This intercepts the plugin C<.trim> method and can be used to modify
trim requests.

If there is an error, C<.trim> should call C<nbdkit_error> with an
error message B<and> set C<errno>, then return C<-1>.

=head2 C<.zero>

 int (*zero) (struct nbdkit_next *next, void *nxdata,
              void *handle, uint32_t count, uint64_t offset, int may_trim);

This intercepts the plugin C<.zero> method and can be used to modify
zero requests.

If there is an error, C<.zero> should call C<nbdkit_error> with an
error message B<and> set C<errno>, then return C<-1>.

=head1 THREADS

Because filters can be mixed and used with any plugin and thus any
threading model supported by L<nbdkit-plugin(3)>, filters must be
thread safe.  They must be able to handle concurrent requests even on
the same handle.

Filters may have to use pthread primitives like mutexes to achieve
this.

=head1 DEBUGGING

Run the server with I<-f> and I<-v> options so it doesn't fork
and you
can see debugging information:

 nbdkit -fv --filter=./myfilter.so plugin [key=value [key=value [...]]]

To print debugging information from within the filter, call
C<nbdkit_debug>, which has the following prototype and works like
L<printf(3)>:

 void nbdkit_debug (const char *fs, ...);
 void nbdkit_vdebug (const char *fs, va_list args);

For convenience, C<nbdkit_debug> preserves the value of C<errno>.
Note that C<nbdkit_debug> only prints things when the server is in
verbose mode (I<-v> option).

=head1 INSTALLING THE FILTER

The filter is a C<*.so> file and possibly a manual page.  You can of
course install the filter C<*.so> file wherever you want, and users
will be able to use it by running:

 nbdkit --filter=/path/to/filter.so plugin [args]

However B<if> the shared library has a name of the form
C<nbdkit-I<name>-filter.so> B<and if> the library is installed
in the
C<$filterdir> directory, then users can be run it by only typing:

 nbdkit --filter=name plugin [args]

The location of the C<$filterdir> directory is set when nbdkit is
compiled and can be found by doing:

 nbdkit --dump-config

If using the pkg-config/pkgconf system then you can also find the
filter directory at compile time by doing:

 pkgconf nbdkit --variable=filterdir

=head1 PKG-CONFIG/PKGCONF

nbdkit provides a pkg-config/pkgconf file called C<nbdkit.pc> which
should be installed on the correct path when the nbdkit development
environment is installed.  You can use this in autoconf
F<configure.ac> scripts to test for the development environment:

 PKG_CHECK_MODULES([NBDKIT], [nbdkit >= 1.2.3])

The above will fail unless nbdkit E<ge> 1.2.3 and the header file is
installed, and will set C<NBDKIT_CFLAGS> and C<NBDKIT_LIBS>
appropriately for compiling filters.

You can also run pkg-config/pkgconf directly, for example:

 if ! pkgconf nbdkit --exists; then
   echo "you must install the nbdkit development environment"
   exit 1
 fi

=head1 SEE ALSO

L<nbdkit(1)>,
L<nbdkit-plugin(1)>.

=head1 AUTHORS

Richard W.M. Jones

=head1 COPYRIGHT

Copyright (C) 2013-2018 Red Hat Inc.

=head1 LICENSE

Redistribution and use in source and binary forms, with or without
modification, are permitted provided that the following conditions are
met:

=over 4

=item *

Redistributions of source code must retain the above copyright
notice, this list of conditions and the following disclaimer.

=item *

Redistributions in binary form must reproduce the above copyright
notice, this list of conditions and the following disclaimer in the
documentation and/or other materials provided with the distribution.

=item *

Neither the name of Red Hat nor the names of its contributors may be
used to endorse or promote products derived from this software without
specific prior written permission.

=back

THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS IS''
AND
ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR
CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
SUCH DAMAGE.

--X78YbkCBd9ye7Cvs--

On 01/16/2018 08:40 AM, Richard W.M. Jones wrote:
> 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).
A read turning into a read+write makes total sense when doing
copy-on-read thin-provisioning (the read from a remote site moves it by
writing a copy into a local site, so that future reads at that same
address are faster).  But yeah, we're trusting that filters will not
abuse expected semantics.
> 
> #ifndef NBDKIT_FILTER_H
> #define NBDKIT_FILTER_H
> 
> /* This header also defines some useful functions like nbdkit_debug
>  * and nbdkit_parse_size which are appropriate for filters to use.
Too much copy-and-paste?
>  */
> #include <nbdkit-plugin.h>
I guess you get those useful functions by inclusion, rather than by
direct definition.

> 
> struct nbdkit_next {
>   int64_t (*get_size) (void *nxdata);
> 
>   int (*can_write) (void *nxdata);
>   int (*can_flush) (void *nxdata);
>   int (*is_rotational) (void *nxdata);
>   int (*can_trim) (void *nxdata);
> 
>   int (*pread) (void *nxdata, void *buf, uint32_t count, uint64_t offset);
>   int (*pwrite) (void *nxdata,
>                  const void *buf, uint32_t count, uint64_t offset);
>   int (*flush) (void *nxdata);
>   int (*trim) (void *nxdata, uint32_t count, uint64_t offset);
>   int (*zero) (void *nxdata, uint32_t count, uint64_t offset, int
may_trim);
> };
While we want to support plugins that use the old API, should we declare
that filters only use the new API?  Or does my work to add FUA
passthrough need to be careful on how a filter does the work;
particularly if mixing a filter without FUA knowledge with a backend
that can do it?  I guess as long as we stabilize all of the filter and
FUA code into the same nbdkit release, it's okay if we tweak the filter
definition slightly in the next few patches.
> 
> struct nbdkit_filter {
>   /* Do not set these fields directly; use NBDKIT_REGISTER_FILTER.
>    * They exist so that we can support filters compiled against
>    * one version of the header with a runtime compiled against a
>    * different version with more (or fewer) fields.
>    */
>   uint64_t _struct_size;
>   int _api_version;
> 
>   /* New fields will only be added at the end of the struct. */
>   const char *name;
>   const char *longname;
>   const char *version;
>   const char *description;
> 
>   void (*load) (void);
>   void (*unload) (void);
> 
>   int (*config) (nbdkit_next_config *next, void *nxdata,
>                  const char *key, const char *value);
>   int (*config_complete) (nbdkit_next_config_complete *next, void *nxdata);
>   const char *config_help;
> 
>   void * (*open) (int readonly);
Is it conceivable that opening a filter may want to read data from the
underlying backend?  Do we have guarantees on lifecycle ordering across
the chain (if the command line uses filter1 filter2 plugin, we open
plugin first, filter2 second, filter1 last; then close in reverse order)?

> nbdkit-filter.pod
> 
> 
> Different filters can be stacked:
> 
>      NBD     ┌─────────┐    ┌─────────┐          ┌────────┐
>   client ───▶│ filter1 │───▶│ filter2 │── ─ ─ ──▶│ plugin │
>  request     └─────────┘    └─────────┘          └────────┘
Does conversion from POD to other formats preserve proper formatting of
this ASCII-art?

> =head2 C<.name>
> 
>  const char *name;
> 
> This field (a string) is required, and B<must> contain only ASCII
> alphanumeric characters and be unique amongst all filters.
Do filters and plugins share the same namespace, or can you have a
filter whose name matches a plugin?
> =head2 C<.open>
> 
>  void * (*open) (int readonly);
> 
> This is called when a new client connection is opened and can be used
> to allocate any per-connection data structures needed by the filter.
> The handle (which is not the same as the plugin handle) is passed back
> to other filter callbacks and could be freed in the C<.close>
> callback.
> 
> Note that the handle is completely opaque to nbdkit, but it must not
> be NULL.
> 
> If there is an error, C<.open> should call C<nbdkit_error> with
an
> error message and return C<NULL>.
I guess if you wanted to be truly opaque and support NULL, you'd have to
have something like:

int (*open) (int readonly, void **result)

which populates *result when returning 0, and returns -1 on failure.
But I'm also fine with your choice of stating that the opaque data has
to be anything other than NULL.

In general, this looks like a bit better approach than your first
attempt, in that it lets filters do a lot more during an open connection.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

On Tue, Jan 16, 2018 at 10:52:12AM -0600, Eric Blake
wrote:
> > /* This header also defines some useful functions like nbdkit_debug
> >  * and nbdkit_parse_size which are appropriate for filters to use.
> 
> Too much copy-and-paste?
> 
> >  */
> > #include <nbdkit-plugin.h>
> 
> I guess you get those useful functions by inclusion, rather than by
> direct definition.
Yes, that comment describes why we include the header.  I guess I
could delete the comment if it's confusing.
> > 
> > struct nbdkit_next {
> >   int64_t (*get_size) (void *nxdata);
> > 
> >   int (*can_write) (void *nxdata);
> >   int (*can_flush) (void *nxdata);
> >   int (*is_rotational) (void *nxdata);
> >   int (*can_trim) (void *nxdata);
> > 
> >   int (*pread) (void *nxdata, void *buf, uint32_t count, uint64_t
offset);
> >   int (*pwrite) (void *nxdata,
> >                  const void *buf, uint32_t count, uint64_t offset);
> >   int (*flush) (void *nxdata);
> >   int (*trim) (void *nxdata, uint32_t count, uint64_t offset);
> >   int (*zero) (void *nxdata, uint32_t count, uint64_t offset, int
may_trim);
> > };
> 
> While we want to support plugins that use the old API, should we declare
> that filters only use the new API?  Or does my work to add FUA
> passthrough need to be careful on how a filter does the work;
> particularly if mixing a filter without FUA knowledge with a backend
> that can do it?  I guess as long as we stabilize all of the filter and
> FUA code into the same nbdkit release, it's okay if we tweak the filter
> definition slightly in the next few patches.
I was hoping we could combine the FUA patches (or rather, push them
all together) so that in the final version the filter .pwrite
definition above would contain a FUA/flags parameter.
> > struct nbdkit_filter {
> >   /* Do not set these fields directly; use NBDKIT_REGISTER_FILTER.
> >    * They exist so that we can support filters compiled against
> >    * one version of the header with a runtime compiled against a
> >    * different version with more (or fewer) fields.
> >    */
> >   uint64_t _struct_size;
> >   int _api_version;
> > 
> >   /* New fields will only be added at the end of the struct. */
> >   const char *name;
> >   const char *longname;
> >   const char *version;
> >   const char *description;
> > 
> >   void (*load) (void);
> >   void (*unload) (void);
> > 
> >   int (*config) (nbdkit_next_config *next, void *nxdata,
> >                  const char *key, const char *value);
> >   int (*config_complete) (nbdkit_next_config_complete *next, void
*nxdata);
> >   const char *config_help;
> > 
> >   void * (*open) (int readonly);
> 
> Is it conceivable that opening a filter may want to read data from the
> underlying backend?
One case where this could make sense would be with a "partition"
filter (which needs to read the partition table early), and I did
consider that.  However I believe -- although haven't proven by
implementation -- that a partition filter can still be written, it
just needs to read the partition table once in one of the other
functions, whichever is called first, with a bit of mutex-ing.

The advantage to make open not be a passthrough is it somewhat
simplifies the implementation, otherwise it has to deal with the new
handle returned from the next element in the chain.
> Do we have guarantees on lifecycle ordering across
> the chain (if the command line uses filter1 filter2 plugin, we open
> plugin first, filter2 second, filter1 last; then close in reverse order)?
At the moment we open plugin, then filter2, then filter1, and close
them in the same order.  I was hoping not to define the order, since I
don't think it matters(?)

It could also be different for load/unload vs open/close.
> > Different filters can be stacked:
> > 
> >      NBD     ┌─────────┐    ┌─────────┐          ┌────────┐
> >   client ───▶│ filter1 │───▶│ filter2 │── ─ ─ ──▶│ plugin │
> >  request     └─────────┘    └─────────┘          └────────┘
> 
> Does conversion from POD to other formats preserve proper formatting of
> this ASCII-art?
We've used utf-8 in POD in libguestfs since forever.
> > =head2 C<.name>
> > 
> >  const char *name;
> > 
> > This field (a string) is required, and B<must> contain only
ASCII
> > alphanumeric characters and be unique amongst all filters.
> 
> Do filters and plugins share the same namespace, or can you have a
> filter whose name matches a plugin?
Different namespaces because they go into different directories
(plugindir vs filterdir).
> > =head2 C<.open>
> > 
> >  void * (*open) (int readonly);
> > 
> > This is called when a new client connection is opened and can be used
> > to allocate any per-connection data structures needed by the filter.
> > The handle (which is not the same as the plugin handle) is passed back
> > to other filter callbacks and could be freed in the C<.close>
> > callback.
> > 
> > Note that the handle is completely opaque to nbdkit, but it must not
> > be NULL.
> > 
> > If there is an error, C<.open> should call C<nbdkit_error>
with an
> > error message and return C<NULL>.
> 
> I guess if you wanted to be truly opaque and support NULL, you'd have
to
> have something like:
> 
> int (*open) (int readonly, void **result)
> 
> which populates *result when returning 0, and returns -1 on failure.
> But I'm also fine with your choice of stating that the opaque data has
> to be anything other than NULL.
It's consistent with how plugins work now.

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/