[Libguestfs] [nbdkit PATCH v3 16/16] multi-conn: Knob to limit consistency emulation by export name

[Richard W.M. Jones]

On Fri, Mar 05, 2021 at 05:31:20PM -0600, Eric Blake wrote:
> Subject: multi-conn: Knob to limit consistency emulation by export name
>
> We can't enable the knob by default since many nbdkit plugins don't
> care about export name, but it is certainly more efficient to limit
> consistency to just clients visiting the same export.  Thankfully,
> NBD_CMD_FLUSH does not take size and offset, and therefore is safe
> (although wasting CPU cycles) to use even when a different export has
> a different size.
> 
> Done by adding another layer of indirection: all connections are
> combined into a group (a singleton when the knob is off, or by name
> when the knob is on), moving fast dirty tracking into that group.

I had to read this several times before I understood exactly what it
was about.  I think because "consistency emulation" sounds like it's a
typo for "consistent emulation" (it isn't).  Could we call it
something else like "flush-consistency" or "write-consistency" or
similar?

Now I think I do understand it, so ...

> ---
>  .../multi-conn/nbdkit-multi-conn-filter.pod   |  27 +++-
>  tests/Makefile.am                             |   2 +
>  filters/multi-conn/multi-conn.c               | 118 +++++++++++++++---
>  tests/test-multi-conn-name.sh                 |  88 +++++++++++++
>  tests/test-multi-conn-plugin.sh               |  63 ++++++----
>  5 files changed, 255 insertions(+), 43 deletions(-)
>  create mode 100755 tests/test-multi-conn-name.sh
> 
> diff --git a/filters/multi-conn/nbdkit-multi-conn-filter.pod b/filters/multi-conn/nbdkit-multi-conn-filter.pod
> index c8f334b3..efeba570 100644
> --- a/filters/multi-conn/nbdkit-multi-conn-filter.pod
> +++ b/filters/multi-conn/nbdkit-multi-conn-filter.pod
> @@ -5,7 +5,7 @@ nbdkit-multi-conn-filter - nbdkit multi-conn filter
>  =head1 SYNOPSIS
> 
>   nbdkit --filter=multi-conn plugin [multi-conn-mode=MODE] \
> -   [multi-conn-track-dirty=LEVEL] [plugin-args...]
> +   [multi-conn-track-dirty=LEVEL] [multi-conn-exportname=BOOL] [plugin-args...]
> 
>  =head1 DESCRIPTION
> 
> @@ -26,7 +26,8 @@ it also has additional modes useful for evaluating performance and
>  correctness of client and plugin multi-conn behaviors.  This filter
>  assumes that multiple connections to a plugin will eventually share
>  data, other than any caching effects; it is not suitable for use with
> -a plugin that produces completely independent data per connection.
> +a plugin that produces completely independent data per connection from
> +the same export name.

Which is (only?) tmpdisk.

Arguably tmpdisk is wrong to produce different output for the same
exportname, but it's also extremely convenient the way it works right
now.  I proposed a Client UUID extension for NBD which allows tmpdisk
to be compatible with multi-conn, but I never came up with a
watertight definition of Client UUID, which perhaps is a message for me.

>  Additional control over the behavior of client flush commands is
>  possible by combining this filter with L<nbdkit-fua-filter(1)>.  Note
> @@ -125,6 +126,27 @@ B<multi-conn-mode=emulate>, a client which disregards
>  NBD_FLAG_MULTI_CONN by flushing on each connection itself results in a
>  quadratic number of flush operations on the plugin.
> 
> +=item B<multi-conn-exportname=false>
> +
> +The exportname switch defaults to false for safety, and causes the
> +filter to flush across all active connections regardless of the export
> +name in use by that connection when doing emulation.  However, when a
> +plugin supports distinct data according to export name, this behavior
> +will penalize the performance of clients visiting an unrelated export
> +by spending time on replicated flush operations not actually relevant
> +to that export.
> +
> +=item B<multi-conn-exportname=true>
> +
> +Setting the exportname switch to true causes the filter to only
> +synchronize flushes to connections visiting the same export name.
> +This avoids penalizing clients visiting an unrelated export name (such
> +as L<nbdkit-file-plugin(1)> in B<dir=> mode), but is unsafe when used
> +with a plugin that serves shared content across all connections
> +regardless of the export name requested by the client, if that plugin
> +is not already multi-conn consistent (such as
> +L<nbdkit-vddk-plugin(1)>).

This description is quite nice, perhaps clearer than the
commit message :-)


Patch looks good, ACK.

Thanks,

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/