Re: [Libguestfs] [nbdkit PATCH 4/4] blocksize-policy: Add blocksize-write-disconnect=N parameter

Allow this filter to emulate qemu's behavior of a hard disconnect on write attempts larger than 32M. --- .../nbdkit-blocksize-policy-filter.pod | 21 ++++ tests/Makefile.am | 12 +- filters/blocksize-policy/policy.c | 27 ++++- tests/test-blocksize-write-disconnect.sh | 107 ++++++++++++++++++ 4 files changed, 164 insertions(+), 3 deletions(-) create mode 100755 tests/test-blocksize-write-disconnect.sh diff --git a/filters/blocksize-policy/nbdkit-blocksize-policy-filter.pod b/filters/blocksize-policy/nbdkit-blocksize-policy-filter.pod index 691ad289..a377829f 100644 --- a/filters/blocksize-policy/nbdkit-blocksize-policy-filter.pod +++ b/filters/blocksize-policy/nbdkit-blocksize-policy-filter.pod @@ -10,6 +10,7 @@ maximum block size, and apply error policy [blocksize-minimum=N] [blocksize-preferred=N] [blocksize-maximum=N] + [blocksize-write-disconnect=N] =head1 DESCRIPTION @@ -49,6 +50,13 @@ read-modify-write for an unaligned write). With this filter you can use C<blocksize-error-policy=error> to reject these requests in the filter with an EINVAL error. The plugin will not see them. +Normally, nbdkit will accept write requests up to 64M in length, and +reply with a gracful error message rather than a hard disconnect for a +buffer up to twice that large. But many other servers (for example, +qemu-nbd) will give a hard disconnect for a write request larger than +32M. With this filter you can use C<blocksize-write-disconnect=32M> +to emulate the behavior of other servers. + =head2 Combining with L<nbdkit-blocksize-filter(1)> A related filter is L<nbdkit-blocksize-filter(1)>. That filter can @@ -87,6 +95,19 @@ means pass the request through to the plugin. Use C<error> to return an EINVAL error back to the client. The plugin will not see the badly formed request in this case. +=item B<blocksize-write-disconnect=>N + +(nbdkit E<ge> 1.34) + +If a client sends a write request which is larger than the specified +I<size> (using the usual size modifiers like C<32M>), abruptly close +the connection. This can be used to emulate qemu's behavior of +disconnecting for write requests larger than 32M, rather than nbdkit's +default of keeping the connection alive for write requests up to 128M +(although nbdkit does not let the plugin see requests larger than +64M). The write disconnect size is independent of any advertised +maximum block size or its accompanying error policy. + =item B<blocksize-minimum=>N =item B<blocksize-preferred=>N diff --git a/tests/Makefile.am b/tests/Makefile.am index e951381d..d59b797c 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -1464,8 +1464,16 @@ EXTRA_DIST += \ $(NULL) # blocksize-policy filter test. -TESTS += test-blocksize-policy.sh test-blocksize-error-policy.sh -EXTRA_DIST += test-blocksize-policy.sh test-blocksize-error-policy.sh +TESTS += \ + test-blocksize-policy.sh \ + test-blocksize-error-policy.sh \ + test-blocksize-write-disconnect.sh \ + $(NULL) +EXTRA_DIST += \ + test-blocksize-policy.sh \ + test-blocksize-error-policy.sh \ + test-blocksize-write-disconnect.sh \ + $(NULL) # cache filter test. TESTS += \ diff --git a/filters/blocksize-policy/policy.c b/filters/blocksize-policy/policy.c index 4ec07d36..f7cff2f1 100644 --- a/filters/blocksize-policy/policy.c +++ b/filters/blocksize-policy/policy.c @@ -42,11 +42,13 @@ #include <nbdkit-filter.h> #include "ispowerof2.h" +#include "rounding.h" /* Block size constraints configured on the command line (0 = unset). */ static uint32_t config_minimum; static uint32_t config_preferred; static uint32_t config_maximum; +static uint32_t config_disconnect; /* Error policy. */ static enum { EP_ALLOW, EP_ERROR } error_policy = EP_ALLOW; @@ -90,6 +92,12 @@ policy_config (nbdkit_next_config *next, nbdkit_backend *nxdata, config_maximum = r; return 0; } + else if (strcmp (key, "blocksize-write-disconnect") == 0) { + r = nbdkit_parse_size (value); + if (r == -1 || r > UINT32_MAX) goto parse_error; + config_disconnect = r; + return 0; + } return next (nxdata, key, value); } @@ -147,6 +155,14 @@ policy_config_complete (nbdkit_next_config_complete *next, } } + if (config_minimum && config_disconnect) { + if (config_disconnect <= config_minimum) { + nbdkit_error ("blocksize-write-disonnect must be larger than " + "blocksize-minimum"); + return -1; + } + } + return next (nxdata); } @@ -192,6 +208,8 @@ policy_block_size (nbdkit_next *next, void *handle, if (config_maximum) *maximum = config_maximum; + else if (config_disconnect) + *maximum = ROUND_DOWN (config_disconnect, *minimum); else *maximum = 0xffffffff; } @@ -220,7 +238,7 @@ policy_block_size (nbdkit_next *next, void *handle, * below. * * The 'data' flag is true for pread and pwrite (where we check the - * maximum bound). We don't check maximum for non-data- carrying + * maximum bound). We don't check maximum for non-data-carrying * calls like zero. * * The NBD specification mandates EINVAL for block size constraint @@ -303,6 +321,13 @@ policy_pwrite (nbdkit_next *next, void *handle, const void *buf, uint32_t count, uint64_t offset, uint32_t flags, int *err) { + if (config_disconnect && count > config_disconnect) { + nbdkit_error ("disconnecting client due to oversize write request"); + nbdkit_disconnect (true); + *err = ESHUTDOWN; + return -1; + } + if (check_policy (next, handle, "pwrite", true, count, offset, err) == -1) return -1; diff --git a/tests/test-blocksize-write-disconnect.sh b/tests/test-blocksize-write-disconnect.sh new file mode 100755 index 00000000..14a35100 --- /dev/null +++ b/tests/test-blocksize-write-disconnect.sh @@ -0,0 +1,107 @@ +#!/usr/bin/env bash +# nbdkit +# Copyright (C) 2019-2022 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. + +source ./functions.sh +set -e +set -x + +requires_plugin eval +requires nbdsh -c 'print(h.get_block_size)' +requires nbdsh -c 'print(h.get_strict_mode)' +requires_nbdsh_uri +requires dd iflag=count_bytes </dev/null + +# Libnbd does not let us test pwrite larger than 64M, so we can't +# test nbdkit's graceful behavior of writes up to 128M. +# In this test, odd size writes fail with EINVAL from the filter (size 1 too +# small, all others unaligned); evens 2 to 8M pass, 8M+2 to 16M fail with +# ENOMEM from the plugin, 16M+2 to 32M fail with EINVAL from the filter, +# 32M+1 to 64M kill the connection (ENOTCONN visible to client), and +# 64M+1 and above fails with ERANGE in libnbd. + +nbdkit -v -U - eval \ + block_size="echo 2 4096 16M" \ + get_size="echo 64M" \ + pread=' dd if=/dev/zero count=$3 iflag=count_bytes ' \ + pwrite=' if test $3 -gt $((8*1024*1024)); then + echo ENOMEM >&2; exit 1 + else + dd of=/dev/null + fi' \ + --filter=blocksize-policy \ + blocksize-error-policy=error blocksize-write-disconnect=32M \ + --run ' +nbdsh -u "$uri" -c " +import errno + +def check(h, size, expect_value, expect_traffic=True): + assert h.aio_is_ready() is True + buf = b\"0\" * size + if hasattr(h, \"stats_bytes_sent\"): + start = h.stats_bytes_sent() + try: + h.pwrite(buf, 0) + assert expect_value == 0 + except nbd.Error as ex: + assert expect_value == ex.errnum + if hasattr(h, \"stats_bytes_sent\"): + if expect_traffic: + assert h.stats_bytes_sent() > start + else: + assert h.stats_bytes_sent() == start + +h.set_strict_mode(0) # Bypass client-side safety checks +# Beyond 64M +check(h, 64*1024*1024 + 1, errno.ERANGE, False) +check(h, 64*1024*1024 + 2, errno.ERANGE, False) +# Small reads +check(h, 1, errno.EINVAL) +check(h, 2, 0) +# Near 8M boundary +check(h, 8*1024*1024 - 2, 0) +check(h, 8*1024*1024 - 1, errno.EINVAL) +check(h, 8*1024*1024, 0) +check(h, 8*1024*1024 + 1, errno.EINVAL) +check(h, 8*1024*1024 + 2, errno.ENOMEM) +# Near 16M boundary +check(h, 16*1024*1024 - 2, errno.ENOMEM) +check(h, 16*1024*1024 - 1, errno.EINVAL) +check(h, 16*1024*1024, errno.ENOMEM) +check(h, 16*1024*1024 + 1, errno.EINVAL) +check(h, 16*1024*1024 + 2, errno.EINVAL) +# Near 32M boundary +check(h, 32*1024*1024 - 2, errno.EINVAL) +check(h, 32*1024*1024 - 1, errno.EINVAL) +check(h, 32*1024*1024, errno.EINVAL) +check(h, 32*1024*1024 + 1, errno.ENOTCONN) +assert h.aio_is_ready() is False +"' --