Libguestfs - Mar 2018 - Re: [PATCH nbdkit 1/2] tests: Remove QEMU_IO / HAVE_QEMU_IO.

Small reworking of tests to remove $QEMU_IO, making that consistent
with other test tools, and to test IPv6 connections.

This is for consistency with qemu-img, socat, ss, etc where we test
for these binaries at run time.
---
 configure.ac                |  4 ----
 tests/Makefile.am           |  8 +++-----
 tests/test-parallel-file.sh | 20 +++++++++++++-------
 tests/test-parallel-nbd.sh  | 20 +++++++++++++-------
 4 files changed, 29 insertions(+), 23 deletions(-)

diff --git a/configure.ac b/configure.ac
index 6025ce0..30e4c43 100644
--- a/configure.ac
+++ b/configure.ac
@@ -407,10 +407,6 @@ dnl Check for guestfish (only needed for some of the
tests).
 AC_CHECK_PROG([GUESTFISH], [guestfish], [guestfish], [no])
 AM_CONDITIONAL([HAVE_GUESTFISH], [test "x$GUESTFISH" !=
"xno"])
 
-dnl Check for qemu-io (only needed for some of the tests).
-AC_CHECK_PROG([QEMU_IO], [qemu-io], [qemu-io], [no])
-AM_CONDITIONAL([HAVE_QEMU_IO], [test "x$QEMU_IO" != "xno"])
-
 dnl See plugins/vddk/README.VDDK.
 AC_CHECK_SIZEOF([size_t])
 AS_IF([test "x$ac_cv_sizeof_size_t" =
"x4"],[bits=32],[bits=64])
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 1e32cb6..89ceb9d 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -178,11 +178,9 @@ file-data:
 
 # While most tests need libguestfs, testing parallel I/O is easier when
 # using qemu-io to kick off asynchronous requests.
-if HAVE_QEMU_IO
-TESTS_ENVIRONMENT += QEMU_IO=$(QEMU_IO)
-TESTS += test-parallel-file.sh
-TESTS += test-parallel-nbd.sh
-endif HAVE_QEMU_IO
+TESTS += \
+	test-parallel-file.sh \
+	test-parallel-nbd.sh
 
 # Most in-depth tests need libguestfs, since that is a convenient way to
 # drive qemu.
diff --git a/tests/test-parallel-file.sh b/tests/test-parallel-file.sh
index 8c307af..d032bf7 100755
--- a/tests/test-parallel-file.sh
+++ b/tests/test-parallel-file.sh
@@ -31,17 +31,23 @@
 # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 # SUCH DAMAGE.
 
