[Libguestfs] [nbdkit PATCH v3 15/16] multi-conn: New filter

[Richard W.M. Jones]

On Fri, Mar 05, 2021 at 05:31:19PM -0600, Eric Blake wrote:
> Implement a TODO item of emulating multi-connection consistency via
> multiple plugin flush calls to allow a client to assume that a flush
> on a single connection is good enough.  This also gives us some
> fine-tuning over whether to advertise the bit, including some setups
> that are unsafe but may be useful in timing tests.
> 
> Testing is interesting: I used the sh plugin to implement a server
> that intentionally keeps a per-connection cache.
> 
> Note that this filter assumes that multiple connections will still
> share the same data (other than caching effects); effects are not
> guaranteed when trying to mix it with more exotic plugins like info
> that violate that premise.
> ---
>  filters/cache/nbdkit-cache-filter.pod         |   5 +-
>  filters/fua/nbdkit-fua-filter.pod             |   7 +
>  .../multi-conn/nbdkit-multi-conn-filter.pod   | 178 +++++++
>  filters/nocache/nbdkit-nocache-filter.pod     |   1 +
>  filters/noextents/nbdkit-noextents-filter.pod |   1 +
>  .../noparallel/nbdkit-noparallel-filter.pod   |   1 +
>  filters/nozero/nbdkit-nozero-filter.pod       |   1 +
>  configure.ac                                  |   4 +-
>  filters/multi-conn/Makefile.am                |  68 +++
>  tests/Makefile.am                             |   9 +
>  filters/multi-conn/multi-conn.c               | 435 ++++++++++++++++++
>  tests/test-multi-conn-plugin.sh               | 122 +++++
>  tests/test-multi-conn.sh                      | 293 ++++++++++++
>  TODO                                          |   7 -
>  14 files changed, 1123 insertions(+), 9 deletions(-)
>  create mode 100644 filters/multi-conn/nbdkit-multi-conn-filter.pod
>  create mode 100644 filters/multi-conn/Makefile.am
>  create mode 100644 filters/multi-conn/multi-conn.c
>  create mode 100755 tests/test-multi-conn-plugin.sh
>  create mode 100755 tests/test-multi-conn.sh
> 
> diff --git a/filters/cache/nbdkit-cache-filter.pod b/filters/cache/nbdkit-cache-filter.pod
> index 34fd0b29..482a9c55 100644
> --- a/filters/cache/nbdkit-cache-filter.pod
> +++ b/filters/cache/nbdkit-cache-filter.pod
> @@ -41,7 +41,9 @@ an explicit flush is done by the client.
> 
>  This is the default caching mode, and is safe if your client issues
>  flush requests correctly (which is true for modern Linux and other
> -well-written NBD clients).
> +well-written NBD clients).  Note that this mode is able to advertise
> +multi-connection consistency even without the use of
> +L<nbdkit-multi-conn-filter(1)>.

I would say this hunk is confusing and unnecessary.  I mean if you
shouldn't be using --filter=multi-conn, then just don't say anything :-)
If it really must be mentioned somewhere then probably better to say
it in the multi-conn man page instead IMHO.

>  =item B<cache=writethrough>
> 
> @@ -162,6 +164,7 @@ L<nbdkit(1)>,
>  L<nbdkit-file-plugin(1)>,
>  L<nbdkit-cacheextents-filter(1)>,
>  L<nbdkit-readahead-filter(1)>,
> +L<nbdkit-multi-conn-filter(1)>,
>  L<nbdkit-filter(3)>,
>  L<qemu-img(1)>.

(Which would imply also removing this hunk)

> diff --git a/filters/fua/nbdkit-fua-filter.pod b/filters/fua/nbdkit-fua-filter.pod
> index 0f9b9744..9b0d84b5 100644
> --- a/filters/fua/nbdkit-fua-filter.pod
> +++ b/filters/fua/nbdkit-fua-filter.pod
> @@ -16,6 +16,12 @@ This filter can be used to disable FUA and flush requests for speed
>  server fallbacks, and for evaluating timing differences between proper
>  use of FUA compared to a full flush.
> 
> +Note that by default, the NBD protocol does not guarantee that the use
> +of FUA from one connection will be visible from another connection
> +unless the server advertised NBD_FLAG_MULTI_CONN.  You may wish to
> +combine this filter with L<nbdkit-multi-conn-filter(1)> if you plan on
> +making multiple connections to the plugin.

This is a case where mentioning the other filter _is_ a good idea.

>  =head1 PARAMETERS
> 
>  The C<fuamode> parameter is optional and controls which mode the
> @@ -137,6 +143,7 @@ L<nbdkit-file-plugin(1)>,
>  L<nbdkit-filter(3)>,
>  L<nbdkit-blocksize-filter(1)>,
>  L<nbdkit-log-filter(1)>,
> +L<nbdkit-multi-conn-filter(1)>,
>  L<nbdkit-nocache-filter(1)>,
>  L<nbdkit-noextents-filter(1)>,
>  L<nbdkit-noparallel-filter(1)>,
> diff --git a/filters/multi-conn/nbdkit-multi-conn-filter.pod b/filters/multi-conn/nbdkit-multi-conn-filter.pod
> new file mode 100644
> index 00000000..c8f334b3
> --- /dev/null
> +++ b/filters/multi-conn/nbdkit-multi-conn-filter.pod
[...]

About the rest of the patch, ACK.

It has to be said I did find the documentation a bit hard to follow.
If you push this I may follow up with my own documentation fixes (I'll
run it by you first though).

Thanks,

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org