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

Richard W.M. Jones rjones at redhat.com
Thu Oct 27 08:55:19 UTC 2022


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


More information about the Libguestfs mailing list