Eric Blake
2022-Oct-26 22:18 UTC
[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 +"' -- 2.37.3
Richard W.M. Jones
2022-Oct-27 08:55 UTC
[Libguestfs] [nbdkit PATCH 4/4] blocksize-policy: Add blocksize-write-disconnect=N parameter
On Wed, Oct 26, 2022 at 05:18:02PM -0500, Eric Blake wrote:> 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 > +"' > --Reviewed-by: Richard W.M. Jones <rjones at redhat.com> nbdkit-sh-plugin patches to follow? Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com nbdkit - Flexible, fast NBD server with plugins https://gitlab.com/nbdkit/nbdkit