Re: [Qemu-devel] [PATCH v2 1/3] iotests: Add 241 to test NBD on unaligned images

[Vladimir Sementsov-Ogievskiy]

28.03.2019 1:39, 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 for the new
> 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.
> 
> 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>
> ---
>   tests/qemu-iotests/241     | 71 ++++++++++++++++++++++++++++++++++++++
>   tests/qemu-iotests/241.out |  7 ++++
>   tests/qemu-iotests/group   |  1 +
>   3 files changed, 79 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..b1f6e6452de
> --- /dev/null
> +++ b/tests/qemu-iotests/241
> @@ -0,0 +1,71 @@
> +#!/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
> +rm -f "${TEST_DIR}/qemu-nbd.pid"

hmm, strange that we need to remove something from test directory at start.

> +
> +_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
> +
> +echo
> +echo "=== Exporting unaligned raw image ==="
> +echo
> +
> +# 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).
> +printf %01000d 0 > "$TEST_IMG_FILE"
> +nbd_server_start_unix_socket -f $IMGFMT -e 42 -x '' "$TEST_IMG_FILE"

why do we need -e 42? we don't have more than one client simultaneously..
and -x '' is the default anyway

> +TEST_IMG="nbd:unix:$nbd_unix_socket"
> +
> +$QEMU_IMG map --output=json "$TEST_IMG" | _filter_qemu_img_map

is filter needed? seems like not

> +$QEMU_IO -c map "$TEST_IMG"
> +
> +# 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.
> +# 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..044afc0c6f8
> --- /dev/null
> +++ b/tests/qemu-iotests/241.out
> @@ -0,0 +1,7 @@
> +QA output created by 241
> +
> +=== Exporting unaligned raw image ===
> +
> +[{ "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
> 


-- 
Best regards,
Vladimir