Richard W.M. Jones
2022-Feb-16 16:20 UTC
[Libguestfs] [PATCH nbdkit 5/6] tests: Add a simple test for block size constraints
--- tests/Makefile.am | 4 +++ tests/test-block-size-constraints.sh | 50 ++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/tests/Makefile.am b/tests/Makefile.am index 16f76eb8..e888b1a0 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -562,6 +562,10 @@ EXTRA_DIST += test-eflags.sh TESTS += test-export-name.sh test-export-info.sh EXTRA_DIST += test-export-name.sh test-export-info.sh +# Test block size constraints. +TESTS += test-block-size-constraints.sh +EXTRA_DIST += test-block-size-constraints.sh + # cdi plugin test. TESTS += test-cdi.sh EXTRA_DIST += test-cdi.sh diff --git a/tests/test-block-size-constraints.sh b/tests/test-block-size-constraints.sh new file mode 100755 index 00000000..658445cd --- /dev/null +++ b/tests/test-block-size-constraints.sh @@ -0,0 +1,50 @@ +#!/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)' + +# Create an nbdkit eval plugin which presents block size constraints. +# Check the advertised block size constraints can be read. +nbdkit -U - eval \ + block_size="echo 64K 128K 32M" \ + get_size="echo 0" \ + --run 'nbdsh \ + -u "$uri" \ + -c "assert h.get_block_size(nbd.SIZE_MINIMUM) == 64 * 1024" \ + -c "assert h.get_block_size(nbd.SIZE_PREFERRED) == 128 * 1024" \ + -c "assert h.get_block_size(nbd.SIZE_MAXIMUM) == 32 * 1024 * 1024" \ + ' -- 2.35.1
Eric Blake
2022-Feb-16 18:36 UTC
[Libguestfs] [PATCH nbdkit 5/6] tests: Add a simple test for block size constraints
On Wed, Feb 16, 2022 at 04:20:40PM +0000, Richard W.M. Jones wrote:> --- > tests/Makefile.am | 4 +++ > tests/test-block-size-constraints.sh | 50 ++++++++++++++++++++++++++++ > 2 files changed, 54 insertions(+)ACK. But food for thought:> +requires_plugin eval > +requires nbdsh -c 'print(h.get_block_size)' > + > +# Create an nbdkit eval plugin which presents block size constraints. > +# Check the advertised block size constraints can be read. > +nbdkit -U - eval \ > + block_size="echo 64K 128K 32M" \ > + get_size="echo 0" \ > + --run 'nbdsh \ > + -u "$uri" \ > + -c "assert h.get_block_size(nbd.SIZE_MINIMUM) == 64 * 1024" \ > + -c "assert h.get_block_size(nbd.SIZE_PREFERRED) == 128 * 1024" \ > + -c "assert h.get_block_size(nbd.SIZE_MAXIMUM) == 32 * 1024 * 1024" \The NBD protocol says that if a client requests block size, it should abide by the constraints (even though we have proven that not all clients do); but if a client does NOT request block size, the server can still advertise sizes but must be prepared for clients to ignore it. When I first tweaked libnbd to unconditionally ask for block size as a client, I did not consider the subtle difference this presents, but at least qemu-nbd has code where it will advertise different sizes based on whether the client requested size vs. did not. So we probably need to add a knob to libnbd to control whether it will request sizes, at which point this test in nbdkit will need to ensure the libnbd knob is in the right position (I haven't yet decided whether it makes more sense to request block sizes by default with the knob allowing a client to not request, or to not request by default such that the explicit request means you are writing a client that intends to comply). But now that I've written that, it also means that if the plugin provides .block_size, we may want to advertise those values even to clients that did not request the info. Furthermore, we may need a boolean that we pass _in_ to .block_size that says whether we are grabbing the information in a non-binding manner (the client didn't request size), or in a strict manner (the client requested size, and promises to obey it). That is, a signature of int (*block_size) (struct context *, bool strict, uint32_t *minimum, uint32_t *preferred, uint32_t *maximum); where strict corresponds to whether the client requested NBD_INFO_BLOCK_SIZE. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org