Re: [Libguestfs] [nbdkit PATCH] tests: Make parallel tests work at 512-byte granularity

qemu-io 2.12 will be changing its default alignment to unknown servers so that it does read-modify-write for anything less than 512 bytes. If we implement NBD_OPT_GO, then we can keep qemu-io using 1-byte alignment; but until then, this breaks our parallel tests when using 1-byte alignment because they end up with more delays than expected (thanks to the read-modify-write). Revamp the tests to not rely on sub-sector alignment, which works with both qemu-io 2.11 and 2.12, and fixes a TODO item. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- I'm pushing this one now. TODO | 5 ----- tests/test-parallel-file.sh | 45 ++++++++++++++++++++++++--------------------- tests/test-parallel-nbd.sh | 43 +++++++++++++++++++++++-------------------- 3 files changed, 47 insertions(+), 46 deletions(-) diff --git a/TODO b/TODO index 37baed2..4731b1e 100644 --- a/TODO +++ b/TODO @@ -40,11 +40,6 @@ General ideas for improvements ones like offset) can fail to initialize if they can't guarantee strict alignment and don't want to deal with bounce buffers. -* Tests written that use qemu-io need to be audited: qemu-io 2.11 - would send 1-byte requests to any server, but 2.12 will tighten it - to do read-modify-write to 512 bytes unless the server supports - NBD_OPT_GO. - Suggestions for plugins ----------------------- diff --git a/tests/test-parallel-file.sh b/tests/test-parallel-file.sh index ed1f99c..8c307af 100755 --- a/tests/test-parallel-file.sh +++ b/tests/test-parallel-file.sh @@ -1,6 +1,6 @@ #!/bin/bash - # nbdkit -# Copyright (C) 2017 Red Hat Inc. +# Copyright (C) 2017-2018 Red Hat Inc. # All rights reserved. # # Redistribution and use in source and binary forms, with or without @@ -31,14 +31,17 @@ # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF # SUCH DAMAGE. -# Makefile sets $QEMU_IO and builds file-data, but it's also nice if the -# script runs again standalone afterwards for diagnosing any failures -test -f file-data || { echo "Missing file-data"; exit 77; } +# Makefile sets $QEMU_IO, but it's also nice if the script runs again +# standalone afterwards for diagnosing any failures : ${QEMU_IO=qemu-io} -# Sanity check that qemu-io can issue parallel requests -$QEMU_IO -f raw -c "aio_write -P 2 1 1" -c "aio_read -P 1 0 1" -c aio_flush \ - file-data || { echo "'$QEMU_IO' can't drive parallel requests"; exit 77; } +trap 'rm -f test-parallel-file.data test-parallel-file.out' 0 1 2 3 15 + +# Populate file, and sanity check that qemu-io can issue parallel requests +printf '%1024s' . > test-parallel-file.data +$QEMU_IO -f raw -c "aio_write -P 1 0 512" -c "aio_write -P 2 512 512" \ + -c aio_flush test-parallel-file.data || + { echo "'$QEMU_IO' can't drive parallel requests"; exit 77; } # Set up the file plugin to delay both reads and writes (for a good chance # that parallel requests are in flight), and with writes longer than reads @@ -46,25 +49,25 @@ $QEMU_IO -f raw -c "aio_write -P 2 1 1" -c "aio_read -P 1 0 1" -c aio_flush \ # may have spurious failures under heavy loads on the test machine, where # tuning the delays may help. -trap 'rm -f test-parallel-file.out' 0 1 2 3 15 - # With --threads=1, the write should complete first because it was issued first -nbdkit -v -t 1 -U - --filter=delay file file=file-data wdelay=2 rdelay=1 --run ' - $QEMU_IO -f raw -c "aio_write -P 2 1 1" -c "aio_read -P 1 0 1" -c aio_flush $nbd -' | tee test-parallel-file.out -if test "$(grep '1/1' test-parallel-file.out)" != \ -"wrote 1/1 bytes at offset 1 -read 1/1 bytes at offset 0"; then +nbdkit -v -t 1 -U - --filter=delay file file=test-parallel-file.data \ + wdelay=2 rdelay=1 --run '$QEMU_IO -f raw -c "aio_write -P 2 512 512" \ + -c "aio_read -P 1 0 512" -c aio_flush $nbd' | + tee test-parallel-file.out +if test "$(grep '512/512' test-parallel-file.out)" != \ +"wrote 512/512 bytes at offset 512 +read 512/512 bytes at offset 0"; then exit 1 fi # With default --threads, the faster read should complete first -nbdkit -v -U - --filter=delay file file=file-data wdelay=2 rdelay=1 --run ' - $QEMU_IO -f raw -c "aio_write -P 2 1 1" -c "aio_read -P 1 0 1" -c aio_flush $nbd -' | tee test-parallel-file.out -if test "$(grep '1/1' test-parallel-file.out)" != \ -"read 1/1 bytes at offset 0 -wrote 1/1 bytes at offset 1"; then +nbdkit -v -U - --filter=delay file file=test-parallel-file.data \ + wdelay=2 rdelay=1 --run '$QEMU_IO -f raw -c "aio_write -P 2 512 512" \ + -c "aio_read -P 1 0 512" -c aio_flush $nbd' | + tee test-parallel-file.out +if test "$(grep '512/512' test-parallel-file.out)" != \ +"read 512/512 bytes at offset 0 +wrote 512/512 bytes at offset 512"; then exit 1 fi diff --git a/tests/test-parallel-nbd.sh b/tests/test-parallel-nbd.sh index 244faf6..fda0b7d 100755 --- a/tests/test-parallel-nbd.sh +++ b/tests/test-parallel-nbd.sh @@ -1,6 +1,6 @@ #!/bin/bash - # nbdkit -# Copyright (C) 2017 Red Hat Inc. +# Copyright (C) 2017-2018 Red Hat Inc. # All rights reserved. # # Redistribution and use in source and binary forms, with or without @@ -31,49 +31,52 @@ # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF # SUCH DAMAGE. -# Makefile sets $QEMU_IO and builds file-data, but it's also nice if the -# script runs again standalone afterwards for diagnosing any failures -test -f file-data || { echo "Missing file-data"; exit 77; } +# Makefile sets $QEMU_IO, but it's also nice if the # script runs again +# standalone afterwards for diagnosing any failures : ${QEMU_IO=qemu-io} -# Sanity check that qemu-io can issue parallel requests -$QEMU_IO -f raw -c "aio_write -P 2 1 1" -c "aio_read -P 1 0 1" -c aio_flush \ - file-data || { echo "'$QEMU_IO' can't drive parallel requests"; exit 77; } - # We require --exit-with-parent to work ( nbdkit --exit-with-parent --help ) >/dev/null 2>&1 || { echo "Missing --exit-with-parent support"; exit 77; } +files='test-parallel-nbd.out test-parallel-nbd.sock test-parallel-nbd.data' +trap 'rm -f $files' 0 1 2 3 15 + +# Populate file, and sanity check that qemu-io can issue parallel requests +printf '%1024s' . > test-parallel-nbd.data +$QEMU_IO -f raw -c "aio_write -P 1 0 512" -c "aio_write -P 2 512 512" \ + -c aio_flush test-parallel-nbd.data || + { echo "'$QEMU_IO' can't drive parallel requests"; exit 77; } + # Set up the file plugin to delay both reads and writes (for a good chance # that parallel requests are in flight), and with writes longer than reads # (to more easily detect if out-of-order completion happens). This test # may have spurious failures under heavy loads on the test machine, where # tuning the delays may help. -trap 'rm -f test-parallel-nbd.out test-parallel-nbd.sock' 0 1 2 3 15 ( rm -f test-parallel-nbd.sock nbdkit --exit-with-parent -v -U test-parallel-nbd.sock \ --filter=delay \ - file file=file-data wdelay=2 rdelay=1 & + file file=test-parallel-nbd.data wdelay=2 rdelay=1 & # With --threads=1, the write should complete first because it was issued first nbdkit -v -t 1 -U - nbd socket=test-parallel-nbd.sock --run ' - $QEMU_IO -f raw -c "aio_write -P 2 1 1" -c "aio_read -P 1 0 1" -c aio_flush $nbd -' | tee test-parallel-nbd.out -if test "$(grep '1/1' test-parallel-nbd.out)" != \ -"wrote 1/1 bytes at offset 1 -read 1/1 bytes at offset 0"; then + $QEMU_IO -f raw -c "aio_write -P 2 512 512" -c "aio_read -P 1 0 512" \ + -c aio_flush $nbd' | tee test-parallel-nbd.out +if test "$(grep '512/512' test-parallel-nbd.out)" != \ +"wrote 512/512 bytes at offset 512 +read 512/512 bytes at offset 0"; then exit 1 fi # With default --threads, the faster read should complete first nbdkit -v -U - nbd socket=test-parallel-nbd.sock --run ' - $QEMU_IO -f raw -c "aio_write -P 2 1 1" -c "aio_read -P 1 0 1" -c aio_flush $nbd -' | tee test-parallel-nbd.out -if test "$(grep '1/1' test-parallel-nbd.out)" != \ -"read 1/1 bytes at offset 0 -wrote 1/1 bytes at offset 1"; then + $QEMU_IO -f raw -c "aio_write -P 2 512 512" -c "aio_read -P 1 0 512" \ + -c aio_flush $nbd' | tee test-parallel-nbd.out +if test "$(grep '512/512' test-parallel-nbd.out)" != \ +"read 512/512 bytes at offset 0 +wrote 512/512 bytes at offset 512"; then exit 1