Eric Blake
2021-Mar-05 23:31 UTC
[Libguestfs] [nbdkit PATCH v3 15/16] multi-conn: New filter
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)>. =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)>. 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. + =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 @@ -0,0 +1,178 @@ +=head1 NAME + +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...] + +=head1 DESCRIPTION + +C<nbdkit-multi-conn-filter> is a filter that enables alterations to +the server's advertisement of NBD_FLAG_MULTI_CONN. When a server +permits multiple simultaneous clients, and sets this flag, a client +may assume that all connections see a consistent view (after getting +the server reply from a write in one connection, sending a flush +command on a single connection and waiting for that reply then +guarantees that all connections will then see the just-written data). +If the flag is not advertised, a client must presume that separate +connections may have utilized independent caches, where a flush on one +connection does not affect the cache of a second connection. + +The main use of this filter is to emulate consistent semantics across +multiple connections when not already provided by a plugin, although +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. + +Additional control over the behavior of client flush commands is +possible by combining this filter with L<nbdkit-fua-filter(1)>. Note +that L<nbdkit-cache-filter(1)> is also able to provide +multi-connection consistency, but at the expense of an extra layer of +caching not needed with this filter. + +=head1 PARAMETERS + +=over 4 + +=item B<multi-conn-mode=auto> + +This filter defaults to B<auto> mode. If the selected thread model is +SERIALIZE_CONNECTIONS, then this filter behaves the same as B<disable> +mode; if the plugin advertises multi-conn, then this filter behaves +the same as B<plugin> mode; otherwise, this filter behaves the same as +B<emulate> mode. As a result, this mode advertises +NBD_FLAG_MULTI_CONN to the client exactly when the server supports +multiple simultaneous connections. + +=item B<multi-conn-mode=emulate> + +When B<emulate> mode is chosen, then this filter tracks all parallel +connections. When a client issues a flush command over any one +connection (including an implied flush by a write command with the FUA +(force unit access) flag set), the filter then replicates that flush +across each connection to the plugin (although the amount of plugin +calls can be tuned by adjusting B<multi-conn-track-dirty>). This +assumes that flushing each connection is enough to clear any +per-connection cached data, in order to give each connection a +consistent view of the image; therefore, this mode advertises +NBD_FLAG_MULTI_CONN to the client. + +Note that in this mode, a client will be unable to connect if the +plugin lacks support for flush, as there would be no way to emulate +cross-connection consistency. + +=item B<multi-conn-mode=disable> + +When B<disable> mode is chosen, this filter disables advertisement of +NBD_FLAG_MULTI_CONN to the client, even if the plugin supports it, and +does not replicate flush commands across connections. This is useful +for testing whether a client with multiple connections properly sends +multiple flushes in order to overcome per-connection caching. + +=item B<multi-conn-mode=plugin> + +When B<plugin> mode is chosen, the filter does not change whether +NBD_FLAG_MULTI_CONN is advertised by the plugin, and does not +replicate flush commands across connections; but still honors +B<multi-conn-track-dirty> for minimizing the number of flush commands +passed on to the plugin. + +=item B<multi-conn-mode=unsafe> + +When B<unsafe> mode is chosen, this filter blindly advertises +NBD_FLAG_MULTI_CONN to the client even if the plugin lacks support. +This is dangerous, and risks data corruption if the client makes +assumptions about data consistency that were not actually met. + +=item B<multi-conn-track-dirty=fast> + +When dirty tracking is set to B<fast>, the filter tracks whether any +connection has caused the image to be dirty (any write, zero, or trim +commands since the last flush, regardless of connection); if all +connections are clean, a client flush command is ignored rather than +sent on to the plugin. In this mode, a flush action on one connection +marks all other connections as clean, regardless of whether the filter +actually advertised NBD_FLAG_MULTI_CONN, which can result in less +activity when a client sends multiple flushes rather than taking +advantage of multi-conn semantics. This is safe with +B<multi-conn-mode=emulate>, but potentially unsafe with +B<multi-conn-mode=plugin> when the plugin did not advertise +multi-conn, as it does not track whether a read may have cached stale +data prior to a flush. + +=item B<multi-conn-track-dirty=connection> + +Dirty tracking is set to B<connection> by default, where the filter +tracks whether a given connection is dirty (any write, zero, or trim +commands since the last flush on the given connection, and any read +since the last flush on any other connection); if the connection is +clean, a flush command to that connection (whether directly from the +client, or replicated by B<multi-conn-mode=emulate> is ignored rather +than sent on to the plugin. This mode may result in more flush calls +than B<multi-conn-track-dirty=fast>, but in turn is safe to use with +B<multi-conn-mode=plugin>. + +=item B<multi-conn-track-dirty=off> + +When dirty tracking is set to B<off>, all flush commands from the +client are passed on to the plugin, regardless of whether the flush +would be needed for consistency. Note that when combined with +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. + +=back + +=head1 EXAMPLES + +Provide consistent cross-connection flush semantics on top of a plugin +that lacks it natively: + + nbdkit --filter=multi-conn vddk /absolute/path/to/file.vmdk + +Minimize the number of expensive flush operations performed when +utilizing a plugin that has multi-conn consistency from a client that +blindly flushes across every connection: + + nbdkit --filter=multi-conn file multi-conn-mode=plugin \ + multi-conn-track-dirty=fast disk.img + +=head1 FILES + +=over 4 + +=item F<$filterdir/nbdkit-multi-conn-filter.so> + +The filter. + +Use C<nbdkit --dump-config> to find the location of C<$filterdir>. + +=back + +=head1 VERSION + +C<nbdkit-multi-conn-filter> first appeared in nbdkit 1.26. + +=head1 SEE ALSO + +L<nbdkit(1)>, +L<nbdkit-file-plugin(1)>, +L<nbdkit-filter(3)>, +L<nbdkit-cache-filter(1)>, +L<nbdkit-fua-filter(1)>, +L<nbdkit-nocache-filter(1)>, +L<nbdkit-noextents-filter(1)>, +L<nbdkit-noparallel-filter(1)>, +L<nbdkit-nozero-filter(1)>. + +=head1 AUTHORS + +Eric Blake + +=head1 COPYRIGHT + +Copyright (C) 2018-2021 Red Hat Inc. diff --git a/filters/nocache/nbdkit-nocache-filter.pod b/filters/nocache/nbdkit-nocache-filter.pod index de970136..3c30533c 100644 --- a/filters/nocache/nbdkit-nocache-filter.pod +++ b/filters/nocache/nbdkit-nocache-filter.pod @@ -77,6 +77,7 @@ L<nbdkit-file-plugin(1)>, L<nbdkit-filter(3)>, L<nbdkit-cache-filter(1)>, L<nbdkit-fua-filter(1)>, +L<nbdkit-multi-conn-filter(1)>, L<nbdkit-noextents-filter(1)>, L<nbdkit-noparallel-filter(1)>, L<nbdkit-nozero-filter(1)>. diff --git a/filters/noextents/nbdkit-noextents-filter.pod b/filters/noextents/nbdkit-noextents-filter.pod index 0260a5cf..891b197d 100644 --- a/filters/noextents/nbdkit-noextents-filter.pod +++ b/filters/noextents/nbdkit-noextents-filter.pod @@ -49,6 +49,7 @@ L<nbdkit(1)>, L<nbdkit-filter(3)>, L<nbdkit-extentlist-filter(1)>, L<nbdkit-fua-filter(1)>, +L<nbdkit-multi-conn-filter(1)>, L<nbdkit-nocache-filter(1)>, L<nbdkit-noparallel-filter(1)>, L<nbdkit-nozero-filter(1)>, diff --git a/filters/noparallel/nbdkit-noparallel-filter.pod b/filters/noparallel/nbdkit-noparallel-filter.pod index 16861ad9..8ac8d1a7 100644 --- a/filters/noparallel/nbdkit-noparallel-filter.pod +++ b/filters/noparallel/nbdkit-noparallel-filter.pod @@ -76,6 +76,7 @@ L<nbdkit-file-plugin(1)>, L<nbdkit-filter(3)>, L<nbdkit-fua-filter(1)>, L<nbdkit-limit-filter(1)>, +L<nbdkit-multi-conn-filter(1)>, L<nbdkit-nocache-filter(1)>, L<nbdkit-noextents-filter(1)>, L<nbdkit-nozero-filter(1)>. diff --git a/filters/nozero/nbdkit-nozero-filter.pod b/filters/nozero/nbdkit-nozero-filter.pod index 27a030f4..ad6e590d 100644 --- a/filters/nozero/nbdkit-nozero-filter.pod +++ b/filters/nozero/nbdkit-nozero-filter.pod @@ -131,6 +131,7 @@ L<nbdkit(1)>, L<nbdkit-file-plugin(1)>, L<nbdkit-filter(3)>, L<nbdkit-fua-filter(1)>, +L<nbdkit-multi-conn-filter(1)>, L<nbdkit-nocache-filter(1)>, L<nbdkit-noparallel-filter(1)>, L<nbdkit-noextents-filter(1)>. diff --git a/configure.ac b/configure.ac index 587fac0c..4c376a07 100644 --- a/configure.ac +++ b/configure.ac @@ -1,5 +1,5 @@ # nbdkit -# Copyright (C) 2013-2020 Red Hat Inc. +# Copyright (C) 2013-2021 Red Hat Inc. # # Redistribution and use in source and binary forms, with or without # modification, are permitted provided that the following conditions are @@ -128,6 +128,7 @@ filters="\ ip \ limit \ log \ + multi-conn \ nocache \ noextents \ nofilter \ @@ -1259,6 +1260,7 @@ AC_CONFIG_FILES([Makefile filters/ip/Makefile filters/limit/Makefile filters/log/Makefile + filters/multi-conn/Makefile filters/nocache/Makefile filters/noextents/Makefile filters/nofilter/Makefile diff --git a/filters/multi-conn/Makefile.am b/filters/multi-conn/Makefile.am new file mode 100644 index 00000000..778b8947 --- /dev/null +++ b/filters/multi-conn/Makefile.am @@ -0,0 +1,68 @@ +# nbdkit +# Copyright (C) 2021 Red Hat Inc. +# +# 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. + +include $(top_srcdir)/common-rules.mk + +EXTRA_DIST = nbdkit-multi-conn-filter.pod + +filter_LTLIBRARIES = nbdkit-multi-conn-filter.la + +nbdkit_multi_conn_filter_la_SOURCES = \ + multi-conn.c \ + $(top_srcdir)/include/nbdkit-filter.h \ + $(NULL) + +nbdkit_multi_conn_filter_la_CPPFLAGS = \ + -I$(top_srcdir)/include \ + -I$(top_srcdir)/common/include \ + -I$(top_srcdir)/common/utils \ + $(NULL) +nbdkit_multi_conn_filter_la_CFLAGS = $(WARNINGS_CFLAGS) +nbdkit_multi_conn_filter_la_LIBADD = \ + $(top_builddir)/common/utils/libutils.la \ + $(IMPORT_LIBRARY_ON_WINDOWS) \ + $(NULL) +nbdkit_multi_conn_filter_la_LDFLAGS = \ + -module -avoid-version -shared $(NO_UNDEFINED_ON_WINDOWS) \ + -Wl,--version-script=$(top_srcdir)/filters/filters.syms \ + $(NULL) + +if HAVE_POD + +man_MANS = nbdkit-multi-conn-filter.1 +CLEANFILES += $(man_MANS) + +nbdkit-multi-conn-filter.1: nbdkit-multi-conn-filter.pod + $(PODWRAPPER) --section=1 --man $@ \ + --html $(top_builddir)/html/$@.html \ + $< + +endif HAVE_POD diff --git a/tests/Makefile.am b/tests/Makefile.am index e8c9c535..22cafa21 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -1544,6 +1544,15 @@ EXTRA_DIST += \ test-log-script-info.sh \ $(NULL) +# multi-conn filter test. +TESTS += \ + test-multi-conn.sh \ + $(NULL) +EXTRA_DIST += \ + test-multi-conn-plugin.sh \ + test-multi-conn.sh \ + $(NULL) + # nofilter test. TESTS += test-nofilter.sh EXTRA_DIST += test-nofilter.sh diff --git a/filters/multi-conn/multi-conn.c b/filters/multi-conn/multi-conn.c new file mode 100644 index 00000000..ed17f7c6 --- /dev/null +++ b/filters/multi-conn/multi-conn.c @@ -0,0 +1,435 @@ +/* nbdkit + * Copyright (C) 2021 Red Hat Inc. + * + * 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. + */ + +#include <config.h> + +#include <stdio.h> +#include <stdlib.h> +#include <stdint.h> +#include <string.h> +#include <stdbool.h> +#include <assert.h> +#include <pthread.h> + +#include <nbdkit-filter.h> + +#include "cleanup.h" +#include "vector.h" + +/* Track results of .config */ +static enum MultiConnMode { + AUTO, + EMULATE, + PLUGIN, + UNSAFE, + DISABLE, +} mode; + +static enum TrackDirtyMode { + CONN, + FAST, + OFF, +} track; + +enum dirty { + WRITE = 1, /* A write may have populated a cache */ + READ = 2, /* A read may have populated a cache */ +}; + +/* Coordination between connections. */ +static pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER; + +/* The list of handles to active connections. */ +struct handle { + nbdkit_next *next; + enum MultiConnMode mode; /* Runtime resolution of mode==AUTO */ + enum dirty dirty; /* What aspects of this connection are dirty */ +}; +DEFINE_VECTOR_TYPE(conns_vector, struct handle *); +static conns_vector conns = empty_vector; +static bool dirty; /* True if any connection is dirty */ + +/* Accept 'multi-conn-mode=mode' and 'multi-conn-track-dirty=level' */ +static int +multi_conn_config (nbdkit_next_config *next, nbdkit_backend *nxdata, + const char *key, const char *value) +{ + if (strcmp (key, "multi-conn-mode") == 0) { + if (strcmp (value, "auto") == 0) + mode = AUTO; + else if (strcmp (value, "emulate") == 0) + mode = EMULATE; + else if (strcmp (value, "plugin") == 0) + mode = PLUGIN; + else if (strcmp (value, "disable") == 0) + mode = DISABLE; + else if (strcmp (value, "unsafe") == 0) + mode = UNSAFE; + else { + nbdkit_error ("unknown multi-conn mode '%s'", value); + return -1; + } + return 0; + } + else if (strcmp (key, "multi-conn-track-dirty") == 0) { + if (strcmp (value, "connection") == 0 || + strcmp (value, "conn") == 0) + track = CONN; + else if (strcmp (value, "fast") == 0) + track = FAST; + else if (strcmp (value, "off") == 0) + track = OFF; + else { + nbdkit_error ("unknown multi-conn track-dirty setting '%s'", value); + return -1; + } + return 0; + } + return next (nxdata, key, value); +} + +#define multi_conn_config_help \ + "multi-conn-mode=<MODE> 'auto' (default), 'emulate', 'plugin',\n" \ + " 'disable', or 'unsafe'.\n" \ + "multi-conn-track-dirty=<LEVEL> 'conn' (default), 'fast', or 'off'.\n" + +static int +multi_conn_get_ready (nbdkit_next_get_ready *next, nbdkit_backend *nxdata, + int thread_model) +{ + if (thread_model == NBDKIT_THREAD_MODEL_SERIALIZE_CONNECTIONS && + mode == AUTO) + mode = DISABLE; + return next (nxdata); +} + +static void * +multi_conn_open (nbdkit_next_open *next, nbdkit_context *nxdata, + int readonly, const char *exportname, int is_tls) +{ + struct handle *h; + + if (next (nxdata, readonly, exportname) == -1) + return NULL; + + /* Allocate here, but populate and insert into list in .prepare */ + h = calloc (1, sizeof *h); + if (h == NULL) { + nbdkit_error ("calloc: %m"); + return NULL; + } + return h; +} + +static int +multi_conn_prepare (nbdkit_next *next, void *handle, int readonly) +{ + struct handle *h = handle; + int r; + + h->next = next; + if (mode == AUTO) { /* See also .get_ready turning AUTO into DISABLE */ + r = next->can_multi_conn (next); + if (r == -1) + return -1; + if (r == 0) + h->mode = EMULATE; + else + h->mode = PLUGIN; + } + else + h->mode = mode; + if (h->mode == EMULATE && next->can_flush (next) != 1) { + nbdkit_error ("emulating multi-conn requires working flush"); + return -1; + } + + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock); + return conns_vector_append (&conns, h); +} + +static int +multi_conn_finalize (nbdkit_next *next, void *handle) +{ + struct handle *h = handle; + + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock); + assert (h->next == next); + + /* XXX should we add a config param to flush if the client forgot? */ + for (size_t i = 0; i < conns.size; i++) { + if (conns.ptr[i] == h) { + conns_vector_remove (&conns, i); + break; + } + } + return 0; +} + +static void +multi_conn_close (void *handle) +{ + free (handle); +} + +static int +multi_conn_can_fua (nbdkit_next *next, void *handle) +{ + /* If the backend has native FUA support but is not multi-conn + * consistent, and we have to flush on every connection, then we are + * better off advertising emulated fua rather than native. + */ + struct handle *h = handle; + int fua = next->can_fua (next); + + assert (h->mode != AUTO); + if (fua == NBDKIT_FUA_NATIVE && h->mode == EMULATE) + return NBDKIT_FUA_EMULATE; + return fua; +} + +static int +multi_conn_can_multi_conn (nbdkit_next *next, void *handle) +{ + struct handle *h = handle; + + switch (h->mode) { + case EMULATE: + return 1; + case PLUGIN: + return next->can_multi_conn (next); + case DISABLE: + return 0; + case UNSAFE: + return 1; + case AUTO: /* Not possible, see .prepare */ + default: + abort (); + } +} + +static void +mark_dirty (struct handle *h, bool is_write) +{ + /* No need to grab lock here: the NBD spec is clear that a client + * must wait for the response to a flush before sending the next + * command that expects to see the result of that flush, so any race + * in accessing dirty can be traced back to the client improperly + * sending a flush in parallel with other live commands. + */ + switch (track) { + case CONN: + h->dirty |= is_write ? WRITE : READ; + /* fallthrough */ + case FAST: + if (is_write) + dirty = true; + break; + case OFF: + break; + default: + abort (); + } +} + +static int +multi_conn_flush (nbdkit_next *next, + void *handle, uint32_t flags, int *err); + +static int +multi_conn_pread (nbdkit_next *next, + void *handle, void *buf, uint32_t count, uint64_t offs, + uint32_t flags, int *err) +{ + struct handle *h = handle; + + mark_dirty (h, false); + return next->pread (next, buf, count, offs, flags, err); +} + +static int +multi_conn_pwrite (nbdkit_next *next, + void *handle, const void *buf, uint32_t count, + uint64_t offs, uint32_t flags, int *err) +{ + struct handle *h = handle; + bool need_flush = false; + + if (flags & NBDKIT_FLAG_FUA) { + if (h->mode == EMULATE) { + mark_dirty (h, true); + need_flush = true; + flags &= ~NBDKIT_FLAG_FUA; + } + } + else + mark_dirty (h, true); + + if (next->pwrite (next, buf, count, offs, flags, err) == -1) + return -1; + if (need_flush) + return multi_conn_flush (next, h, 0, err); + return 0; +} + +static int +multi_conn_zero (nbdkit_next *next, + void *handle, uint32_t count, uint64_t offs, uint32_t flags, + int *err) +{ + struct handle *h = handle; + bool need_flush = false; + + if (flags & NBDKIT_FLAG_FUA) { + if (h->mode == EMULATE) { + mark_dirty (h, true); + need_flush = true; + flags &= ~NBDKIT_FLAG_FUA; + } + } + else + mark_dirty (h, true); + + if (next->zero (next, count, offs, flags, err) == -1) + return -1; + if (need_flush) + return multi_conn_flush (next, h, 0, err); + return 0; +} + +static int +multi_conn_trim (nbdkit_next *next, + void *handle, uint32_t count, uint64_t offs, uint32_t flags, + int *err) +{ + struct handle *h = handle; + bool need_flush = false; + + if (flags & NBDKIT_FLAG_FUA) { + if (h->mode == EMULATE) { + mark_dirty (h, true); + need_flush = true; + flags &= ~NBDKIT_FLAG_FUA; + } + } + else + mark_dirty (h, true); + + if (next->trim (next, count, offs, flags, err) == -1) + return -1; + if (need_flush) + return multi_conn_flush (next, h, 0, err); + return 0; +} + +static int +multi_conn_cache (nbdkit_next *next, + void *handle, uint32_t count, uint64_t offs, uint32_t flags, + int *err) +{ + struct handle *h = handle; + + mark_dirty (h, false); + return next->cache (next, count, offs, flags, err); +} + +static int +multi_conn_flush (nbdkit_next *next, + void *handle, uint32_t flags, int *err) +{ + struct handle *h = handle, *h2; + size_t i; + + if (h->mode == EMULATE) { + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock); + for (i = 0; i < conns.size; i++) { + h2 = conns.ptr[i]; + if (track == OFF || (dirty && (track == FAST || h2->dirty & READ)) || + h2->dirty & WRITE) { + if (h2->next->flush (h2->next, flags, err) == -1) + return -1; + h2->dirty = 0; + } + } + dirty = 0; + } + else { + /* !EMULATE: Check if the image is clean, allowing us to skip a flush. */ + if (track != OFF && !dirty) + return 0; + /* Perform the flush, then update dirty tracking. */ + if (next->flush (next, flags, err) == -1) + return -1; + switch (track) { + case CONN: + if (next->can_multi_conn (next) == 1) { + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock); + for (i = 0; i < conns.size; i++) + conns.ptr[i]->dirty = 0; + dirty = 0; + } + else + h->dirty = 0; + break; + case FAST: + dirty = false; + break; + case OFF: + break; + default: + abort (); + } + } + return 0; +} + +static struct nbdkit_filter filter = { + .name = "multi-conn", + .longname = "nbdkit multi-conn filter", + .config = multi_conn_config, + .config_help = multi_conn_config_help, + .get_ready = multi_conn_get_ready, + .open = multi_conn_open, + .prepare = multi_conn_prepare, + .finalize = multi_conn_finalize, + .close = multi_conn_close, + .can_fua = multi_conn_can_fua, + .can_multi_conn = multi_conn_can_multi_conn, + .pread = multi_conn_pread, + .pwrite = multi_conn_pwrite, + .trim = multi_conn_trim, + .zero = multi_conn_zero, + .cache = multi_conn_cache, + .flush = multi_conn_flush, +}; + +NBDKIT_REGISTER_FILTER(filter) diff --git a/tests/test-multi-conn-plugin.sh b/tests/test-multi-conn-plugin.sh new file mode 100755 index 00000000..7262a348 --- /dev/null +++ b/tests/test-multi-conn-plugin.sh @@ -0,0 +1,122 @@ +#!/usr/bin/env bash +# nbdkit +# Copyright (C) 2018-2021 Red Hat Inc. +# +# 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. + +# Test plugin used by test-multi-conn.sh. +# This plugin purposefully maintains a per-connection cache. +# An optional parameter tightfua=true controls whether FUA acts on +# just the given region, or on all pending ops in the current connection. +# Note that an earlier cached write on one connection can overwrite a later +# FUA write on another connection - this is okay (the client is buggy if +# it ever sends overlapping writes without coordinating flushes and still +# expects any particular write to occur last). + +fill_cache() { + if test ! -f "$tmpdir/$1"; then + cp "$tmpdir/0" "$tmpdir/$1" + fi +} +do_fua() { + case ,$4, in + *,fua,*) + if test -f "$tmpdir/strictfua"; then + dd of="$tmpdir/0" if="$tmpdir/$1" skip=$3 seek=$3 count=$2 \ + conv=notrunc iflag=count_bytes,skip_bytes oflag=seek_bytes + else + do_flush $1 + fi ;; + esac +} +do_flush() { + if test -f "$tmpdir/$1-replay"; then + while read cnt off; do + dd of="$tmpdir/0" if="$tmpdir/$1" skip=$off seek=$off count=$cnt \ + conv=notrunc iflag=count_bytes,skip_bytes oflag=seek_bytes + done < "$tmpdir/$1-replay" + fi + rm -f "$tmpdir/$1" "$tmpdir/$1-replay" +} +case "$1" in + config) + case $2 in + strictfua) + case $3 in + true | on | 1) touch "$tmpdir/strictfua" ;; + false | off | 0) ;; + *) echo "unknown value for strictfua $3" >&2; exit 1 ;; + esac ;; + *) echo "unknown config key $2" >&2; exit 1 ;; + esac + ;; + get_ready) + printf "%-32s" 'Initial contents' > "$tmpdir/0" + echo 0 > "$tmpdir/counter" + ;; + get_size) + echo 32 + ;; + can_write | can_zero | can_trim | can_flush) + exit 0 + ;; + can_fua | can_cache) + echo native + ;; + open) + read i < "$tmpdir/counter" + echo $((i+1)) | tee "$tmpdir/counter" + ;; + pread) + fill_cache $2 + dd if="$tmpdir/$2" skip=$4 count=$3 iflag=count_bytes,skip_bytes + ;; + cache) + fill_cache $2 + ;; + pwrite) + fill_cache $2 + dd of="$tmpdir/$2" seek=$4 conv=notrunc oflag=seek_bytes + echo $3 $4 >> "$tmpdir/$2-replay" + do_fua $2 $3 $4 $5 + ;; + zero | trim) + fill_cache $2 + dd of="$tmpdir/$2" if="/dev/zero" count=$3 seek=$4 conv=notrunc\ + oflag=seek_bytes iflag=count_bytes + echo $3 $4 >> "$tmpdir/$2-replay" + do_fua $2 $3 $4 $5 + ;; + flush) + do_flush $2 + ;; + *) + exit 2 + ;; +esac diff --git a/tests/test-multi-conn.sh b/tests/test-multi-conn.sh new file mode 100755 index 00000000..8580608e --- /dev/null +++ b/tests/test-multi-conn.sh @@ -0,0 +1,293 @@ +#!/usr/bin/env bash +# nbdkit +# Copyright (C) 2018-2021 Red Hat Inc. +# +# 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. + +# Demonstrate various multi-conn filter behaviors. + +source ./functions.sh +set -e +set -x + +requires_plugin sh +requires_nbdsh_uri +requires dd iflag=count_bytes </dev/null + +files="test-multi-conn.out test-multi-conn.stat" +rm -f $files +cleanup_fn rm -f $files + +fail=0 +export handles preamble uri +uri= # will be set by --run later +handles=3 +preamble=' +import os + +uri = os.environ["uri"] +handles = int(os.environ["handles"]) +h = [] +for i in range(handles): + h.append(nbd.NBD()) + h[i].connect_uri(uri) +print(h[0].can_multi_conn()) +' + +# Demonstrate the caching present without use of filter +for filter in '' '--filter=multi-conn multi-conn-mode=plugin'; do + nbdkit -vf -U - sh test-multi-conn-plugin.sh $filter \ + --run 'handles=4 nbdsh -c "$preamble" -c " +# Without flush, reads cache, and writes do not affect persistent data +print(h[0].pread(4, 0)) +h[1].pwrite(b'\''next '\'', 0) +print(h[0].pread(4, 0)) +print(h[1].pread(4, 0)) +print(h[2].pread(4, 0)) +# Flushing an unrelated connection does not make writes persistent +h[2].flush() +print(h[0].pread(4, 0)) +print(h[1].pread(4, 0)) +print(h[2].pread(4, 0)) +# After write is flushed, only connections without cache see new data +h[1].flush() +print(h[0].pread(4, 0)) +print(h[1].pread(4, 0)) +print(h[2].pread(4, 0)) +print(h[3].pread(4, 0)) +# Flushing before reads clears the cache +h[0].flush() +h[2].flush() +print(h[0].pread(4, 0)) +print(h[2].pread(4, 0)) +"' > test-multi-conn.out || fail=1 + diff -u <(cat <<\EOF +False +b'Init' +b'Init' +b'next' +b'Init' +b'Init' +b'next' +b'Init' +b'Init' +b'next' +b'Init' +b'next' +b'next' +b'next' +EOF + ) test-multi-conn.out || fail=1 +done + +# Demonstrate specifics of FUA flag +for filter in '' '--filter=multi-conn multi-conn-mode=plugin'; do + nbdkit -vf -U - sh test-multi-conn-plugin.sh $filter \ + --run 'nbdsh -c "$preamble" -c " +# Some servers let FUA flush all outstanding requests +h[0].pwrite(b'\''hello '\'', 0) +h[0].pwrite(b'\''world.'\'', 6, nbd.CMD_FLAG_FUA) +print(h[1].pread(12, 0)) +"' > test-multi-conn.out || fail=1 + diff -u <(cat <<\EOF +False +b'hello world.' +EOF + ) test-multi-conn.out || fail=1 +done +for filter in '' '--filter=multi-conn multi-conn-mode=plugin'; do + nbdkit -vf -U - sh test-multi-conn-plugin.sh strictfua=1 $filter \ + --run 'nbdsh -c "$preamble" -c " +# But it is also compliant for a server that only flushes the exact request +h[0].pwrite(b'\''hello '\'', 0) +h[0].pwrite(b'\''world.'\'', 6, nbd.CMD_FLAG_FUA) +print(h[1].pread(12, 0)) +# Without multi-conn, data flushed in one connection can later be reverted +# by a flush of earlier data in another connection +h[1].pwrite(b'\''H'\'', 0, nbd.CMD_FLAG_FUA) +h[2].flush() +print(h[2].pread(12, 0)) +h[0].flush() +h[2].flush() +print(h[2].pread(12, 0)) +h[1].flush() +h[2].flush() +print(h[2].pread(12, 0)) +"' > test-multi-conn.out || fail=1 + diff -u <(cat <<\EOF +False +b'Initiaworld.' +b'Hnitiaworld.' +b'hello world.' +b'Hello world.' +EOF + ) test-multi-conn.out || fail=1 +done + +# Demonstrate multi-conn effects. The cache filter in writeback +# mode is also able to supply multi-conn by a different technique. +for filter in '--filter=multi-conn' 'strictfua=1 --filter=multi-conn' \ + '--filter=multi-conn multi-conn-mode=plugin --filter=cache' ; do + nbdkit -vf -U - sh test-multi-conn-plugin.sh $filter \ + --run 'nbdsh -c "$preamble" -c " +# FUA writes are immediately visible on all connections +h[0].cache(12, 0) +h[1].pwrite(b'\''Hello '\'', 0, nbd.CMD_FLAG_FUA) +print(h[0].pread(12, 0)) +# A flush on an unrelated connection makes all other connections consistent +h[1].pwrite(b'\''world.'\'', 6) +h[2].flush() +print(h[0].pread(12, 0)) +"' > test-multi-conn.out || fail=1 + diff -u <(cat <<\EOF +True +b'Hello l cont' +b'Hello world.' +EOF + ) test-multi-conn.out || fail=1 +done + +# unsafe mode intentionally lacks consistency, use at your own risk +nbdkit -vf -U - sh test-multi-conn-plugin.sh \ + --filter=multi-conn multi-conn-mode=unsafe \ + --run 'nbdsh -c "$preamble" -c " +h[0].cache(12, 0) +h[1].pwrite(b'\''Hello '\'', 0, nbd.CMD_FLAG_FUA) +print(h[0].pread(12, 0)) +h[1].pwrite(b'\''world.'\'', 6) +h[2].flush() +print(h[0].pread(12, 0)) +"' > test-multi-conn.out || fail=1 +diff -u <(cat <<\EOF +True +b'Initial cont' +b'Initial cont' +EOF + ) test-multi-conn.out || fail=1 + +# auto mode devolves to multi-conn disable when connections are serialized +nbdkit -vf -U - sh test-multi-conn-plugin.sh --filter=noparallel \ + serialize=connections --filter=multi-conn --filter=cache \ + --run 'handles=1 nbdsh -c "$preamble" +' > test-multi-conn.out || fail=1 +diff -u <(cat <<\EOF +False +EOF + ) test-multi-conn.out || fail=1 + +# Use --filter=stats to show track-dirty effects +for level in off connection fast; do + for mode in emulate 'emulate --filter=cache' \ + plugin 'plugin --filter=cache'; do + echo "setup: $level $mode" >> test-multi-conn.stat + # Flush with no activity + nbdkit -vf -U - sh test-multi-conn-plugin.sh --filter=multi-conn \ + --filter=stats statsfile=test-multi-conn.stat statsappend=true \ + multi-conn-track-dirty=$level multi-conn-mode=$mode \ + --run 'nbdsh -c "$preamble" -c " +h[0].flush() +h[0].pread(1, 0) +h[0].flush() +"' > test-multi-conn.out || fail=1 + # Client that flushes assuming multi-conn semantics + nbdkit -vf -U - sh test-multi-conn-plugin.sh --filter=multi-conn \ + --filter=stats statsfile=test-multi-conn.stat statsappend=true \ + multi-conn-track-dirty=$level multi-conn-mode=$mode \ + --run 'handles=4 nbdsh -c "$preamble" -c " +h[0].pread(1, 0) +h[1].zero(1, 0) +h[3].flush() +h[2].zero(1, 1) +h[0].pread(1, 0) +h[3].flush() +h[3].flush() +"' > test-multi-conn.out || fail=1 + # Client that flushes assuming inconsistent semantics + nbdkit -vf -U - sh test-multi-conn-plugin.sh --filter=multi-conn \ + --filter=stats statsfile=test-multi-conn.stat statsappend=true \ + multi-conn-track-dirty=$level multi-conn-mode=$mode \ + --run 'nbdsh -c "$preamble" -c " +h[0].pread(1, 0) +h[1].trim(1, 0) +h[0].flush() +h[1].flush() +h[0].pread(1, 0) +h[2].trim(1, 1) +h[0].flush() +h[2].flush() +"' > test-multi-conn.out || fail=1 + done +done +cat test-multi-conn.stat +diff -u <(cat <<\EOF +setup: off emulate +flush: 6 ops +flush: 12 ops +flush: 12 ops +setup: off emulate --filter=cache +flush: 6 ops +flush: 12 ops +flush: 12 ops +setup: off plugin +flush: 2 ops +flush: 3 ops +flush: 4 ops +setup: off plugin --filter=cache +flush: 2 ops +flush: 3 ops +flush: 4 ops +setup: connection emulate +flush: 4 ops +flush: 4 ops +setup: connection emulate --filter=cache +flush: 4 ops +flush: 4 ops +setup: connection plugin +flush: 3 ops +flush: 4 ops +setup: connection plugin --filter=cache +flush: 2 ops +flush: 2 ops +setup: fast emulate +flush: 8 ops +flush: 6 ops +setup: fast emulate --filter=cache +flush: 8 ops +flush: 6 ops +setup: fast plugin +flush: 2 ops +flush: 2 ops +setup: fast plugin --filter=cache +flush: 2 ops +flush: 2 ops +EOF + ) <(sed -n 's/\(flush:.*ops\).*/\1/p; /^setup:/p' \ + test-multi-conn.stat) || fail=1 + +exit $fail diff --git a/TODO b/TODO index 2bde2478..6857484a 100644 --- a/TODO +++ b/TODO @@ -210,13 +210,6 @@ Suggestions for filters * masking plugin features for testing clients (see 'nozero' and 'fua' filters for examples) -* multi-conn filter to adjust advertisement of multi-conn bit. In - particular, if the plugin lacks .can_multi_conn, then .open/.close - track all open connections, and .flush and FUA flag will call - next_ops->flush() on all of them. Conversely, if plugin supports - multi-conn, we can cache whether the image is dirty, and avoid - expense of next_ops->flush when it is clean. - * "bandwidth quota" filter which would close a connection after it exceeded a certain amount of bandwidth up or down. -- 2.30.1
Richard W.M. Jones
2021-Mar-09 14:59 UTC
[Libguestfs] [nbdkit PATCH v3 15/16] multi-conn: New filter
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