Libguestfs - Mar 2018 - [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@redhat.com>
---

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
 fi

-- 
2.14.3

On Fri, Mar 02, 2018 at 03:18:07PM -0600, Eric Blake
wrote:
> 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@redhat.com>
> ---
> 
> 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
ACK.

Thanks,

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW