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