[Libguestfs] [PATCH nbdkit filters-v2 2/5] Introduce filters.
Eric Blake
eblake at redhat.com
Fri Jan 19 16:22:27 UTC 2018
On 01/19/2018 09:23 AM, Richard W.M. Jones wrote:
> Filters can be placed in front of plugins to modify their behaviour.
>
> This commit adds the <nbdkit-filter.h> header file, the manual page,
> the ‘filterdir’ directory (like ‘plugindir’), the ‘filters/’ source
> directory which will contain the actual filters, the ‘--filters’
> parameter, and the filters backend logic.
> ---
> +++ b/TODO
> @@ -34,10 +34,8 @@ nbdkit there is no compelling reason unless the result is better than
> qemu-nbd. For the majority of users it would be better if they were
> directed to qemu-nbd for these use cases.
>
> -Filters
> --------
> -
> -It should be possible to layer filters over plugins to do things like:
> +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?
> +++ b/docs/nbdkit-filter.pod
> @@ -0,0 +1,501 @@
> +=encoding utf8
> +
> +=head1 NAME
> +
> +nbdkit-filter - How to write nbdkit filters
> +
> +=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?
> +=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.
> +
> +=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.
> +=head2 C<.zero>
> +
> + int (*zero) (struct nbdkit_next_ops *next_ops, void *nxdata,
> + void *handle, uint32_t count, uint64_t offset, int may_trim);
> +
And so on (uint32_t flags instead of int may_trim).
> +=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.
> +++ b/docs/nbdkit.pod
> @@ -7,7 +7,7 @@ nbdkit - A toolkit for creating NBD servers
> =head1 SYNOPSIS
>
> nbdkit [-e EXPORTNAME] [--exit-with-parent] [-f]
> - [-g GROUP] [-i IPADDR]
> + [--filter=FILTER ...] [-g GROUP] [-i IPADDR]
> [--newstyle] [--oldstyle] [-P PIDFILE] [-p PORT] [-r]
> [--run CMD] [-s] [--selinux-label LABEL] [-t THREADS]
> [--tls=off|on|require] [--tls-certificates /path/to/certificates]
> @@ -119,6 +119,13 @@ not allowed with the oldstyle protocol.
>
> I<Don't> fork into the background.
>
> +=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?
> +struct nbdkit_next_ops {
> + 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);
> +};
Your documentation matches your code, and did not pick up my potential
changes to add 'uint32_t flags' everywhere.
I'll reply shortly with what my rebase looks like on top of yours...
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20180119/5d919d9f/attachment.sig>
More information about the Libguestfs
mailing list