Final round of NBD alignment fixes

[Qemu-devel] [PATCH 4/7] iotests: Update 241 to expose backing layer fragmentation

Posted by Eric Blake 6 years, 10 months ago

Previous commits have mentioned that our NBD server still sends
unaligned fragments when an active layer with large advertised minimum
block size is backed by another layer with a smaller block
size. Expand the test to actually cover this scenario, by using qcow2
encryption (which forces 512-byte alignment) with an unaligned raw
backing file.

The test passes, but only because the client side works around the
server's non-compliance; if you repeat the test manually with tracing
turned on, you will see the server sending a status for 1000 bytes
data then 1048 bytes hole, which is not aligned. But reverting commit
737d3f5244 shows that it is indeed the client working around the bug
in the server.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 tests/qemu-iotests/241     | 20 +++++++++++++++++++-
 tests/qemu-iotests/241.out |  9 +++++++++
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/241 b/tests/qemu-iotests/241
index 4b196857387..c1fa6980dc8 100755
--- a/tests/qemu-iotests/241
+++ b/tests/qemu-iotests/241
@@ -28,6 +28,7 @@ nbd_unix_socket=$TEST_DIR/test_qemu_nbd_socket
 _cleanup()
 {
     _cleanup_test_img
+    rm -f "$TEST_IMAGE_FILE.qcow2"
     nbd_server_stop
 }
 trap "_cleanup; exit \$status" 0 1 2 3 15
@@ -37,7 +38,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 . ./common.filter
 . ./common.nbd

-_supported_fmt raw
+_supported_fmt raw # although the test also requires use of qcow2
 _supported_proto nbd
 _supported_os Linux
 _require_command QEMU_NBD
@@ -88,6 +89,23 @@ $QEMU_IMG map --output=json "$TEST_IMG" | _filter_qemu_img_map
 $QEMU_IO -c map "$TEST_IMG"
 nbd_server_stop

