Re: [Qemu-block] [PULL 07/14] iotests: Add 241 to test NBD on unaligned images

[Max Reitz]

On 01.04.19 16:08, Eric Blake wrote:
> Add a test for the NBD client workaround in the previous patch.  It's
> not really feasible for an iotest to assume a specific tracing engine,
> so we can't really probe trace_nbd_parse_blockstatus_compliance to see
> if the server was fixed vs. whether the client just worked around the
> server (other than by rearranging order between code patches and this
> test). But having a successful exchange sure beats the previous state
> of an error message. Since format probing can change alignment, we can
> use that as an easy way to test several configurations.
> 
> Not tested yet, but worth adding to this test in future patches: an
> NBD server that can advertise a non-sector-aligned size (such as
> nbdkit) causes qemu as the NBD client to misbehave when it rounds the
> size up and accesses beyond the advertised size. Qemu as NBD server
> never advertises a non-sector-aligned size (since bdrv_getlength()
> currently rounds up to sector boundaries); until qemu can act as such
> a server, testing that flaw will have to rely on external binaries.
> 
> Signed-off-by: Eric Blake <address@hidden>
> Message-Id: <address@hidden>
> Tested-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> [eblake: add forced-512 alignment, and nbdkit reproducer comment]
> ---
>  tests/qemu-iotests/241     | 100 +++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/241.out |  26 ++++++++++
>  tests/qemu-iotests/group   |   1 +
>  3 files changed, 127 insertions(+)
>  create mode 100755 tests/qemu-iotests/241
>  create mode 100644 tests/qemu-iotests/241.out
> 
> diff --git a/tests/qemu-iotests/241 b/tests/qemu-iotests/241
> new file mode 100755
> index 00000000000..4b196857387
> --- /dev/null
> +++ b/tests/qemu-iotests/241
> @@ -0,0 +1,100 @@
> +#!/bin/bash
> +#
> +# Test qemu-nbd vs. unaligned images
> +#
> +# Copyright (C) 2018-2019 Red Hat, Inc.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +#
> +
> +seq="$(basename $0)"
> +echo "QA output created by $seq"
> +
> +status=1 # failure is the default!
> +
> +nbd_unix_socket=$TEST_DIR/test_qemu_nbd_socket
> +
> +_cleanup()
> +{
> +    _cleanup_test_img
> +    nbd_server_stop
> +}
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +# get standard environment, filters and checks
> +. ./common.rc
> +. ./common.filter
> +. ./common.nbd
> +
> +_supported_fmt raw
> +_supported_proto nbd
> +_supported_os Linux
> +_require_command QEMU_NBD
> +
> +# can't use _make_test_img, because qemu-img rounds image size up,
> +# and because we want to use Unix socket rather than TCP port. Likewise,
> +# we have to redirect TEST_IMG to our server.
> +# This tests that we can deal with the hole at the end of an unaligned
> +# raw file (either because the server doesn't advertise alignment too
> +# large, or because the client ignores the server's noncompliance - even
> +# though we can't actually wire iotests into checking trace messages).
> +printf %01000d 0 > "$TEST_IMG_FILE"
> +TEST_IMG="nbd:unix:$nbd_unix_socket"
> +
> +echo
> +echo "=== Exporting unaligned raw image, natural alignment ==="
> +echo
> +
> +nbd_server_start_unix_socket -f $IMGFMT "$TEST_IMG_FILE"
> +
> +$QEMU_NBD_PROG --list -k $nbd_unix_socket | grep '\(size\|min\)'
> +$QEMU_IMG map -f raw --output=json "$TEST_IMG" | _filter_qemu_img_map
> +$QEMU_IO -f raw -c map "$TEST_IMG"
> +nbd_server_stop
> +
> +echo
> +echo "=== Exporting unaligned raw image, forced server sector alignment ==="
> +echo
> +
> +# Intentionally omit '-f' to force image probing, which in turn forces
> +# sector alignment, here at the server.
> +nbd_server_start_unix_socket "$TEST_IMG_FILE"
> +
> +$QEMU_NBD_PROG --list -k $nbd_unix_socket | grep '\(size\|min\)'
> +$QEMU_IMG map -f raw --output=json "$TEST_IMG" | _filter_qemu_img_map
> +$QEMU_IO -f raw -c map "$TEST_IMG"
> +nbd_server_stop
> +
> +echo
> +echo "=== Exporting unaligned raw image, forced client sector alignment ==="
> +echo
> +
> +# Now force sector alignment at the client.
> +nbd_server_start_unix_socket -f $IMGFMT "$TEST_IMG_FILE"
> +
> +$QEMU_NBD_PROG --list -k $nbd_unix_socket | grep '\(size\|min\)'
> +$QEMU_IMG map --output=json "$TEST_IMG" | _filter_qemu_img_map
> +$QEMU_IO -c map "$TEST_IMG"
> +nbd_server_stop
> +
> +# Not tested yet: we also want to ensure that qemu as NBD client does
> +# not access beyond the end of a server's advertised unaligned size:
> +#  nbdkit -U - memory size=513 --run 'qemu-io -f raw -c "r 512 512" $nbd'
> +# However, since qemu as server always rounds up to a sector alignment,
> +# we would have to use nbdkit to provoke the current client failures.
> +
> +# success, all done
> +echo '*** done'
> +rm -f $seq.full
> +status=0
> diff --git a/tests/qemu-iotests/241.out b/tests/qemu-iotests/241.out
> new file mode 100644
> index 00000000000..b76a6234d72
> --- /dev/null
> +++ b/tests/qemu-iotests/241.out
> @@ -0,0 +1,26 @@
> +QA output created by 241
> +
> +=== Exporting unaligned raw image, natural alignment ===
> +
> +  size:  1024
> +  min block: 512
> +[{ "start": 0, "length": 1024, "depth": 0, "zero": false, "data": true}]
> +1 KiB (0x400) bytes     allocated at offset 0 bytes (0x0)
> +
> +=== Exporting unaligned raw image, forced server sector alignment ===
> +
> +WARNING: Image format was not specified for 
> '/home/eblake/qemu/tests/qemu-iotests/scratch/t.raw' and probing guessed raw.
> +         Automatically detecting the format is dangerous for raw images, 
> write operations on block 0 will be restricted.
> +         Specify the 'raw' format explicitly to remove the restrictions.

Is there a patch for this already?

Also, I planned to reply to the original patch – but where is it?  The
latest version I can see on the list is v3, and that only has seven
lines in the reference output.  Well, the difference would explain why
Vladimir would give a Tested-by to a test that clearly will not run on
his machine.

Max

> +  size:  1024
> +  min block: 512
> +[{ "start": 0, "length": 1024, "depth": 0, "zero": false, "data": true}]
> +1 KiB (0x400) bytes     allocated at offset 0 bytes (0x0)
> +
> +=== Exporting unaligned raw image, forced client sector alignment ===
> +
> +  size:  1024
> +  min block: 512
> +[{ "start": 0, "length": 1024, "depth": 0, "zero": false, "data": true}]
> +1 KiB (0x400) bytes     allocated at offset 0 bytes (0x0)
> +*** done
> diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
> index 41da10c6cf5..bae77183809 100644
> --- a/tests/qemu-iotests/group
> +++ b/tests/qemu-iotests/group
> @@ -240,6 +240,7 @@
>  238 auto quick
>  239 rw auto quick
>  240 auto quick
> +241 rw auto quick
>  242 rw auto quick
>  243 rw auto quick
>  244 rw auto quick
>

signature.asc
Description: OpenPGP digital signature