[Libguestfs] [nbdkit PATCH 3/4] tests: Don't let test-parallel-* hang on nbdkit bug

Richard W.M. Jones rjones at redhat.com
Tue Mar 17 08:16:19 UTC 2020


On Mon, Mar 16, 2020 at 10:36:16PM -0500, Eric Blake wrote:
> If nbdkit has a bug (such as the nbd-standalone bug fixed in the
> previous commit), qemu-io ends up waiting forever rather than
> realizing that if the server disappears unexpectedly then qemu-io
> should quit.  So add timeouts so the testsuite will flag the problem
> instead of hang (tested by reordering this commit before the
> previous).
> 
> It's trickier than I expected: from the command line, 'timeout 10s
> qemu-io ...' works just fine, but from a script, it fails.  Why?
> Because timeout defaults to creating a new process group when run
> non-interactively, but qemu-io is generally an interactive program
> that expects to be able to modify the terminal settings when stdin is
> a terminal, even when it will not be reading from stdin because of the
> use of -c.  When stdin is inherited but no longer the controlling
> terminal because of timeout's new process group, qemu-io fails to make
> progress. As solutions, we can either use 'timeout --foreground', or
> redirect stdin to something that is not a terminal; this patch uses
> the latter.

Since the timeout is only used here as a last resort, can
we make it much larger?  This is only going to cause weird
test failures on the usual suspects (z, armv7, valgrind, ...)
The timeout could be 60s or even 5 minutes without it affecting
normal users.

With a change like that, ACK.

Rich.

> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---
>  tests/test-parallel-file.sh | 19 ++++++++++---------
>  tests/test-parallel-nbd.sh  | 13 ++++++++-----
>  tests/test-parallel-sh.sh   | 19 ++++++++++---------
>  3 files changed, 28 insertions(+), 23 deletions(-)
> 
> diff --git a/tests/test-parallel-file.sh b/tests/test-parallel-file.sh
> index 20276d48..136c2db5 100755
> --- a/tests/test-parallel-file.sh
> +++ b/tests/test-parallel-file.sh
> @@ -1,6 +1,6 @@
>  #!/usr/bin/env bash
>  # nbdkit
> -# Copyright (C) 2017-2019 Red Hat Inc.
> +# Copyright (C) 2017-2020 Red Hat Inc.
>  #
>  # Redistribution and use in source and binary forms, with or without
>  # modification, are permitted provided that the following conditions are
> @@ -35,6 +35,7 @@ source ./functions.sh
>  # Check file-data was created by Makefile and qemu-io exists.
>  requires test -f file-data
>  requires qemu-io --version
> +requires timeout --version
> 
>  nbdkit --dump-plugin file | grep -q ^thread_model=parallel ||
>      { echo "nbdkit lacks support for parallel requests"; exit 77; }
> @@ -43,8 +44,8 @@ cleanup_fn rm -f test-parallel-file.data test-parallel-file.out
> 
>  # 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 ||
> +timeout 10s </dev/null 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
> @@ -55,8 +56,8 @@ qemu-io -f raw -c "aio_write -P 1 0 512" -c "aio_write -P 2 512 512" \
> 
>  # With --threads=1, the write should complete first because it was issued first
>  nbdkit -v -t 1 -U - --filter=delay 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' |
> +  wdelay=2 rdelay=1 --run 'timeout 10s </dev/null 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
> @@ -66,8 +67,8 @@ fi
> 
>  # With default --threads, the faster read should complete first
>  nbdkit -v -U - --filter=delay 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' |
> +  wdelay=2 rdelay=1 --run 'timeout 10s </dev/null 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
> @@ -77,8 +78,8 @@ fi
> 
>  # With --filter=noparallel, the write should complete first because it was issued first
>  nbdkit -v -U - --filter=noparallel --filter=delay 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' |
> +  wdelay=2 rdelay=1 --run 'timeout 10s </dev/null 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
> diff --git a/tests/test-parallel-nbd.sh b/tests/test-parallel-nbd.sh
> index 4e546df4..1fdf7df4 100755
> --- a/tests/test-parallel-nbd.sh
> +++ b/tests/test-parallel-nbd.sh
> @@ -1,6 +1,6 @@
>  #!/usr/bin/env bash
>  # nbdkit
> -# Copyright (C) 2017-2019 Red Hat Inc.
> +# Copyright (C) 2017-2020 Red Hat Inc.
>  #
>  # Redistribution and use in source and binary forms, with or without
>  # modification, are permitted provided that the following conditions are
> @@ -35,6 +35,7 @@ source ./functions.sh
>  # Check file-data was created by Makefile and qemu-io exists.
>  requires test -f file-data
>  requires qemu-io --version
> +requires timeout --version
> 
>  nbdkit --dump-plugin nbd | grep -q ^thread_model=parallel ||
>      { echo "nbdkit lacks support for parallel requests"; exit 77; }
> @@ -46,8 +47,8 @@ cleanup_fn rm -f $files
> 
>  # 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 ||
> +timeout 10s </dev/null 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
> @@ -63,7 +64,8 @@ start_nbdkit -P test-parallel-nbd.pid \
> 
>  # With --threads=1, the write should complete first because it was issued first
>  nbdkit -v -t 1 -U - nbd socket=$sock --run '
> -  qemu-io -f raw -c "aio_write -P 2 512 512" -c "aio_read -P 1 0 512" \
> +  timeout 10s </dev/null 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
> @@ -73,7 +75,8 @@ fi
> 
>  # With default --threads, the faster read should complete first
>  nbdkit -v -U - nbd socket=$sock --run '
> -  qemu-io -f raw -c "aio_write -P 2 512 512" -c "aio_read -P 1 0 512" \
> +  timeout 10s </dev/null 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
> diff --git a/tests/test-parallel-sh.sh b/tests/test-parallel-sh.sh
> index 15d8875d..4cc93c33 100755
> --- a/tests/test-parallel-sh.sh
> +++ b/tests/test-parallel-sh.sh
> @@ -1,6 +1,6 @@
>  #!/usr/bin/env bash
>  # nbdkit
> -# Copyright (C) 2017-2019 Red Hat Inc.
> +# Copyright (C) 2017-2020 Red Hat Inc.
>  #
>  # Redistribution and use in source and binary forms, with or without
>  # modification, are permitted provided that the following conditions are
> @@ -35,6 +35,7 @@ source ./functions.sh
>  # Check file-data was created by Makefile and qemu-io exists.
>  requires test -f file-data
>  requires qemu-io --version
> +requires timeout --version
> 
>  nbdkit --dump-plugin sh | grep -q ^thread_model=parallel ||
>      { echo "nbdkit lacks support for parallel requests"; exit 77; }
> @@ -43,8 +44,8 @@ cleanup_fn rm -f test-parallel-sh.data test-parallel-sh.out test-parallel-sh.scr
> 
>  # Populate file, and sanity check that qemu-io can issue parallel requests
>  printf '%1024s' . > test-parallel-sh.data
> -qemu-io -f raw -c "aio_write -P 1 0 512" -c "aio_write -P 2 512 512" \
> -         -c aio_flush test-parallel-sh.data ||
> +timeout 10s </dev/null qemu-io -f raw -c "aio_write -P 1 0 512" \
> +        -c "aio_write -P 2 512 512" -c aio_flush test-parallel-sh.data ||
>      { echo "'qemu-io' can't drive parallel requests"; exit 77; }
> 
>  # Set up the sh plugin to delay both reads and writes (for a good
> @@ -96,8 +97,8 @@ chmod +x test-parallel-sh.script
> 
>  # With --threads=1, the write should complete first because it was issued first
>  nbdkit -v -t 1 -U - --filter=delay sh test-parallel-sh.script \
> -  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' |
> +  wdelay=2 rdelay=1 --run 'timeout 10s </dev/null 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-sh.out
>  if test "$(grep '512/512' test-parallel-sh.out)" != \
>  "wrote 512/512 bytes at offset 512
> @@ -107,8 +108,8 @@ fi
> 
>  # With default --threads, the faster read should complete first
>  nbdkit -v -U - --filter=delay sh test-parallel-sh.script \
> -  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' |
> +  wdelay=2 rdelay=1 --run 'timeout 10s </dev/null 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-sh.out
>  if test "$(grep '512/512' test-parallel-sh.out)" != \
>  "read 512/512 bytes at offset 0
> @@ -120,8 +121,8 @@ fi
>  # issued first. Also test that the log filter doesn't leak an fd
>  nbdkit -v -U - --filter=noparallel --filter=log --filter=delay \
>    sh test-parallel-sh.script logfile=/dev/null \
> -  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' |
> +  wdelay=2 rdelay=1 --run 'timeout 10s </dev/null 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-sh.out
>  if test "$(grep '512/512' test-parallel-sh.out)" != \
>  "wrote 512/512 bytes at offset 512
> -- 
> 2.25.1
> 
> _______________________________________________
> Libguestfs mailing list
> Libguestfs at redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs

-- 
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




More information about the Libguestfs mailing list