From e9dce9cb6eae57834cd80324ff43069299198bab Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Thu, 28 Mar 2019 23:27:45 -0500 Subject: iotests: Add 241 to test NBD on unaligned images 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 Message-Id: <20190329042750.14704-2-eblake@redhat.com> Tested-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Vladimir Sementsov-Ogievskiy [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 (limited to 'tests') diff --git a/tests/qemu-iotests/241 b/tests/qemu-iotests/241 new file mode 100755 index 0000000..4b19685 --- /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 . +# + +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 0000000..b76a623 --- /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. + 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 41da10c..bae7718 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 -- cgit v1.1 From a62a85ef5ccd764d03d72d6c3cd558f9755b3457 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Sat, 30 Mar 2019 20:28:37 -0500 Subject: nbd/client: Report offsets in bdrv_block_status It is desirable for 'qemu-img map' to have the same output for a file whether it is served over file or nbd protocols. However, ever since we implemented block status for NBD (2.12), the NBD protocol forgot to inform the block layer that as the final layer in the chain, the offset is valid; without an offset, the human-readable form of qemu-img map gives up with the unhelpful: $ nbdkit -U - data data="1" size=512 --run 'qemu-img map $nbd' Offset Length Mapped to File qemu-img: File contains external, encrypted or compressed clusters. The --output=json form always works, because it is reporting the lower-level bdrv_block_status results directly rather than trying to filter out sparse ranges for human consumption - but now it also shows the offset member. With this patch, the human output changes to: Offset Length Mapped to File 0 0x200 0 nbd+unix://?socket=/tmp/nbdkitOxeoLa/socket This change is observable to several iotests. Fixes: 78a33ab5 Reported-by: Richard W.M. Jones Signed-off-by: Eric Blake Message-Id: <20190329042750.14704-4-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy --- tests/qemu-iotests/209.out | 4 ++-- tests/qemu-iotests/223.out | 18 +++++++++--------- tests/qemu-iotests/241.out | 6 +++--- 3 files changed, 14 insertions(+), 14 deletions(-) (limited to 'tests') diff --git a/tests/qemu-iotests/209.out b/tests/qemu-iotests/209.out index 0d29724..214e27b 100644 --- a/tests/qemu-iotests/209.out +++ b/tests/qemu-iotests/209.out @@ -1,2 +1,2 @@ -[{ "start": 0, "length": 524288, "depth": 0, "zero": false, "data": true}, -{ "start": 524288, "length": 524288, "depth": 0, "zero": true, "data": false}] +[{ "start": 0, "length": 524288, "depth": 0, "zero": false, "data": true, "offset": 0}, +{ "start": 524288, "length": 524288, "depth": 0, "zero": true, "data": false, "offset": 524288}] diff --git a/tests/qemu-iotests/223.out b/tests/qemu-iotests/223.out index 95c40a1..7828a44 100644 --- a/tests/qemu-iotests/223.out +++ b/tests/qemu-iotests/223.out @@ -67,18 +67,18 @@ read 1048576/1048576 bytes at offset 1048576 1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) read 2097152/2097152 bytes at offset 2097152 2 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -[{ "start": 0, "length": 4096, "depth": 0, "zero": false, "data": true}, -{ "start": 4096, "length": 1044480, "depth": 0, "zero": true, "data": false}, -{ "start": 1048576, "length": 3145728, "depth": 0, "zero": false, "data": true}] +[{ "start": 0, "length": 4096, "depth": 0, "zero": false, "data": true, "offset": OFFSET}, +{ "start": 4096, "length": 1044480, "depth": 0, "zero": true, "data": false, "offset": OFFSET}, +{ "start": 1048576, "length": 3145728, "depth": 0, "zero": false, "data": true, "offset": OFFSET}] [{ "start": 0, "length": 65536, "depth": 0, "zero": false, "data": false}, -{ "start": 65536, "length": 2031616, "depth": 0, "zero": false, "data": true}, +{ "start": 65536, "length": 2031616, "depth": 0, "zero": false, "data": true, "offset": OFFSET}, { "start": 2097152, "length": 2097152, "depth": 0, "zero": false, "data": false}] === Contrast to small granularity dirty-bitmap === -[{ "start": 0, "length": 512, "depth": 0, "zero": false, "data": true}, +[{ "start": 0, "length": 512, "depth": 0, "zero": false, "data": true, "offset": OFFSET}, { "start": 512, "length": 512, "depth": 0, "zero": false, "data": false}, -{ "start": 1024, "length": 2096128, "depth": 0, "zero": false, "data": true}, +{ "start": 1024, "length": 2096128, "depth": 0, "zero": false, "data": true, "offset": OFFSET}, { "start": 2097152, "length": 2097152, "depth": 0, "zero": false, "data": false}] === End qemu NBD server === @@ -94,10 +94,10 @@ read 2097152/2097152 bytes at offset 2097152 === Use qemu-nbd as server === [{ "start": 0, "length": 65536, "depth": 0, "zero": false, "data": false}, -{ "start": 65536, "length": 2031616, "depth": 0, "zero": false, "data": true}, +{ "start": 65536, "length": 2031616, "depth": 0, "zero": false, "data": true, "offset": OFFSET}, { "start": 2097152, "length": 2097152, "depth": 0, "zero": false, "data": false}] -[{ "start": 0, "length": 512, "depth": 0, "zero": false, "data": true}, +[{ "start": 0, "length": 512, "depth": 0, "zero": false, "data": true, "offset": OFFSET}, { "start": 512, "length": 512, "depth": 0, "zero": false, "data": false}, -{ "start": 1024, "length": 2096128, "depth": 0, "zero": false, "data": true}, +{ "start": 1024, "length": 2096128, "depth": 0, "zero": false, "data": true, "offset": OFFSET}, { "start": 2097152, "length": 2097152, "depth": 0, "zero": false, "data": false}] *** done diff --git a/tests/qemu-iotests/241.out b/tests/qemu-iotests/241.out index b76a623..f22eabb 100644 --- a/tests/qemu-iotests/241.out +++ b/tests/qemu-iotests/241.out @@ -4,7 +4,7 @@ QA output created by 241 size: 1024 min block: 512 -[{ "start": 0, "length": 1024, "depth": 0, "zero": false, "data": true}] +[{ "start": 0, "length": 1024, "depth": 0, "zero": false, "data": true, "offset": OFFSET}] 1 KiB (0x400) bytes allocated at offset 0 bytes (0x0) === Exporting unaligned raw image, forced server sector alignment === @@ -14,13 +14,13 @@ WARNING: Image format was not specified for '/home/eblake/qemu/tests/qemu-iotest Specify the 'raw' format explicitly to remove the restrictions. size: 1024 min block: 512 -[{ "start": 0, "length": 1024, "depth": 0, "zero": false, "data": true}] +[{ "start": 0, "length": 1024, "depth": 0, "zero": false, "data": true, "offset": OFFSET}] 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}] +[{ "start": 0, "length": 1024, "depth": 0, "zero": false, "data": true, "offset": OFFSET}] 1 KiB (0x400) bytes allocated at offset 0 bytes (0x0) *** done -- cgit v1.1 From b0245d6478ea5906e3d7a542244d5c015fd47bc7 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Sat, 30 Mar 2019 20:36:36 -0500 Subject: nbd/server: Advertise actual minimum block size Both NBD_CMD_BLOCK_STATUS and structured NBD_CMD_READ will split their reply according to bdrv_block_status() boundaries. If the block device has a request_alignment smaller than 512, but we advertise a block alignment of 512 to the client, then this can result in the server reply violating client expectations by reporting a smaller region of the export than what the client is permitted to address (although this is less of an issue for qemu 4.0 clients, given recent client patches to overlook our non-compliance at EOF). Since it's always better to be strict in what we send, it is worth advertising the actual minimum block limit rather than blindly rounding it up to 512. Note that this patch is not foolproof - it is still possible to provoke non-compliant server behavior using: $ qemu-nbd --image-opts driver=blkdebug,align=512,image.driver=file,image.filename=/path/to/non-aligned-file That is arguably a bug in the blkdebug driver (it should never pass back block status smaller than its alignment, even if it has to make multiple bdrv_get_status calls and determine the least-common-denominator status among the group to return). It may also be possible to observe issues with a backing layer with smaller alignment than the active layer, although so far I have been unable to write a reliable iotest for that scenario (but again, an issue like that could be argued to be a bug in the block layer, or something where we need a flag to bdrv_block_status() to state whether the result must be aligned to the current layer's limits or can be subdivided for accuracy when chasing backing files). Anyways, as blkdebug is not normally used, and as this patch makes our server more interoperable with qemu 3.1 clients, it is worth applying now, even while we still work on a larger patch series for the 4.1 timeframe to have byte-accurate file lengths. Note that the iotests output changes - for 223 and 233, we can see the server's better granularity advertisement; and for 241, the three test cases have the following effects: - natural alignment: the server's smaller alignment is now advertised, and the hole reported at EOF is now the right result; we've gotten rid of the server's non-compliance - forced server alignment: the server still advertises 512 bytes, but still sends a mid-sector hole. This is still a server compliance bug, which needs to be fixed in the block layer in a later patch; output does not change because the client is already being tolerant of the non-compliance - forced client alignment: the server's smaller alignment means that the client now sees the server's status change mid-sector without any protocol violations, but the fact that the map shows an unaligned mid-sector hole is evidence of the block layer problems with aligned block status, to be fixed in a later patch Signed-off-by: Eric Blake Message-Id: <20190329042750.14704-7-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy [eblake: rebase to enhanced iotest 241 coverage] --- tests/qemu-iotests/223.out | 4 ++-- tests/qemu-iotests/233.out | 2 +- tests/qemu-iotests/241.out | 10 ++++++---- 3 files changed, 9 insertions(+), 7 deletions(-) (limited to 'tests') diff --git a/tests/qemu-iotests/223.out b/tests/qemu-iotests/223.out index 7828a44..d5201b2 100644 --- a/tests/qemu-iotests/223.out +++ b/tests/qemu-iotests/223.out @@ -41,7 +41,7 @@ exports available: 2 export: 'n' size: 4194304 flags: 0x4ef ( readonly flush fua trim zeroes df cache ) - min block: 512 + min block: 1 opt block: 4096 max block: 33554432 available meta contexts: 2 @@ -50,7 +50,7 @@ exports available: 2 export: 'n2' size: 4194304 flags: 0x4ed ( flush fua trim zeroes df cache ) - min block: 512 + min block: 1 opt block: 4096 max block: 33554432 available meta contexts: 2 diff --git a/tests/qemu-iotests/233.out b/tests/qemu-iotests/233.out index 5acbc13..9511b6e 100644 --- a/tests/qemu-iotests/233.out +++ b/tests/qemu-iotests/233.out @@ -38,7 +38,7 @@ exports available: 1 export: '' size: 67108864 flags: 0x4ed ( flush fua trim zeroes df cache ) - min block: 512 + min block: 1 opt block: 4096 max block: 33554432 available meta contexts: 1 diff --git a/tests/qemu-iotests/241.out b/tests/qemu-iotests/241.out index f22eabb..f481074 100644 --- a/tests/qemu-iotests/241.out +++ b/tests/qemu-iotests/241.out @@ -3,8 +3,9 @@ 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, "offset": OFFSET}] + min block: 1 +[{ "start": 0, "length": 1000, "depth": 0, "zero": false, "data": true, "offset": OFFSET}, +{ "start": 1000, "length": 24, "depth": 0, "zero": true, "data": true, "offset": OFFSET}] 1 KiB (0x400) bytes allocated at offset 0 bytes (0x0) === Exporting unaligned raw image, forced server sector alignment === @@ -20,7 +21,8 @@ WARNING: Image format was not specified for '/home/eblake/qemu/tests/qemu-iotest === Exporting unaligned raw image, forced client sector alignment === size: 1024 - min block: 512 -[{ "start": 0, "length": 1024, "depth": 0, "zero": false, "data": true, "offset": OFFSET}] + min block: 1 +[{ "start": 0, "length": 1000, "depth": 0, "zero": false, "data": true, "offset": OFFSET}, +{ "start": 1000, "length": 24, "depth": 0, "zero": true, "data": true, "offset": OFFSET}] 1 KiB (0x400) bytes allocated at offset 0 bytes (0x0) *** done -- cgit v1.1