+echo
+echo "=== Encrypted qcow2 file backed by unaligned raw image  ==="
+echo
+
+# Enabling encryption in qcow2 forces 512-alignment
+SECRET=secret,id=sec0,data=12345
+$QEMU_IMG create -f qcow2 -b "$TEST_IMG_FILE" -F raw --object "$SECRET" \
+  -o encrypt.format=luks,encrypt.key-secret=sec0,encrypt.iter-time=10 \
+  "$TEST_IMG_FILE.qcow2" 2k | _filter_img_create
+nbd_server_start_unix_socket --object "$SECRET" --image-opts \
+  driver=qcow2,file.filename="$TEST_IMG_FILE.qcow2",encrypt.key-secret=sec0
+
+$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'
diff --git a/tests/qemu-iotests/241.out b/tests/qemu-iotests/241.out
index f481074a02e..ef7de1205d2 100644
--- a/tests/qemu-iotests/241.out
+++ b/tests/qemu-iotests/241.out
@@ -25,4 +25,13 @@ WARNING: Image format was not specified for '/home/eblake/qemu/tests/qemu-iotest
 [{ "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)
+
+=== Encrypted qcow2 file backed by unaligned raw image  ===
+
+Formatting 'TEST_DIR/t.IMGFMT.qcow2', fmt=qcow2 size=2048 backing_file=TEST_DIR/t.IMGFMT backing_fmt=IMGFMT encrypt.format=luks encrypt.key-secret=sec0 encrypt.iter-time=10
+  size:  2048
+  min block: 512
+[{ "start": 0, "length": 1024, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
+{ "start": 1024, "length": 1024, "depth": 0, "zero": true, "data": false, "offset": OFFSET}]
+2 KiB (0x800) bytes     allocated at offset 0 bytes (0x0)
 *** done
-- 
2.20.1

Re: [Qemu-devel] [PATCH 4/7] iotests: Update 241 to expose backing layer fragmentation

Posted by Vladimir Sementsov-Ogievskiy 6 years, 10 months ago

03.04.2019 6:05, Eric Blake wrote:
> Previous commits have mentioned that our NBD server still sends
> unaligned fragments when an active layer with large advertised minimum
> block size is backed by another layer with a smaller block
> size. Expand the test to actually cover this scenario, by using qcow2
> encryption (which forces 512-byte alignment) with an unaligned raw
> backing file.
> 
> The test passes, but only because the client side works around the
> server's non-compliance; if you repeat the test manually with tracing
> turned on, you will see the server sending a status for 1000 bytes
> data then 1048 bytes hole, which is not aligned. But reverting commit
> 737d3f5244 shows that it is indeed the client working around the bug
> in the server.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>

Oops, 241 fails for me:

-WARNING: Image format was not specified for '/home/eblake/qemu/tests/qemu-iotests/scratch/t.raw' and probing guessed raw.
+WARNING: Image format was not specified for '/work/src/qemu/eric/tests/qemu-iotests/scratch/t.raw' and probing guessed raw.

We forget to filter output :(

Tested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

> ---
>   tests/qemu-iotests/241     | 20 +++++++++++++++++++-
>   tests/qemu-iotests/241.out |  9 +++++++++
>   2 files changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/qemu-iotests/241 b/tests/qemu-iotests/241
> index 4b196857387..c1fa6980dc8 100755
> --- a/tests/qemu-iotests/241
> +++ b/tests/qemu-iotests/241
> @@ -28,6 +28,7 @@ nbd_unix_socket=$TEST_DIR/test_qemu_nbd_socket
>   _cleanup()
>   {
>       _cleanup_test_img
> +    rm -f "$TEST_IMAGE_FILE.qcow2"

you mean _IMG_, not _IMAGE_.

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

>       nbd_server_stop
>   }
>   trap "_cleanup; exit \$status" 0 1 2 3 15
> @@ -37,7 +38,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
>   . ./common.filter
>   . ./common.nbd
> 
> -_supported_fmt raw
> +_supported_fmt raw # although the test also requires use of qcow2
>   _supported_proto nbd
>   _supported_os Linux
>   _require_command QEMU_NBD
> @@ -88,6 +89,23 @@ $QEMU_IMG map --output=json "$TEST_IMG" | _filter_qemu_img_map
>   $QEMU_IO -c map "$TEST_IMG"
>   nbd_server_stop
> 
> +echo
> +echo "=== Encrypted qcow2 file backed by unaligned raw image  ==="
> +echo
> +
> +# Enabling encryption in qcow2 forces 512-alignment
> +SECRET=secret,id=sec0,data=12345
> +$QEMU_IMG create -f qcow2 -b "$TEST_IMG_FILE" -F raw --object "$SECRET" \
> +  -o encrypt.format=luks,encrypt.key-secret=sec0,encrypt.iter-time=10 \
> +  "$TEST_IMG_FILE.qcow2" 2k | _filter_img_create
> +nbd_server_start_unix_socket --object "$SECRET" --image-opts \
> +  driver=qcow2,file.filename="$TEST_IMG_FILE.qcow2",encrypt.key-secret=sec0
> +
> +$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'
> diff --git a/tests/qemu-iotests/241.out b/tests/qemu-iotests/241.out
> index f481074a02e..ef7de1205d2 100644
> --- a/tests/qemu-iotests/241.out
> +++ b/tests/qemu-iotests/241.out
> @@ -25,4 +25,13 @@ WARNING: Image format was not specified for '/home/eblake/qemu/tests/qemu-iotest
>   [{ "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)
> +
> +=== Encrypted qcow2 file backed by unaligned raw image  ===
> +
> +Formatting 'TEST_DIR/t.IMGFMT.qcow2', fmt=qcow2 size=2048 backing_file=TEST_DIR/t.IMGFMT backing_fmt=IMGFMT encrypt.format=luks encrypt.key-secret=sec0 encrypt.iter-time=10
> +  size:  2048
> +  min block: 512
> +[{ "start": 0, "length": 1024, "depth": 0, "zero": false, "data": true, "offset": OFFSET},
> +{ "start": 1024, "length": 1024, "depth": 0, "zero": true, "data": false, "offset": OFFSET}]
> +2 KiB (0x800) bytes     allocated at offset 0 bytes (0x0)
>   *** done
> 


-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH 4/7] iotests: Update 241 to expose backing layer fragmentation

Posted by Eric Blake 6 years, 10 months ago

On 4/8/19 8:51 AM, Vladimir Sementsov-Ogievskiy wrote:
> 03.04.2019 6:05, Eric Blake wrote:
>> Previous commits have mentioned that our NBD server still sends
>> unaligned fragments when an active layer with large advertised minimum
>> block size is backed by another layer with a smaller block
>> size. Expand the test to actually cover this scenario, by using qcow2
>> encryption (which forces 512-byte alignment) with an unaligned raw
>> backing file.
>>
>> The test passes, but only because the client side works around the
>> server's non-compliance; if you repeat the test manually with tracing
>> turned on, you will see the server sending a status for 1000 bytes
>> data then 1048 bytes hole, which is not aligned. But reverting commit
>> 737d3f5244 shows that it is indeed the client working around the bug
>> in the server.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
> 
> Oops, 241 fails for me:
> 
> -WARNING: Image format was not specified for '/home/eblake/qemu/tests/qemu-iotests/scratch/t.raw' and probing guessed raw.
> +WARNING: Image format was not specified for '/work/src/qemu/eric/tests/qemu-iotests/scratch/t.raw' and probing guessed raw.
> 
> We forget to filter output :(

The filter bug is pre-existing; separate patch for that is now posted.

> 
> Tested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

I saw this email before -rc3, but thought I had only introduced the
problem in this patch which I omitted from my rc3 pull request. Oh well,
the actual break in iotests came earlier; if there's an -rc4 for other
reasons, we'll get the test fixed then.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org