[Libguestfs] [PATCH nbdkit filters-v2 2/5] Introduce filters.

[Richard W.M. Jones]

On Fri, Jan 19, 2018 at 10:22:27AM -0600, Eric Blake wrote:
> > +Suggestions for filters
> > +-----------------------
> >  
> >  * adding artificial delays (see wdelay/rdelay options in the file
> >    plugin)
> 
> How about a filter that intentionally disables WRITE_ZEROES and/or FUA
> support, if for no other reason than for testing sane fallbacks and
> comparing speed differences?

If you have ideas for the TODO file just go ahead and
commit them, no need for review.

> > +=head1 SYNOPSIS
> 
> > +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.
> 
> Should we mention that the rules on filter semantics are stricter than
> for plugins, and that unlike plugins, we don't guarantee stable
> cross-version API?

Yes, we'll need to tighten up the language here.  I noted your other
patch which modified this text.

> > +=head1 CALLBACKS
> 
> > +=head2 C<.get_size>
> > +
> > + int64_t (*get_size) (struct nbdkit_next_ops *next_ops, 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>.
> 
> The rule on non-zero size matches plugins, but I'm not sure if there is
> a technical reason why we have that restriction.  Down the road, when I
> get the upstream NBD resize proposal finalized, I can see the ability to
> create an image with size 0, then use resize to update it, as something
> that would be nice to support.

OK that's interesting.  BTW qemu has historically had lots of bugs in
the block layer related to zero (or even < 4096 byte) devices.  We
added a workaround in libguestfs:

https://github.com/libguestfs/libguestfs/blob/a30b51747fc6605f50a9d5d8095fd2b6d1473b1c/lib/drives.c#L395

> > +
> > +=head2 C<.pread>
> > +
> > + int (*pread) (struct nbdkit_next_ops *next_ops, void *nxdata,
> > +               void *handle, void *buf, uint32_t count, uint64_t offset);
> 
> Wrong signature, missing the uint32_t flags that the backend interface
> has, and that I'm adding for plugins API_VERSION 2 for FUA support.

Right, but this patch is against the version 1 API.  We will need to
add flags at some point.

> > +=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.
> 
> Would it ever make sense to allow a filter to intentionally request a
> lower concurrency level than the underlying plugin?  At connection time,
> you'd compute the threading level as the lowest level requested by any
> element of the chain; it might make it easier to write plugins that
> aren't fully parallel (such filters may penalize the speed that can
> otherwise be achieved by using the plugin in isolation - but again, it
> may be useful for testing).  That would imply adding a
> backend.thread_model() callback, which needs to be exposed to filters
> but not to plugins (plugins continue to use the #define at compile
> time); if the filter does not define the .thread_model callback, it
> defaults to fully-parallel; but if it does define the callback, it can
> result in something less concurrent.

Yes, this is a good idea actually.

> > +=item B<--filter> FILTER
> > +
> > +Add a filter before the plugin.  This option may be given one or more
> > +times to stack filters in front of the plugin.  They are processed in
> > +the order they appear on the command line.  See L</FILTERS> and
> > +L<nbdkit-filter(3)>.
> > +
> 
> Does it ever make sense to provide the same filter more than once on the
> command line?  Or are we stating that a given filter can only be used
> once in the chain?

I couldn't think of a case where we'd need the same filter multiple
times.  It could be made to work but we'd have to ban global
variables and modify the load() function.

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/