[Libguestfs] [nbdkit PATCH] server: Add --no-meta-contexts option

It is nice to be able to configure nbdkit to behave like other common servers to ease interoperability testing. With nbd-server 3.24, there was no support for structured replies, so --no-sr was good enough; but nbd-server 3.25 learned structured replies for the sake of sparse reads while still leaving meta contexts unsupported. The existing noextents filter is insufficient to emulate this behavior of nbd-server, since nbdkit has defaulted to supplying its own all-data "base:allocation" context when the plugin does not support extents. Note that libnbd 1.15.3 through current 1.18.2 cause nbd_can_meta_context() to fail with EINVAL when the server supports structured replies but lacks meta contexts; the manual testing via a one-off hack to nbdkit, as documented in libnbd commit 55b09667, did not expose this corner case. While the failure was a desirable change in light of the API addition of nbd_opt_set_meta_context(), it was an unintended regression when paired with nbd-server and no explicit control over the handshake process. The test case added here works whether or not that bug in libnbd has been fixed. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- Counterpart patch matching message to my recent libnbd series (and now that I fixed git to send to the new list address instead of the old, it's not getting lost in the ether...) docs/nbdkit-protocol.pod | 76 +++++++++---- docs/nbdkit.pod | 12 ++ docs/synopsis.txt | 2 +- filters/noextents/nbdkit-noextents-filter.pod | 8 +- tests/Makefile.am | 2 + server/internal.h | 1 + server/options.h | 3 + server/main.c | 5 + server/protocol-handshake-newstyle.c | 6 + tests/test-crippled-extents.sh | 107 ++++++++++++++++++ 10 files changed, 192 insertions(+), 30 deletions(-) create mode 100755 tests/test-crippled-extents.sh diff --git a/docs/nbdkit-protocol.pod b/docs/nbdkit-protocol.pod index dabdf23c..74fe4576 100644 --- a/docs/nbdkit-protocol.pod +++ b/docs/nbdkit-protocol.pod @@ -4,8 +4,8 @@ nbdkit-protocol - which parts of the NBD protocol nbdkit supports =head1 SYNOPSIS - nbdkit [-n|--newstyle] [--mask-handshake MASK] [--no-sr] [-o|--oldstyle] - [...] + nbdkit [-n|--newstyle] [--mask-handshake MASK] [--no-meta-contexts] + [--no-sr] [-o|--oldstyle] [...] =head1 DESCRIPTION @@ -19,32 +19,49 @@ The NBD protocol comes in two incompatible forms that we call use depends on the client and cannot be known in advance, nor can it be negotiated from the server side. -nbdkit defaults to the newstyle protocol since nbdkit E<ge> 1.3. The -newstyle protocol is better in every respect than the oldstyle -protocol and you should prefer it if possible. The newstyle protocol -also includes an extension where a client may request structured -replies for even more capabilities, such as sparse reads or obtaining -block status. By default, nbdkit advertises as many features as it -can support (in some cases, this can be limited by what callbacks the -plugin handles), even if the client does not negotiate to use all -advertised features. +nbdkit defaults to the newstyle protocol since nbdkit E<ge> 1.3 (the +command line flag I<-n> or I<--newstyle> is ignored for backwards +compatibility with older versions). The newstyle protocol is better +in every respect than the oldstyle protocol and you should prefer it +if possible. The newstyle protocol also includes extensions where a +client may request structured replies for even more capabilities, such +as sparse reads or meta contexts for obtaining block status. By +default, nbdkit advertises as many features as it can support (in some +cases, this can be limited by what callbacks the plugin handles), even +if the client does not negotiate to use all advertised features. Nbdkit also includes some options that are useful mainly when performing integration tests, for proving whether clients have sane fallback behavior when dealing various older servers permitted by the -NBD protocol. Use the I<--no-sr> flag to force the newstyle protocol -to decline any client request for structured replies. Use the -I<--mask-handshake> parameter to mask off particular global features -which are advertised during new-style handshake (defaulting to all -supported bits set). Clearing bit 0 (the low order bit) limits a -client to using just C<NBD_OPT_EXPORT_NAME> (and is incompatible with -TLS or structured replies); clearing bit 1 causes the handshake to -send more padding bytes in response to C<NBD_OPT_EXPORT_NAME>. Other -bits in the mask will only have an effect if the NBD protocol is +NBD protocol. The following options intentionally disable optional +parts of the NBD protocol, with successively larger impacts: + +Use the I<--no-meta-contexts> flag to force the newstyle protocol to +treat all requests for meta context negotiation from the client as +unsupported; the client will be unable to query block status. By +default, nbdkit gracefully handles all meta context requests, even +though it currently supports only the C<base:allocation> context +(possibly by it synthesizing a context that represents an all-data +disk when the plugin lacks support for extents); but treating meta +contexts as unsupported is useful for emulating nbd-server 3.25. + +Use the I<--no-sr> flag to force the newstyle protocol to decline any +client request for structured replies; this is stronger than +I<--no-meta-contexts> in that it also disables the opportunity for +sparse reads. This is useful for emulating nbd-server 3.24. + +Use the I<--mask-handshake> parameter to mask off particular global +features which are advertised during new-style handshake (defaulting +to all supported bits set). Clearing bit 0 (the low order bit) limits +a client to using just C<NBD_OPT_EXPORT_NAME> (and is incompatible +with TLS or structured replies); clearing bit 1 causes the handshake +to send more padding bytes in response to C<NBD_OPT_EXPORT_NAME>. +Other bits in the mask will only have an effect if the NBD protocol is extended in the future to define other global bits. -Use the I<-o> or I<--oldstyle> flag to force the oldstyle protocol. -In this mode, I<--no-sr> and I<--mask-handshake> have no effect. +Finally, use the I<-o> or I<--oldstyle> flag to force the oldstyle +protocol. In this mode, I<--no-meta-contexts>, I<--no-sr> and +I<--mask-handshake> have no effect. =head2 Common clients and the protocol they require @@ -153,14 +170,19 @@ However we don’t expose the capability to send structured replies to plugins yet, nor do we send human-readable error messages using this facility. -In nbdkit E<ge> 1.13.9, the command-line option I<--no-sr> can be -used to disable server support for structured replies, for testing -client fallbacks. +In nbdkit E<ge> 1.13.9, the command-line option I<--no-sr> can be used +to disable server support for structured replies, for testing client +fallbacks; disabling structured replies also disables features like +block status queries that depend on structured replies. =item Metadata Querying Supported in nbdkit E<ge> 1.11.8. +In nbdkit E<ge> 1.37.9, the command-line option I<--no-meta-contexts> +can be used to disable server support for meta contexts, for testing +client fallbacks. + =item Block Status Supported in nbdkit E<ge> 1.11.10. @@ -218,6 +240,10 @@ Supported in nbdkit E<ge> 1.30. I<Not supported>. +=item Extended Headers Extension + +I<Not supported>. + =back =head1 SEE ALSO diff --git a/docs/nbdkit.pod b/docs/nbdkit.pod index 12488a40..d24b58e2 100644 --- a/docs/nbdkit.pod +++ b/docs/nbdkit.pod @@ -318,6 +318,18 @@ Use the newstyle NBD protocol. This is the default in nbdkit E<ge> 1.3. In earlier versions the default was oldstyle. See L<nbdkit-protocol(1)>. +=item B<--no-meta-contexts> + +Do not advertise meta contexts. Normally, nbdkit handles all client +requests for arbitrary meta context names, with the response being +affirmative for only the C<base:allocation> meta context (nbdkit +synthesizes this context even when the plugin does not support +extents), so that clients can use the block status command. However, +meta contexts are not a mandatory part of the newstyle NBD protocol, +so this option can be used to debug client fallbacks for dealing with +older servers that lack meta context support. See +L<nbdkit-protocol(1)>. + =item B<--no-sr> Do not advertise structured replies. A client must request structured diff --git a/docs/synopsis.txt b/docs/synopsis.txt index b1e154bb..4fbca826 100644 --- a/docs/synopsis.txt +++ b/docs/synopsis.txt @@ -4,7 +4,7 @@ nbdkit [-4|--ipv4-only] [-6|--ipv6-only] [--filter=FILTER ...] [-f|--foreground] [-g|--group GROUP] [-i|--ipaddr IPADDR] [--log=stderr|syslog|null] [--mask-handshake=MASK] - [-n|--newstyle] [--no-sr] [-o|--oldstyle] + [-n|--newstyle] [--no-meta-contexts] [--no-sr] [-o|--oldstyle] [-P|--pidfile PIDFILE] [-p|--port PORT] [-r|--readonly] [--run 'COMMAND ARGS ...'] [--selinux-label=LABEL] [-s|--single] [--swap] diff --git a/filters/noextents/nbdkit-noextents-filter.pod b/filters/noextents/nbdkit-noextents-filter.pod index 47daf5cb..3f0143ee 100644 --- a/filters/noextents/nbdkit-noextents-filter.pod +++ b/filters/noextents/nbdkit-noextents-filter.pod @@ -12,10 +12,10 @@ nbdkit-noextents-filter - disable extents in the underlying plugin client to detect sparse regions of the underlying disk. C<nbdkit-noextents-filter> disables this so that the plugin appears to be fully allocated, at least to a client that requests structured -replies. It is also possible to use the I<--no-sr> option to nbdkit -to prevent the client from using structured replies, which among its -other side effects will prevent the client from querying extents at -all. +replies. It is also possible to use the I<--no-meta-contexts> or +I<--no-sr> options to nbdkit to prevent the client from negotiating +extents at all, but that is accompanied by more side effects than this +filter. This filter can be useful when combined with L<nbdkit-file-plugin(1)> serving a file from a file system known to have poor C<lseek(2)> diff --git a/tests/Makefile.am b/tests/Makefile.am index 7087c663..fe1b7c36 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -263,6 +263,7 @@ TESTS += \ test-debug-flags.sh \ test-long-name.sh \ test-flush.sh \ + test-crippled-extents.sh \ test-swap.sh \ test-disconnect.sh \ test-disconnect-tls.sh \ @@ -292,6 +293,7 @@ EXTRA_DIST += \ test-dump-plugin-thread-model.sh \ test-dump-plugin.sh \ test-flush.sh \ + test-crippled-extents.sh \ test-foreground.sh \ test-help-example1.sh \ test-help-plugin.sh \ diff --git a/server/internal.h b/server/internal.h index 2f101ba4..3c5dfbdf 100644 --- a/server/internal.h +++ b/server/internal.h @@ -130,6 +130,7 @@ extern const char *ipaddr; extern enum log_to log_to; extern unsigned mask_handshake; extern bool newstyle; +extern bool no_mc; extern bool no_sr; extern const char *port; extern bool read_only; diff --git a/server/options.h b/server/options.h index aedaf1c1..544f8335 100644 --- a/server/options.h +++ b/server/options.h @@ -47,6 +47,7 @@ enum { LOG_OPTION, LONG_OPTIONS_OPTION, MASK_HANDSHAKE_OPTION, + NO_MC_OPTION, NO_SR_OPTION, RUN_OPTION, SELINUX_LABEL_OPTION, @@ -82,6 +83,8 @@ static const struct option long_options[] = { { "mask-handshake", required_argument, NULL, MASK_HANDSHAKE_OPTION }, { "new-style", no_argument, NULL, 'n' }, { "newstyle", no_argument, NULL, 'n' }, + { "no-mc", no_argument, NULL, NO_MC_OPTION }, + { "no-meta-contexts", no_argument, NULL, NO_MC_OPTION }, { "no-sr", no_argument, NULL, NO_SR_OPTION }, { "old-style", no_argument, NULL, 'o' }, { "oldstyle", no_argument, NULL, 'o' }, diff --git a/server/main.c b/server/main.c index 228aa8e6..0ca4ea84 100644 --- a/server/main.c +++ b/server/main.c @@ -101,6 +101,7 @@ const char *ipaddr; /* -i */ enum log_to log_to = LOG_TO_DEFAULT; /* --log */ unsigned mask_handshake = ~0U; /* --mask-handshake */ bool newstyle = true; /* false = -o, true = -n */ +bool no_mc; /* --no-meta-contexts */ bool no_sr; /* --no-sr */ char *pidfile; /* -P */ const char *port; /* -p */ @@ -459,6 +460,10 @@ main (int argc, char *argv[]) newstyle = true; break; + case NO_MC_OPTION: + no_mc = true; + break; + case NO_SR_OPTION: no_sr = true; break; diff --git a/server/protocol-handshake-newstyle.c b/server/protocol-handshake-newstyle.c index b70732b6..6b3bc76f 100644 --- a/server/protocol-handshake-newstyle.c +++ b/server/protocol-handshake-newstyle.c @@ -784,6 +784,12 @@ negotiate_handshake_newstyle_options (void) if (conn_recv_full (data, optlen, "read: %s: %m", optname) == -1) return -1; + if (no_mc) { + if (send_newstyle_option_reply (option, NBD_REP_ERR_UNSUP) == -1) + return -1; + continue; + } + /* Note that we support base:allocation whether or not the plugin * supports can_extents. */ diff --git a/tests/test-crippled-extents.sh b/tests/test-crippled-extents.sh new file mode 100755 index 00000000..117d938f --- /dev/null +++ b/tests/test-crippled-extents.sh @@ -0,0 +1,107 @@ +#!/usr/bin/env bash +# nbdkit +# Copyright Red Hat +# +# 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. + +source ./functions.sh +set -e +set -x + +fail=0 + +# Test various means for crippling extents, for interop testing. + +requires_run +requires_filter noextents +requires_nbdsh_uri + +fail=0 + +# By default, the memory plugin advertises extents, and starts with a hole +nbdkit memory 1M --run 'nbdsh --base -u "$uri" -c - <<\EOF +def f(context, offset, entries, err): + assert entries == [ 1024*1024, nbd.STATE_HOLE | nbd.STATE_ZERO] + +assert h.get_structured_replies_negotiated() is True +assert h.can_meta_context(nbd.CONTEXT_BASE_ALLOCATION) is True +h.block_status(1024*1024, 0, f) +EOF +' || fail=1 + +# Even when a plugin or filter sets can_extents false, nbdkit still +# synthesizes one that advertises all data +nbdkit --filter=noextents memory 1M --run 'nbdsh --base -u "$uri" -c - <<\EOF +def f(context, offset, entries, err): + assert entries == [ 1024*1024, 0] + +assert h.get_structured_replies_negotiated() is True +assert h.can_meta_context(nbd.CONTEXT_BASE_ALLOCATION) is True +h.block_status(1024*1024, 0, f) +EOF +' || fail=1 + +# Disabling meta contexts prevents extents, but still allows structured +# replies; libnbd 1.18 errors out on can_meta_context in that case, but +# other versions of libnbd gracefully treat it as false +nbdkit --no-meta-contexts memory 1M --run 'nbdsh --base -u "$uri" -c - <<\EOF +assert h.get_structured_replies_negotiated() is True +try: + ret = h.can_meta_context(nbd.CONTEXT_BASE_ALLOCATION) + assert ret is False +except nbd.Error: + print("treating libnbd error as meta context unsupported") +EOF +' || fail=1 + +# Disabling structured replies prevents extents as a side effect +nbdkit --no-sr memory 1M --run 'nbdsh --base -u "$uri" -c - <<\EOF +assert h.get_protocol() == "newstyle-fixed" +assert h.get_structured_replies_negotiated() is False +assert h.can_meta_context(nbd.CONTEXT_BASE_ALLOCATION) is False +EOF +' || fail=1 + +# Disabling fixed newstyle negotiation prevents structured replies +nbdkit --mask-handshake=0 memory 1M --run 'nbdsh --base -u "$uri" -c - <<\EOF +assert h.get_protocol() == "newstyle" +assert h.get_structured_replies_negotiated() is False +assert h.can_meta_context(nbd.CONTEXT_BASE_ALLOCATION) is False +EOF +' || fail=1 + +# Oldstyle protocol prevents structured replies +nbdkit --oldstyle memory 1M --run 'nbdsh --base -u "$uri" -c - <<\EOF +assert h.get_protocol() == "oldstyle" +assert h.get_structured_replies_negotiated() is False +assert h.can_meta_context(nbd.CONTEXT_BASE_ALLOCATION) is False +EOF +' || fail=1 + +exit $fail -- 2.43.2