-# Makefile sets $QEMU_IO, but it's also nice if the script runs again
-# standalone afterwards for diagnosing any failures
-: ${QEMU_IO=qemu-io}
+# Check file-data was created by Makefile and qemu-io exists.
+if ! test -f file-data; then
+    echo "$0: missing file-data"
+    exit 77
+fi
+if ! qemu-io --version; then
+    echo "$0: missing qemu-io"
+    exit 77
+fi
 
 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" \
+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; }
+    { 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
@@ -51,7 +57,7 @@ $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 file=test-parallel-file.data \
-  wdelay=2 rdelay=1 --run '$QEMU_IO -f raw -c "aio_write -P 2 512
512" \
+  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)" != \
@@ -62,7 +68,7 @@ fi
 
 # With default --threads, the faster read should complete first
 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" \
+  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)" != \
diff --git a/tests/test-parallel-nbd.sh b/tests/test-parallel-nbd.sh
index fda0b7d..a8f0d2d 100755
--- a/tests/test-parallel-nbd.sh
+++ b/tests/test-parallel-nbd.sh
@@ -31,9 +31,15 @@
 # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 # SUCH DAMAGE.
 
-# Makefile sets $QEMU_IO, but it's also nice if the # script runs again
-# standalone afterwards for diagnosing any failures
-: ${QEMU_IO=qemu-io}
+# Check file-data was created by Makefile and qemu-io exists.
+if ! test -f file-data; then
+    echo "$0: missing file-data"
+    exit 77
+fi
+if ! qemu-io --version; then
+    echo "$0: missing qemu-io"
+    exit 77
+fi
 
 # We require --exit-with-parent to work
 ( nbdkit --exit-with-parent --help ) >/dev/null 2>&1 ||
@@ -44,9 +50,9 @@ 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" \
+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; }
+    { 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
@@ -62,7 +68,7 @@ nbdkit --exit-with-parent -v -U test-parallel-nbd.sock \
 
 # 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 512 512" -c "aio_read -P 1
0 512" \
+  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
@@ -72,7 +78,7 @@ 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 512 512" -c "aio_read -P 1
0 512" \
+  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
-- 
2.13.2

---
 tests/Makefile.am                  |  4 +--
 tests/{test-ipv4.sh => test-ip.sh} | 51
+++++++++++++++++++++++++++-----------
 tests/test-socket-activation.c     |  2 +-
 3 files changed, 40 insertions(+), 17 deletions(-)

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 89ceb9d..9be57e6 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -50,7 +50,7 @@ EXTRA_DIST = \
 	test-foreground.sh \
 	test-help.sh \
 	test-help-plugin.sh \
-	test-ipv4.sh \
+	test-ip.sh \
 	test_ocaml_plugin.ml \
 	test-ocaml.c \
 	test-parallel-file.sh \
@@ -104,7 +104,7 @@ TESTS += \
 	test-captive.sh \
 	test-random-sock.sh \
 	test-tls.sh \
-	test-ipv4.sh \
+	test-ip.sh \
 	test-socket-activation \
 	test-foreground.sh
 
diff --git a/tests/test-ipv4.sh b/tests/test-ip.sh
similarity index 64%
rename from tests/test-ipv4.sh
rename to tests/test-ip.sh
index 67a5281..4bd841e 100755
--- a/tests/test-ipv4.sh
+++ b/tests/test-ip.sh
@@ -1,6 +1,6 @@
 #!/bin/bash -
 # nbdkit
-# Copyright (C) 2016 Red Hat Inc.
+# Copyright (C) 2016-2018 Red Hat Inc.
 # All rights reserved.
 #
 # Redistribution and use in source and binary forms, with or without
@@ -32,21 +32,28 @@
 # SUCH DAMAGE.
 
 # Every other test uses a Unix domain socket.  This tests nbdkit over
-# IPv4 localhost connections.
-#
-# XXX We should be able to test "just IPv6".  However there is
-# currently no option to listen only on particular interfaces.
+# IPv4 and IPv6 localhost connections.
 
 set -e
 source ./functions.sh
 
+rm -f ip.pid ipv4.out ipv6.out
+
 # Don't fail if certain commands aren't available.
 if ! ss --version; then
     echo "$0: 'ss' command not available"
     exit 77
 fi
-if ! socat -h; then
-    echo "$0: 'socat' command not available"
+if ! command -v qemu-img; then
+    echo "$0: 'qemu-img' command not available"
+    exit 77
+fi
+if ! qemu-img --help | grep -- --image-opts; then
+    echo "$0: 'qemu-img' command does not have the --image-opts
option"
+    exit 77
+fi
+if ! ip -V; then
+    echo "$0: 'ip' command not available"
     exit 77
 fi
 
@@ -56,28 +63,44 @@ for port in `seq 49152 65535`; do
 done
 echo picked unused port $port
 
-nbdkit -P ipv4.pid -p $port example1
+# By default nbdkit will listen on all available interfaces, ie.
+# IPv4 and IPv6.
+nbdkit -P ip.pid -p $port example1
 
 # We may have to wait a short time for the pid file to appear.
 for i in `seq 1 10`; do
-    if test -f ipv4.pid; then
+    if test -f ip.pid; then
         break
     fi
     sleep 1
 done
-if ! test -f ipv4.pid; then
+if ! test -f ip.pid; then
     echo "$0: PID file was not created"
     exit 1
 fi
 
-pid="$(cat ipv4.pid)"
+pid="$(cat ip.pid)"
 
 # Check the process exists.
 kill -s 0 $pid
 
-# Check we can connect to the socket.
-socat TCP:localhost:$port STDIO </dev/null
+# Check we can connect over the IPv4 loopback interface.
+ipv4_lo="$(ip -o -4 addr show scope host)"
+if test -n "$ipv4_lo"; then
+    qemu-img info --image-opts
"file.driver=nbd,file.host=127.0.0.1,file.port=$port" > ipv4.out
+    cat ipv4.out
+    grep -sq "^virtual size: 100M" ipv4.out
+fi
+
+# Check we can connect over the IPv6 loopback interface.
+ipv6_lo="$(ip -o -6 addr show scope host)"
+if test -n "$ipv6_lo"; then
+    qemu-img info --image-opts
"file.driver=nbd,file.host=::1,file.port=$port" > ipv6.out
+    cat ipv6.out
+    grep -sq "^virtual size: 100M" ipv6.out
+fi
 
 # Kill the process.
 kill $pid
-rm ipv4.pid
+rm ip.pid
+rm -f ipv4.out ipv6.out
diff --git a/tests/test-socket-activation.c b/tests/test-socket-activation.c
index 81bd79e..8b1a1f0 100644
--- a/tests/test-socket-activation.c
+++ b/tests/test-socket-activation.c
@@ -39,7 +39,7 @@
  * from the test framework.
  *
  * It's *almost* possible to test this from a shell script
- * (cf. test-ipv4.sh) but as far as I can tell setting LISTEN_PID
+ * (cf. test-ip.sh) but as far as I can tell setting LISTEN_PID
  * correctly is impossible from shell.
  */
 
-- 
2.13.2

On 03/06/2018 06:21 AM, Richard W.M. Jones wrote:
> This is for consistency with qemu-img, socat, ss, etc where we test
> for these binaries at run time.
> ---
>   configure.ac                |  4 ----
>   tests/Makefile.am           |  8 +++-----
>   tests/test-parallel-file.sh | 20 +++++++++++++-------
>   tests/test-parallel-nbd.sh  | 20 +++++++++++++-------
>   4 files changed, 29 insertions(+), 23 deletions(-)
> 
> +++ b/tests/test-parallel-file.sh
> @@ -31,17 +31,23 @@
>   # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
>   # SUCH DAMAGE.
>   
> -# Makefile sets $QEMU_IO, but it's also nice if the script runs again
> -# standalone afterwards for diagnosing any failures
> -: ${QEMU_IO=qemu-io}
> +# Check file-data was created by Makefile and qemu-io exists.
> +if ! test -f file-data; then
Needs rebasing now that the test no longer relies on file-data.  Also, I 
still appreciate being able to override $QEMU_IO from the command line 
(to point to an alternative version on the fly), so even though I agree 
with your Makefile changes, I disagree with dropping the defaulting of 
QEMU_IO to qemu-io,
>   # 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" \
> +qemu-io -f raw -c "aio_write -P 1 0 512" -c "aio_write -P 2
512 512" \
as well as disagree with hard-coding only the first qemu-io in $PATH 
here, instead of allowing a command-line override.

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

On 03/06/2018 06:21 AM, Richard W.M. Jones wrote:
> ---
>   tests/Makefile.am                  |  4 +--
>   tests/{test-ipv4.sh => test-ip.sh} | 51
+++++++++++++++++++++++++++-----------
>   tests/test-socket-activation.c     |  2 +-
>   3 files changed, 40 insertions(+), 17 deletions(-)
> 
ACK

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