[PATCH nbdkit 0/4] tests: Move common functions into tests/functions.sh
Combine much common code into tests/functions.sh. Patch 1: Preparation for patch 3. Patch 2: Fix a long-standing bug in how man pages links are generated. Patch 3: Common code for iterating a test function over every plugin. Patch 4: Common code for starting nbdkit in a test and waiting for the PID file to appear. This is the largest and most complex of the patches but is basically repetitive. Rich.
It's easier to get it to other places if it starts out in the configure script. Also split the list into language and non-language plugins. --- common-rules.mk | 44 ---------------------------------------- configure.ac | 53 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 44 deletions(-) diff --git a/common-rules.mk b/common-rules.mk index 2c5609f..03e01e6 100644 --- a/common-rules.mk +++ b/common-rules.mk @@ -30,50 +30,6 @@ # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF # SUCH DAMAGE. -plugins = \ - curl \ - data \ - example1 \ - example2 \ - example3 \ - example4 \ - ext2 \ - file \ - guestfs \ - gzip \ - libvirt \ - lua \ - memory \ - nbd \ - null \ - ocaml \ - pattern \ - perl \ - python \ - random \ - ruby \ - sh \ - split \ - streaming \ - tar \ - tcl \ - vddk \ - xz \ - zero - -filters = \ - blocksize \ - cache \ - cow \ - delay \ - error \ - fua \ - log \ - nozero \ - offset \ - partition \ - truncate - plugindir = $(libdir)/nbdkit/plugins filterdir = $(libdir)/nbdkit/filters diff --git a/configure.ac b/configure.ac index 4d340d4..3a02c66 100644 --- a/configure.ac +++ b/configure.ac @@ -542,6 +542,59 @@ AC_ARG_ENABLE([vddk],[ [enable_vddk=yes]) AM_CONDITIONAL([HAVE_VDDK], [test "x$enable_vddk" = "xyes"]) +dnl List of plugins and filters. +lang_plugins="\ + lua \ + ocaml \ + perl \ + python \ + ruby \ + sh \ + tcl \ + " +non_lang_plugins="\ + curl \ + data \ + example1 \ + example2 \ + example3 \ + example4 \ + ext2 \ + file \ + guestfs \ + gzip \ + libvirt \ + memory \ + nbd \ + null \ + pattern \ + random \ + split \ + streaming \ + tar \ + vddk \ + xz \ + zero \ + " +plugins="$(echo $lang_plugins $non_lang_plugins | xargs -n1 | sort -u | xargs)" +filters="\ + blocksize \ + cache \ + cow \ + delay \ + error \ + fua \ + log \ + nozero \ + offset \ + partition \ + truncate \ + " +AC_SUBST([plugins]) +AC_SUBST([lang_plugins]) +AC_SUBST([non_lang_plugins]) +AC_SUBST([filters]) + dnl Produce output files. AC_CONFIG_HEADERS([config.h]) AC_CONFIG_FILES([nbdkit], -- 2.19.0.rc0
On 9/11/18 1:47 PM, Richard W.M. Jones wrote:
It's easier to get it to other places if it starts out in the configure script.
Also split the list into language and non-language plugins. --- common-rules.mk | 44 ---------------------------------------- configure.ac | 53 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 44 deletions(-)
+++ b/configure.ac @@ -542,6 +542,59 @@ AC_ARG_ENABLE([vddk],[ [enable_vddk=yes]) AM_CONDITIONAL([HAVE_VDDK], [test "x$enable_vddk" = "xyes"])
+dnl List of plugins and filters. +lang_plugins="\ + lua \ + ocaml \ + perl \ + python \ + ruby \ + sh \ + tcl \ + " +non_lang_plugins="\ + curl \ + data \ + example1 \ + example2 \
Inconsistent spacing (space-vs-tab doesn't matter in this particular context, but it looks weird to use both)
+ " +plugins="$(echo $lang_plugins $non_lang_plugins | xargs -n1 | sort -u | xargs)"
Your use of xargs as a reformatter is interesting ;) Could also be spelled: $(echo $lang_plugins $non_lang_plugins | tr -s ' \t\n' ' ' | sort -u | tr ' ' '\n')" but that's longer to type, so your version is fine. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
On Tue, Sep 11, 2018 at 02:13:03PM -0500, Eric Blake wrote:
On 9/11/18 1:47 PM, Richard W.M. Jones wrote:
It's easier to get it to other places if it starts out in the configure script.
Also split the list into language and non-language plugins. --- common-rules.mk | 44 ---------------------------------------- configure.ac | 53 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 44 deletions(-)
+++ b/configure.ac @@ -542,6 +542,59 @@ AC_ARG_ENABLE([vddk],[ [enable_vddk=yes]) AM_CONDITIONAL([HAVE_VDDK], [test "x$enable_vddk" = "xyes"]) +dnl List of plugins and filters. +lang_plugins="\ + lua \ + ocaml \ + perl \ + python \ + ruby \ + sh \ + tcl \ + " +non_lang_plugins="\ + curl \ + data \ + example1 \ + example2 \
Inconsistent spacing (space-vs-tab doesn't matter in this particular context, but it looks weird to use both)
Ooops. Wasn't visible in emacs, but I can fix that next time.
+ " +plugins="$(echo $lang_plugins $non_lang_plugins | xargs -n1 | sort -u | xargs)"
Your use of xargs as a reformatter is interesting ;) Could also be spelled:
$(echo $lang_plugins $non_lang_plugins | tr -s ' \t\n' ' ' | sort -u | tr ' ' '\n')"
but that's longer to type, so your version is fine.
This one actually came from stackoverflow: https://stackoverflow.com/a/8802788 I wanted something that would work on BSD (although I didn't test it yet ...) Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html
On 9/11/18 4:06 PM, Richard W.M. Jones wrote:
+ " +plugins="$(echo $lang_plugins $non_lang_plugins | xargs -n1 | sort -u | xargs)"
Your use of xargs as a reformatter is interesting ;) Could also be spelled:
$(echo $lang_plugins $non_lang_plugins | tr -s ' \t\n' ' ' | sort -u | tr ' ' '\n')"
but that's longer to type, so your version is fine.
This one actually came from stackoverflow:
which also mentioned: plugins=$(echo $(printf %s\\n $lang_plugins $non_lang_plugins | sort -u)) for less typing, fewer processes, and no dependence on xargs. Isn't shell golf fun :)
I wanted something that would work on BSD (although I didn't test it yet ...)
I haven't tested on BSD either, but all of the solutions listed here [whether xargs, tr, or $echo $(printf %s\\n)] are portable at least according to POSIX. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
--- docs/Makefile.am | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/Makefile.am b/docs/Makefile.am index ccf0a79..cbe7033 100644 --- a/docs/Makefile.am +++ b/docs/Makefile.am @@ -104,10 +104,11 @@ nbdkit-filter.3: nbdkit-filter.pod filter-links.pod endif HAVE_POD -# XXX Language links are broken - they link to section 1 instead of section 3. plugin-links.pod: $(top_srcdir)/common-rules.mk rm -f $@ $@-t - $(srcdir)/make-links.sh plugin 1 $(plugins) > $@-t + $(srcdir)/make-links.sh plugin 1 $(non_lang_plugins) > $@-t + echo \; >> $@-t + $(srcdir)/make-links.sh plugin 3 $(lang_plugins) >> $@-t mv $@-t $@ filter-links.pod: $(top_srcdir)/common-rules.mk -- 2.19.0.rc0
This resurrects the unused tests/functions.sh file (although now we need to generate it from tests/functions.sh.in). Put the common code for running a test against every plugin here. Because of the previous commits we can now use the plugins list directly from configure.ac instead of needing to use weird shell script, although we still need to preserve the test that the plugin was built so that the tests continue to work if a plugin is disabled or not built through missing dependencies. --- .gitignore | 1 + configure.ac | 1 + tests/Makefile.am | 1 - tests/{functions.sh => functions.sh.in} | 23 +++++++++++++++++++---- tests/test-dump-plugin.sh | 19 ++++++++----------- tests/test-help.sh | 18 ++++++++---------- tests/test-version.sh | 18 ++++++++---------- 7 files changed, 45 insertions(+), 36 deletions(-) diff --git a/.gitignore b/.gitignore index a21484e..b8396f1 100644 --- a/.gitignore +++ b/.gitignore @@ -55,6 +55,7 @@ Makefile.in /tests/disk.xz /tests/ext2.img /tests/file-data +/tests/functions.sh /tests/keys.psk /tests/offset-data /tests/partition-disk diff --git a/configure.ac b/configure.ac index 3a02c66..518b2bc 100644 --- a/configure.ac +++ b/configure.ac @@ -651,6 +651,7 @@ AC_CONFIG_FILES([Makefile filters/truncate/Makefile src/Makefile src/nbdkit.pc + tests/functions.sh tests/Makefile]) AC_OUTPUT diff --git a/tests/Makefile.am b/tests/Makefile.am index a7d0c2d..23316ea 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -35,7 +35,6 @@ include $(top_srcdir)/common-rules.mk MAINTAINERCLEANFILES = EXTRA_DIST = \ - functions.sh \ make-pki.sh \ make-psk.sh \ python-exception.py \ diff --git a/tests/functions.sh b/tests/functions.sh.in similarity index 74% rename from tests/functions.sh rename to tests/functions.sh.in index e54bb9d..4071384 100644 --- a/tests/functions.sh +++ b/tests/functions.sh.in @@ -1,5 +1,7 @@ # nbdkit -# Copyright (C) 2017 Red Hat Inc. +# Common functions used by the tests. +# @configure_input@ +# Copyright (C) 2017-2018 Red Hat Inc. # All rights reserved. # # Redistribution and use in source and binary forms, with or without @@ -30,6 +32,19 @@ # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF # SUCH DAMAGE. -# Some functions used by the tests. - -# (Currently empty) +# foreach_plugin f +# +# For each plugin that was built, call test function f with the plugin +# name as argument. +foreach_plugin () +{ + for p in @plugins@; do + # Was the plugin built? + d=@top_builddir@/plugins/$p + if [ -f $d/.libs/nbdkit-$p-plugin.so ] || + [ -f $d/nbdkit-$p-plugin ]; then + # Yes so run the test. + "$1" "$p" + fi + done +} diff --git a/tests/test-dump-plugin.sh b/tests/test-dump-plugin.sh index e08db84..77bd995 100755 --- a/tests/test-dump-plugin.sh +++ b/tests/test-dump-plugin.sh @@ -31,9 +31,9 @@ # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF # SUCH DAMAGE. +source functions.sh set -e set -x -source ./functions.sh # Basic check that the name field is present. output="$(nbdkit example1 --dump-plugin)" @@ -51,15 +51,11 @@ if [[ ! ( "$output" =~ example2_extra\=hello ) ]]; then exit 1 fi -# Get a list of all plugins and run --dump-plugin on all of them. +# Run nbdkit plugin --dump-plugin for each plugin. # However some of these tests are expected to fail. -plugins="$( - cd ..; - ls -1 plugins/*/.libs/nbdkit-*-plugin.so plugins/*/nbdkit-*-plugin | - sed 's,^plugins/\([^/]*\)/.*,\1,' -)" -for p in $plugins; do - case "$p${NBDKIT_VALGRIND:+-valgrind}" in +test () +{ + case "$1${NBDKIT_VALGRIND:+-valgrind}" in vddk | vddk-valgrind) # VDDK won't run without special environment variables # being set, so ignore it. @@ -70,7 +66,8 @@ for p in $plugins; do # Plugins written in scripting languages can't run under valgrind. ;; *) - nbdkit $p --dump-plugin + nbdkit $1 --dump-plugin ;; esac -done +} +foreach_plugin test diff --git a/tests/test-help.sh b/tests/test-help.sh index f1a988c..9bda3c2 100755 --- a/tests/test-help.sh +++ b/tests/test-help.sh @@ -31,6 +31,7 @@ # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF # SUCH DAMAGE. +source functions.sh set -e output="$(nbdkit --help)" @@ -40,15 +41,11 @@ if [[ ! ( "$output" =~ dump-config ) ]]; then exit 1 fi -# Get a list of all plugins and run --help on all of them. +# Run nbdkit plugin --help for each plugin. # However some of these tests are expected to fail. -plugins="$( - cd ..; - ls -1 plugins/*/.libs/nbdkit-*-plugin.so plugins/*/nbdkit-*-plugin | - sed 's,^plugins/\([^/]*\)/.*,\1,' -)" -for p in $plugins; do - case "$p${NBDKIT_VALGRIND:+-valgrind}" in +test () +{ + case "$1${NBDKIT_VALGRIND:+-valgrind}" in vddk | vddk-valgrind) # VDDK won't run without special environment variables # being set, so ignore it. @@ -59,7 +56,8 @@ for p in $plugins; do # Plugins written in scripting languages can't run under valgrind. ;; *) - nbdkit $p --help + nbdkit $1 --help ;; esac -done +} +foreach_plugin test diff --git a/tests/test-version.sh b/tests/test-version.sh index 39fe74a..262e1a2 100755 --- a/tests/test-version.sh +++ b/tests/test-version.sh @@ -31,6 +31,7 @@ # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF # SUCH DAMAGE. +source functions.sh set -e output="$(nbdkit --version)" @@ -40,15 +41,11 @@ if [[ ! ( "$output" =~ ^nbdkit\ 1\. ) ]]; then exit 1 fi -# Get a list of all plugins and run --version on all of them. +# Run nbdkit plugin --version for each plugin. # However some of these tests are expected to fail. -plugins="$( - cd ..; - ls -1 plugins/*/.libs/nbdkit-*-plugin.so plugins/*/nbdkit-*-plugin | - sed 's,^plugins/\([^/]*\)/.*,\1,' -)" -for p in $plugins; do - case "$p${NBDKIT_VALGRIND:+-valgrind}" in +test () +{ + case "$1${NBDKIT_VALGRIND:+-valgrind}" in vddk | vddk-valgrind) # VDDK won't run without special environment variables # being set, so ignore it. @@ -59,7 +56,8 @@ for p in $plugins; do # Plugins written in scripting languages can't run under valgrind. ;; *) - nbdkit $p --version + nbdkit $1 --version ;; esac -done +} +foreach_plugin test -- 2.19.0.rc0
On 9/11/18 1:47 PM, Richard W.M. Jones wrote:
This resurrects the unused tests/functions.sh file (although now we need to generate it from tests/functions.sh.in). Put the common code for running a test against every plugin here.
Because of the previous commits we can now use the plugins list directly from configure.ac instead of needing to use weird shell script, although we still need to preserve the test that the plugin was built so that the tests continue to work if a plugin is disabled or not built through missing dependencies. --- .gitignore | 1 + configure.ac | 1 + tests/Makefile.am | 1 - tests/{functions.sh => functions.sh.in} | 23 +++++++++++++++++++---- tests/test-dump-plugin.sh | 19 ++++++++----------- tests/test-help.sh | 18 ++++++++---------- tests/test-version.sh | 18 ++++++++---------- 7 files changed, 45 insertions(+), 36 deletions(-)
+# foreach_plugin f +# +# For each plugin that was built, call test function f with the plugin +# name as argument. +foreach_plugin () +{ + for p in @plugins@; do + # Was the plugin built? + d=@top_builddir@/plugins/$p + if [ -f $d/.libs/nbdkit-$p-plugin.so ] || + [ -f $d/nbdkit-$p-plugin ]; then
A bit cavalier on not quoting arguments to [; but works since neither $d nor $p should contain spaces.
+ # Yes so run the test. + "$1" "$p"
Looks funny to quote $p here but not above.
+++ b/tests/test-dump-plugin.sh @@ -31,9 +31,9 @@ # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF # SUCH DAMAGE.
+source functions.sh set -e set -x -source ./functions.sh
You NEED to use ./ with source (otherwise, you perform a PATH search, which fails if ./ is not in PATH); this change is a regression. (Whether you use the shorter '.' instead of the longer bashism 'source' is up to you, since the tests all use bash)
+++ b/tests/test-help.sh @@ -31,6 +31,7 @@ # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF # SUCH DAMAGE.
+source functions.sh set -e
Multiple spots in your patch need ./ Otherwise the refactoring looks sane. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
On Tue, Sep 11, 2018 at 02:21:27PM -0500, Eric Blake wrote:
+++ b/tests/test-dump-plugin.sh @@ -31,9 +31,9 @@ # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF # SUCH DAMAGE. +source functions.sh set -e set -x -source ./functions.sh
You NEED to use ./ with source (otherwise, you perform a PATH search, which fails if ./ is not in PATH); this change is a regression.
(Whether you use the shorter '.' instead of the longer bashism 'source' is up to you, since the tests all use bash)
Fair enough. I didn't realize that source would use a path search, since I guess it can't use exec*p internally. But I checked the man page and you are quite right -- it's using $PATH. I will change all these back to "./functions.sh". 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
This assumes bashisms, but bash is required to run the tests. This is mostly simple refactoring. Except for the test-memory*.sh tests where nbdkit used to run in the foreground, but that seems to be a consequence of some left over debugging. --- tests/functions.sh.in | 35 ++++++++++++++++++++ tests/test-blocksize.sh | 23 +++---------- tests/test-cache.sh | 16 ++------- tests/test-cow.sh | 16 ++------- tests/test-data-7E.sh | 16 ++------- tests/test-data-base64.sh | 16 ++------- tests/test-data-raw.sh | 16 ++------- tests/test-fua.sh | 45 ++++++++++---------------- tests/test-ip.sh | 17 ++-------- tests/test-log.sh | 16 ++------- tests/test-memory-largest-for-qemu.sh | 19 ++--------- tests/test-memory-largest.sh | 20 ++---------- tests/test-nozero.sh | 41 ++++++++--------------- tests/test-offset2.sh | 16 ++------- tests/test-parallel-nbd.sh | 15 ++------- tests/test-pattern-largest-for-qemu.sh | 16 ++------- tests/test-pattern-largest.sh | 16 ++------- tests/test-pattern.sh | 16 ++------- tests/test-start.sh | 17 ++-------- tests/test-tls-psk.sh | 18 ++--------- tests/test-tls.sh | 17 ++-------- tests/test-truncate1.sh | 16 ++------- tests/test-truncate2.sh | 16 ++------- 23 files changed, 112 insertions(+), 347 deletions(-) diff --git a/tests/functions.sh.in b/tests/functions.sh.in index 4071384..01f3a60 100644 --- a/tests/functions.sh.in +++ b/tests/functions.sh.in @@ -32,6 +32,41 @@ # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF # SUCH DAMAGE. +# start_nbdkit args... +# +# Run nbdkit with args and wait for it to start up. If it fails +# to start up, exit with an error message. +start_nbdkit () +{ + # Find the -P <pidfile> argument. + pidfile= + for i in `seq 1 $#`; do + if [ "${!i}" = "-P" ]; then + j=$((i+1)) + pidfile="${!j}" + fi + done + if [ "$pidfile" = "" ]; then + echo "$0: start_nbdkit: -P option not present on nbdkit command line" + exit 1 + fi + + # Run nbdkit. + nbdkit "$@" + + # Wait for the pidfile to appear. + for i in `seq 1 10`; do + if test -f "$pidfile"; then + break + fi + sleep 1 + done + if ! test -f "$pidfile"; then + echo "$0: start_nbdkit: PID file $pidfile was not created" + exit 1 + fi +} + # foreach_plugin f # # For each plugin that was built, call test function f with the plugin diff --git a/tests/test-blocksize.sh b/tests/test-blocksize.sh index cb9b09f..3fe3723 100755 --- a/tests/test-blocksize.sh +++ b/tests/test-blocksize.sh @@ -31,6 +31,7 @@ # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF # SUCH DAMAGE. +source functions.sh set -e files="blocksize1.img blocksize1.log blocksize1.sock blocksize1.pid @@ -72,27 +73,13 @@ cleanup () trap cleanup INT QUIT TERM EXIT ERR # Run two parallel nbdkit; to compare the logs and see what changes. -nbdkit -P blocksize1.pid -U blocksize1.sock \ +start_nbdkit -P blocksize1.pid -U blocksize1.sock \ --filter=log file logfile=blocksize1.log blocksize1.img -nbdkit -P blocksize2.pid -U blocksize2.sock --filter=blocksize \ +start_nbdkit -P blocksize2.pid -U blocksize2.sock --filter=blocksize \ --filter=log file logfile=blocksize2.log blocksize2.img \ minblock=1024 maxdata=512k maxlen=1M - -# We may have to wait a short time for the pid files to appear. -for i in `seq 1 10`; do - if test -f blocksize1.pid && test -f blocksize2.pid; then - break - fi - sleep 1 -done - -pid1="$(cat blocksize1.pid)" || : -pid2="$(cat blocksize2.pid)" || : - -if ! test -f blocksize1.pid || ! test -f blocksize2.pid; then - echo "$0: PID files were not created" - exit 1 -fi +pid1="$(cat blocksize1.pid)" +pid2="$(cat blocksize2.pid)" # Test behavior on short accesses. qemu-io -f raw -c 'r 1 1' -c 'w 10001 1' -c 'w -z 20001 1' \ diff --git a/tests/test-cache.sh b/tests/test-cache.sh index 215bab1..81c85a3 100755 --- a/tests/test-cache.sh +++ b/tests/test-cache.sh @@ -31,6 +31,7 @@ # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF # SUCH DAMAGE. +source functions.sh set -e set -x @@ -41,20 +42,7 @@ rm -f $files truncate -s 1G cache.img # Run nbdkit with the caching filter. -nbdkit -P cache.pid -U cache.sock --filter=cache file cache.img - -# We may have to wait a short time for the pid file to appear. -for i in `seq 1 10`; do - if test -f cache.pid; then - break - fi - sleep 1 -done -if ! test -f cache.pid; then - echo "$0: PID file was not created" - exit 1 -fi - +start_nbdkit -P cache.pid -U cache.sock --filter=cache file cache.img pid="$(cat cache.pid)" # Kill the nbdkit process on exit. diff --git a/tests/test-cow.sh b/tests/test-cow.sh index f9b0649..abe6dbd 100755 --- a/tests/test-cow.sh +++ b/tests/test-cow.sh @@ -31,6 +31,7 @@ # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF # SUCH DAMAGE. +source functions.sh set -e set -x @@ -42,20 +43,7 @@ guestfish -N cow-base.img=fs exit lastmod="$(stat -c "%y" cow-base.img)" # Run nbdkit with a COW overlay. -nbdkit -P cow.pid -U cow.sock --filter=cow file cow-base.img - -# We may have to wait a short time for the pid file to appear. -for i in `seq 1 10`; do - if test -f cow.pid; then - break - fi - sleep 1 -done -if ! test -f cow.pid; then - echo "$0: PID file was not created" - exit 1 -fi - +start_nbdkit -P cow.pid -U cow.sock --filter=cow file cow-base.img pid="$(cat cow.pid)" # Kill the nbdkit process on exit. diff --git a/tests/test-data-7E.sh b/tests/test-data-7E.sh index 8327511..1ffc668 100755 --- a/tests/test-data-7E.sh +++ b/tests/test-data-7E.sh @@ -34,6 +34,7 @@ # Test the data plugin creating a 7 EB partitioned disk, and # the partition filter on top. +source functions.sh set -e set -x @@ -47,7 +48,7 @@ if ! qemu-io --help >/dev/null; then fi # Run nbdkit. -nbdkit -P data-7E.pid -U data-7E.sock \ +start_nbdkit -P data-7E.pid -U data-7E.sock \ --filter=partition \ data size=7E partition=1 \ data=" @@ -86,19 +87,6 @@ nbdkit -P data-7E.pid -U data-7E.sock \ 0xdf 0xff 0xff 0xff 0xff 0xff 0x37 0 0x80 0 0 0 0x80 0 0 0 0x79 0x8a 0xd0 0x7e 0 0 0 0 " - -# We may have to wait a short time for the pid file to appear. -for i in `seq 1 10`; do - if test -f data-7E.pid; then - break - fi - sleep 1 -done -if ! test -f data-7E.pid; then - echo "$0: PID file was not created" - exit 1 -fi - pid="$(cat data-7E.pid)" # Kill the nbdkit process on exit. diff --git a/tests/test-data-base64.sh b/tests/test-data-base64.sh index 6ff324b..be9f25a 100755 --- a/tests/test-data-base64.sh +++ b/tests/test-data-base64.sh @@ -33,6 +33,7 @@ # Test the data plugin with base64= parameter. +source functions.sh set -e set -x @@ -52,21 +53,8 @@ if ! qemu-io --help >/dev/null; then fi # Run nbdkit. -nbdkit -P data-base64.pid -U data-base64.sock \ +start_nbdkit -P data-base64.pid -U data-base64.sock \ data base64=MTIz size=512 - -# We may have to wait a short time for the pid file to appear. -for i in `seq 1 10`; do - if test -f data-base64.pid; then - break - fi - sleep 1 -done -if ! test -f data-base64.pid; then - echo "$0: PID file was not created" - exit 1 -fi - pid="$(cat data-base64.pid)" # Kill the nbdkit process on exit. diff --git a/tests/test-data-raw.sh b/tests/test-data-raw.sh index c6652b7..d875777 100755 --- a/tests/test-data-raw.sh +++ b/tests/test-data-raw.sh @@ -33,6 +33,7 @@ # Test the data plugin with raw= parameter. +source functions.sh set -e set -x @@ -46,21 +47,8 @@ if ! qemu-io --help >/dev/null; then fi # Run nbdkit. -nbdkit -P data-raw.pid -U data-raw.sock \ +start_nbdkit -P data-raw.pid -U data-raw.sock \ data raw=123 size=512 - -# We may have to wait a short time for the pid file to appear. -for i in `seq 1 10`; do - if test -f data-raw.pid; then - break - fi - sleep 1 -done -if ! test -f data-raw.pid; then - echo "$0: PID file was not created" - exit 1 -fi - pid="$(cat data-raw.pid)" # Kill the nbdkit process on exit. diff --git a/tests/test-fua.sh b/tests/test-fua.sh index f95aa18..88c14b5 100755 --- a/tests/test-fua.sh +++ b/tests/test-fua.sh @@ -31,6 +31,7 @@ # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF # SUCH DAMAGE. +source functions.sh set -e set -x @@ -83,34 +84,22 @@ trap cleanup INT QUIT TERM EXIT ERR # 2: fuamode=emulate: log shows that blocksize optimizes fua to flush # 3: fuamode=native: log shows that blocksize preserves fua # 4: fuamode=force: log shows that fua is always enabled -nbdkit -P fua1.pid -U fua1.sock --filter=log --filter=fua \ - file logfile=fua1.log fua.img -nbdkit -P fua2.pid -U fua2.sock --filter=blocksize --filter=log --filter=fua \ - file logfile=fua2.log fua.img fuamode=emulate maxdata=4k maxlen=4k -nbdkit -P fua3.pid -U fua3.sock --filter=blocksize --filter=log --filter=fua \ - file logfile=fua3.log fua.img fuamode=native maxdata=4k maxlen=4k -nbdkit -P fua4.pid -U fua4.sock --filter=fua --filter=log \ - file logfile=fua4.log fua.img fuamode=force - -# We may have to wait a short time for the pid files to appear. -for i in `seq 1 10`; do - if test -f fua1.pid && test -f fua2.pid && test -f fua3.pid && - test -f fua4.pid; then - break - fi - sleep 1 -done - -pid1="$(cat fua1.pid)" || : -pid2="$(cat fua2.pid)" || : -pid3="$(cat fua3.pid)" || : -pid4="$(cat fua4.pid)" || : - -if ! test -f fua1.pid || ! test -f fua2.pid || ! test -f fua3.pid || - ! test -f fua4.pid; then - echo "$0: PID files were not created" - exit 1 -fi +start_nbdkit -P fua1.pid -U fua1.sock \ + --filter=log --filter=fua \ + file logfile=fua1.log fua.img +start_nbdkit -P fua2.pid -U fua2.sock \ + --filter=blocksize --filter=log --filter=fua \ + file logfile=fua2.log fua.img fuamode=emulate maxdata=4k maxlen=4k +start_nbdkit -P fua3.pid -U fua3.sock \ + --filter=blocksize --filter=log --filter=fua \ + file logfile=fua3.log fua.img fuamode=native maxdata=4k maxlen=4k +start_nbdkit -P fua4.pid -U fua4.sock \ + --filter=fua --filter=log \ + file logfile=fua4.log fua.img fuamode=force +pid1="$(cat fua1.pid)" +pid2="$(cat fua2.pid)" +pid3="$(cat fua3.pid)" +pid4="$(cat fua4.pid)" # Perform a flush, write, and zero, first without then with FUA for f in '' -f; do diff --git a/tests/test-ip.sh b/tests/test-ip.sh index 12448c4..10c6f88 100755 --- a/tests/test-ip.sh +++ b/tests/test-ip.sh @@ -34,8 +34,8 @@ # Every other test uses a Unix domain socket. This tests nbdkit over # IPv4 and IPv6 localhost connections. +source functions.sh set -e -source ./functions.sh rm -f ip.pid ipv4.out ipv6.out @@ -65,20 +65,7 @@ echo picked unused port $port # 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 ip.pid; then - break - fi - sleep 1 -done -if ! test -f ip.pid; then - echo "$0: PID file was not created" - exit 1 -fi - +start_nbdkit -P ip.pid -p $port example1 pid="$(cat ip.pid)" # Check the process exists. diff --git a/tests/test-log.sh b/tests/test-log.sh index f811de4..1cddb7b 100755 --- a/tests/test-log.sh +++ b/tests/test-log.sh @@ -31,6 +31,7 @@ # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF # SUCH DAMAGE. +source functions.sh set -e files="log.img log.log log.sock log.pid" @@ -44,20 +45,7 @@ if ! qemu-io -f raw -c 'w 1M 2M' log.img; then fi # Run nbdkit with logging enabled to file. -nbdkit -P log.pid -U log.sock --filter=log file log.img logfile=log.log - -# We may have to wait a short time for the pid file to appear. -for i in `seq 1 10`; do - if test -f log.pid; then - break - fi - sleep 1 -done -if ! test -f log.pid; then - echo "$0: PID file was not created" - exit 1 -fi - +start_nbdkit -P log.pid -U log.sock --filter=log file log.img logfile=log.log pid="$(cat log.pid)" # Kill the nbdkit process on exit. diff --git a/tests/test-memory-largest-for-qemu.sh b/tests/test-memory-largest-for-qemu.sh index 9bc498d..325919d 100755 --- a/tests/test-memory-largest-for-qemu.sh +++ b/tests/test-memory-largest-for-qemu.sh @@ -34,6 +34,7 @@ # Test the memory plugin with the largest possible size supported # by qemu and nbdkit. +source functions.sh set -e files="memory-largest-for-qemu.out memory-largest-for-qemu.pid memory-largest-for-qemu.sock" @@ -47,22 +48,8 @@ fi # Run nbdkit with memory plugin. # size = (2^63-1) & ~511 which is the largest supported by qemu. -nbdkit -f -v -D memory.dir=1 \ - -P memory-largest-for-qemu.pid -U memory-largest-for-qemu.sock \ - memory size=9223372036854775296 & - -# We may have to wait a short time for the pid file to appear. -for i in `seq 1 10`; do - if test -f memory-largest-for-qemu.pid; then - break - fi - sleep 1 -done -if ! test -f memory-largest-for-qemu.pid; then - echo "$0: PID file was not created" - exit 1 -fi - +start_nbdkit -P memory-largest-for-qemu.pid -U memory-largest-for-qemu.sock \ + memory size=9223372036854775296 pid="$(cat memory-largest-for-qemu.pid)" # Kill the nbdkit process on exit. diff --git a/tests/test-memory-largest.sh b/tests/test-memory-largest.sh index d0a6c77..b2115f7 100755 --- a/tests/test-memory-largest.sh +++ b/tests/test-memory-largest.sh @@ -34,6 +34,7 @@ # Test the memory plugin with the largest possible size supported # by nbdkit. +source functions.sh set -e files="memory-largest.out memory-largest.pid memory-largest.sock" @@ -47,23 +48,8 @@ fi # Run nbdkit with memory plugin. # size = 2^63-1 -nbdkit -f -v \ - -D memory.dir=1 \ - -P memory-largest.pid -U memory-largest.sock \ - memory size=9223372036854775807 & - -# We may have to wait a short time for the pid file to appear. -for i in `seq 1 10`; do - if test -f memory-largest.pid; then - break - fi - sleep 1 -done -if ! test -f memory-largest.pid; then - echo "$0: PID file was not created" - exit 1 -fi - +start_nbdkit -P memory-largest.pid -U memory-largest.sock \ + memory size=9223372036854775807 pid="$(cat memory-largest.pid)" # Kill the nbdkit process on exit. diff --git a/tests/test-nozero.sh b/tests/test-nozero.sh index 904d822..33456e1 100755 --- a/tests/test-nozero.sh +++ b/tests/test-nozero.sh @@ -31,6 +31,7 @@ # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF # SUCH DAMAGE. +source functions.sh set -e files="nozero1.img nozero1.log nozero1.sock nozero1.pid @@ -101,40 +102,24 @@ trap cleanup INT QUIT TERM EXIT ERR # 4: log after filter with zeromode=emulate, to ensure no ZERO to plugin # 5a/b: both sides of nbd plugin: even though server side does not advertise # ZERO, the client side still exposes it, and just skips calling nbd's .zero -nbdkit -P nozero1.pid -U nozero1.sock --filter=log \ +start_nbdkit -P nozero1.pid -U nozero1.sock --filter=log \ file logfile=nozero1.log nozero1.img -nbdkit -P nozero2.pid -U nozero2.sock --filter=log --filter=nozero \ +start_nbdkit -P nozero2.pid -U nozero2.sock --filter=log --filter=nozero \ file logfile=nozero2.log nozero2.img -nbdkit -P nozero3.pid -U nozero3.sock --filter=log --filter=nozero \ +start_nbdkit -P nozero3.pid -U nozero3.sock --filter=log --filter=nozero \ file logfile=nozero3.log nozero3.img zeromode=emulate -nbdkit -P nozero4.pid -U nozero4.sock --filter=nozero --filter=log \ +start_nbdkit -P nozero4.pid -U nozero4.sock --filter=nozero --filter=log \ file logfile=nozero4.log nozero4.img zeromode=emulate -nbdkit -P nozero5a.pid -U nozero5a.sock --filter=log --filter=nozero \ +start_nbdkit -P nozero5a.pid -U nozero5a.sock --filter=log --filter=nozero \ file logfile=nozero5a.log nozero5.img -nbdkit -P nozero5b.pid -U nozero5b.sock --filter=log \ +start_nbdkit -P nozero5b.pid -U nozero5b.sock --filter=log \ nbd logfile=nozero5b.log socket=nozero5a.sock - -# We may have to wait a short time for the pid files to appear. -for i in `seq 1 10`; do - if test -f nozero1.pid && test -f nozero2.pid && test -f nozero3.pid && - test -f nozero4.pid && test -f nozero5a.pid && test -f nozero5b.pid; then - break - fi - sleep 1 -done - -pid1="$(cat nozero1.pid)" || : -pid2="$(cat nozero2.pid)" || : -pid3="$(cat nozero3.pid)" || : -pid4="$(cat nozero4.pid)" || : -pid5a="$(cat nozero5a.pid)" || : -pid5b="$(cat nozero5b.pid)" || : - -if ! test -f nozero1.pid || ! test -f nozero2.pid || ! test -f nozero3.pid || - ! test -f nozero4.pid || ! test -f nozero5a.pid || ! test -f nozero5b.pid; then - echo "$0: PID files were not created" - exit 1 -fi +pid1="$(cat nozero1.pid)" +pid2="$(cat nozero2.pid)" +pid3="$(cat nozero3.pid)" +pid4="$(cat nozero4.pid)" +pid5a="$(cat nozero5a.pid)" +pid5b="$(cat nozero5b.pid)" # Perform the zero write. qemu-io -f raw -c 'w -z -u 0 1M' 'nbd+unix://?socket=nozero1.sock' diff --git a/tests/test-offset2.sh b/tests/test-offset2.sh index c37e060..ec1ad91 100755 --- a/tests/test-offset2.sh +++ b/tests/test-offset2.sh @@ -33,6 +33,7 @@ # Additional test of the offset filter using the pattern plugin. +source functions.sh set -e set -x @@ -47,23 +48,10 @@ fi # Run nbdkit with pattern plugin and offset filter in front. # 8070450532247927809 = 7E - 1023 -nbdkit -P offset2.pid -U offset2.sock \ +start_nbdkit -P offset2.pid -U offset2.sock \ --filter=offset \ pattern size=7E \ offset=8070450532247927809 range=512 - -# We may have to wait a short time for the pid file to appear. -for i in `seq 1 10`; do - if test -f offset2.pid; then - break - fi - sleep 1 -done -if ! test -f offset2.pid; then - echo "$0: PID file was not created" - exit 1 -fi - pid="$(cat offset2.pid)" # Kill the nbdkit process on exit. diff --git a/tests/test-parallel-nbd.sh b/tests/test-parallel-nbd.sh index cb70f46..223d953 100755 --- a/tests/test-parallel-nbd.sh +++ b/tests/test-parallel-nbd.sh @@ -31,6 +31,8 @@ # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF # SUCH DAMAGE. +source functions.sh + # Check file-data was created by Makefile and qemu-io exists. if ! test -f file-data; then echo "$0: missing file-data" @@ -61,21 +63,10 @@ qemu-io -f raw -c "aio_write -P 1 0 512" -c "aio_write -P 2 512 512" \ # may have spurious failures under heavy loads on the test machine, where # tuning the delays may help. -nbdkit --exit-with-parent -v \ +start_nbdkit --exit-with-parent -v \ -U test-parallel-nbd.sock -P test-parallel-nbd.pid \ --filter=delay \ file test-parallel-nbd.data wdelay=2 rdelay=1 & -# We may have to wait a short time for the pid file to appear. -for i in `seq 1 10`; do - if test -f test-parallel-nbd.pid; then - break - fi - sleep 1 -done -if ! test -f test-parallel-nbd.pid; then - echo "$0: PID file was not created" - exit 1 -fi # 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 ' diff --git a/tests/test-pattern-largest-for-qemu.sh b/tests/test-pattern-largest-for-qemu.sh index cf70354..0900f94 100755 --- a/tests/test-pattern-largest-for-qemu.sh +++ b/tests/test-pattern-largest-for-qemu.sh @@ -34,6 +34,7 @@ # Test the pattern plugin with the largest possible size supported # by qemu and nbdkit. +source functions.sh set -e files="pattern-largest-for-qemu.out pattern-largest-for-qemu.pid pattern-largest-for-qemu.sock" @@ -47,21 +48,8 @@ fi # Run nbdkit with pattern plugin. # size = (2^63-1) & ~511 which is the largest supported by qemu. -nbdkit -P pattern-largest-for-qemu.pid -U pattern-largest-for-qemu.sock \ +start_nbdkit -P pattern-largest-for-qemu.pid -U pattern-largest-for-qemu.sock \ pattern size=9223372036854775296 - -# We may have to wait a short time for the pid file to appear. -for i in `seq 1 10`; do - if test -f pattern-largest-for-qemu.pid; then - break - fi - sleep 1 -done -if ! test -f pattern-largest-for-qemu.pid; then - echo "$0: PID file was not created" - exit 1 -fi - pid="$(cat pattern-largest-for-qemu.pid)" # Kill the nbdkit process on exit. diff --git a/tests/test-pattern-largest.sh b/tests/test-pattern-largest.sh index 2a903bf..8accc14 100755 --- a/tests/test-pattern-largest.sh +++ b/tests/test-pattern-largest.sh @@ -34,6 +34,7 @@ # Test the pattern plugin with the largest possible size supported # by nbdkit. +source functions.sh set -e files="pattern-largest.out pattern-largest.pid pattern-largest.sock" @@ -47,21 +48,8 @@ fi # Run nbdkit with pattern plugin. # size = 2^63-1 -nbdkit -P pattern-largest.pid -U pattern-largest.sock \ +start_nbdkit -P pattern-largest.pid -U pattern-largest.sock \ pattern size=9223372036854775807 - -# We may have to wait a short time for the pid file to appear. -for i in `seq 1 10`; do - if test -f pattern-largest.pid; then - break - fi - sleep 1 -done -if ! test -f pattern-largest.pid; then - echo "$0: PID file was not created" - exit 1 -fi - pid="$(cat pattern-largest.pid)" # Kill the nbdkit process on exit. diff --git a/tests/test-pattern.sh b/tests/test-pattern.sh index 68022c9..4ff2996 100755 --- a/tests/test-pattern.sh +++ b/tests/test-pattern.sh @@ -38,6 +38,7 @@ # what read parameters we give it. Hence these tests are rather # limited. (XXX) +source functions.sh set -e files="pattern.out pattern.pid pattern.sock" @@ -50,20 +51,7 @@ if ! qemu-io --help >/dev/null; then fi # Run nbdkit with pattern plugin. -nbdkit -P pattern.pid -U pattern.sock pattern size=1G - -# We may have to wait a short time for the pid file to appear. -for i in `seq 1 10`; do - if test -f pattern.pid; then - break - fi - sleep 1 -done -if ! test -f pattern.pid; then - echo "$0: PID file was not created" - exit 1 -fi - +start_nbdkit -P pattern.pid -U pattern.sock pattern size=1G pid="$(cat pattern.pid)" # Kill the nbdkit process on exit. diff --git a/tests/test-start.sh b/tests/test-start.sh index dc578e7..e866688 100755 --- a/tests/test-start.sh +++ b/tests/test-start.sh @@ -31,29 +31,16 @@ # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF # SUCH DAMAGE. +source functions.sh set -e set -x -source ./functions.sh # Test nbdkit starts up, forks in the background, writes a PID file, # and can be killed. rm -f start.pid start.sock -nbdkit -P start.pid -U start.sock 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 start.pid; then - break - fi - sleep 1 -done -if ! test -f start.pid; then - echo "$0: PID file was not created" - exit 1 -fi - +start_nbdkit -P start.pid -U start.sock example1 pid="$(cat start.pid)" # Check the process exists. diff --git a/tests/test-tls-psk.sh b/tests/test-tls-psk.sh index 99c5945..f561588 100755 --- a/tests/test-tls-psk.sh +++ b/tests/test-tls-psk.sh @@ -31,9 +31,9 @@ # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF # SUCH DAMAGE. +source functions.sh set -e set -x -source ./functions.sh # Don't fail if certain commands aren't available. if ! ss --version; then @@ -78,20 +78,8 @@ for port in `seq 51000 65535`; do done echo picked unused port $port -nbdkit -P tls-psk.pid -p $port -n --tls=require --tls-psk=keys.psk 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 tls-psk.pid; then - break - fi - sleep 1 -done -if ! test -f tls-psk.pid; then - echo "$0: PID file was not created" - exit 1 -fi - +start_nbdkit -P tls-psk.pid -p $port -n \ + --tls=require --tls-psk=keys.psk example1 pid="$(cat tls-psk.pid)" # Kill the process on exit. diff --git a/tests/test-tls.sh b/tests/test-tls.sh index 71bd8a4..79f08d3 100755 --- a/tests/test-tls.sh +++ b/tests/test-tls.sh @@ -31,9 +31,9 @@ # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF # SUCH DAMAGE. +source functions.sh set -e set -x -source ./functions.sh # Don't fail if certain commands aren't available. if ! ss --version; then @@ -71,21 +71,8 @@ for port in `seq 50000 65535`; do done echo picked unused port $port -nbdkit -P tls.pid -p $port -n --tls=require \ +start_nbdkit -P tls.pid -p $port -n --tls=require \ --tls-certificates="$pkidir" 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 tls.pid; then - break - fi - sleep 1 -done -if ! test -f tls.pid; then - echo "$0: PID file was not created" - exit 1 -fi - pid="$(cat tls.pid)" # Kill the process on exit. diff --git a/tests/test-truncate1.sh b/tests/test-truncate1.sh index 719f63b..fb5ef65 100755 --- a/tests/test-truncate1.sh +++ b/tests/test-truncate1.sh @@ -33,6 +33,7 @@ # Test the truncate filter using the pattern plugin. +source functions.sh set -e set -x @@ -46,23 +47,10 @@ if ! qemu-io --help >/dev/null; then fi # Run nbdkit with pattern plugin and truncate filter in front. -nbdkit -P truncate1.pid -U truncate1.sock \ +start_nbdkit -P truncate1.pid -U truncate1.sock \ --filter=truncate \ pattern size=503 \ truncate=512 - -# We may have to wait a short time for the pid file to appear. -for i in `seq 1 10`; do - if test -f truncate1.pid; then - break - fi - sleep 1 -done -if ! test -f truncate1.pid; then - echo "$0: PID file was not created" - exit 1 -fi - pid="$(cat truncate1.pid)" # Kill the nbdkit process on exit. diff --git a/tests/test-truncate2.sh b/tests/test-truncate2.sh index 0295b96..6dd7ba0 100755 --- a/tests/test-truncate2.sh +++ b/tests/test-truncate2.sh @@ -33,6 +33,7 @@ # Test the truncate filter using the pattern plugin. +source functions.sh set -e set -x @@ -46,23 +47,10 @@ if ! qemu-io --help >/dev/null; then fi # Run nbdkit with pattern plugin and truncate filter in front. -nbdkit -P truncate2.pid -U truncate2.sock \ +start_nbdkit -P truncate2.pid -U truncate2.sock \ --filter=truncate \ pattern size=503 \ round-up=512 - -# We may have to wait a short time for the pid file to appear. -for i in `seq 1 10`; do - if test -f truncate2.pid; then - break - fi - sleep 1 -done -if ! test -f truncate2.pid; then - echo "$0: PID file was not created" - exit 1 -fi - pid="$(cat truncate2.pid)" # Kill the nbdkit process on exit. -- 2.19.0.rc0
On 9/11/18 1:47 PM, Richard W.M. Jones wrote:
This assumes bashisms, but bash is required to run the tests.
This is mostly simple refactoring. Except for the test-memory*.sh tests where nbdkit used to run in the foreground, but that seems to be a consequence of some left over debugging. ---
+++ b/tests/functions.sh.in @@ -32,6 +32,41 @@ # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF # SUCH DAMAGE.
+# start_nbdkit args... +# +# Run nbdkit with args and wait for it to start up. If it fails +# to start up, exit with an error message. +start_nbdkit () +{ + # Find the -P <pidfile> argument. + pidfile= + for i in `seq 1 $#`; do
Is seq universally available, even on BSD? It's not in POSIX. Since you're already using bashisms, you could instead do for (( i=1; i <= $#; i++)); do or even: for i in {1..$#}; do and drop the need for seq or even a fork (the former works regardless of $#, the latter has poor scaling effects if $# is large since it is expanded in advance - but then again, none of our tests pass that large of $#)
+ if [ "${!i}" = "-P" ]; then
Unspecified behavior if ${!i} evaluates to "!" (which none of the tests do). Since you're already using bashisms, you could instead do if [[ ${!i} == -P ]]; then
+ j=$((i+1)) + pidfile="${!j}" + fi + done
This parse loop requires tests to spell it "-P /path/to/file" as two separate args, rather than also permitting "-P/path/to/file". A reasonable restriction but might be worth a comment.
+ if [ "$pidfile" = "" ]; then + echo "$0: start_nbdkit: -P option not present on nbdkit command line" + exit 1 + fi + + # Run nbdkit. + nbdkit "$@"
You could also change the contract of this function to REQUIRE the pid filename be first, so that you don't have to hunt over $# where -P lives, something like: start_nbdkit() { pidfile=$1 shift nbdkit -P $pidfile "$@" and change callers to look like: start_nbdkit foo.pid --myoption1 myplugin ...
+ + # Wait for the pidfile to appear. + for i in `seq 1 10`; do
I guess we were already relying on seq. Could also be spelled {1..10}
+ if test -f "$pidfile"; then + break + fi + sleep 1 + done + if ! test -f "$pidfile"; then + echo "$0: start_nbdkit: PID file $pidfile was not created" + exit 1 + fi +} +
+++ b/tests/test-blocksize.sh @@ -31,6 +31,7 @@ # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF # SUCH DAMAGE.
+source functions.sh set -e
More need for source ./functions.sh throughout the patch
files="blocksize1.img blocksize1.log blocksize1.sock blocksize1.pid @@ -72,27 +73,13 @@ cleanup () trap cleanup INT QUIT TERM EXIT ERR
# Run two parallel nbdkit; to compare the logs and see what changes. -nbdkit -P blocksize1.pid -U blocksize1.sock \ +start_nbdkit -P blocksize1.pid -U blocksize1.sock \ --filter=log file logfile=blocksize1.log blocksize1.img -nbdkit -P blocksize2.pid -U blocksize2.sock --filter=blocksize \ +start_nbdkit -P blocksize2.pid -U blocksize2.sock --filter=blocksize \ --filter=log file logfile=blocksize2.log blocksize2.img \ minblock=1024 maxdata=512k maxlen=1M
If the first process starts but the second fails, then you have not set pid1=, and leak the first process. This is a regression from the old code, which managed to capture pid1 before triggering the trap to cleanup(), and thus killed the successful nbdkit. Several tests are impacted. Less importantly, the old code performed startup in parallel (kick off both nbdkit processes, then start the loop; if a pid file was not produced by either one of the nbdkit processes, the test determined that within 10 seconds); now startup is serial (you don't start a second nbdkit until the first one has succeeded). For a test with two nbdkit processes, that could expand typical test time from 1 second to 2 (if the pid file creation is not instantaneous, but the first sleep 1 was sufficient for both processes to be ready); and expand worst-case failure detection under heavy load from 10 into 20 seconds (if the first succeeded on the last iteration, but the second timed out). I don't see anything wrong with the change, although it might be worth a mention in the commit message as intentional. I'm also seeing a common pattern of assigning pid=$(cat file.pid) right after calling start_nbdkt. Would it be worth changing the signature of start_nbdkit() to also populate a pid variable on success, as in: start_nbdkit blocksize1.pid pid1 myarg1 start_nbdkit() { pidfile=$1 pidvar=$2 shift 2 ... pid=$(cat $pidfile) eval $pidvar=$pid which would also have the added benefit of avoiding the regression of pid1 not being set if the second nbdkit process doesn't start? But overall, I like the idea of refactoring the common code for less repetition. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
On Tue, Sep 11, 2018 at 03:02:47PM -0500, Eric Blake wrote: [Lots of issues] I'll fix all these, thanks.
files="blocksize1.img blocksize1.log blocksize1.sock blocksize1.pid @@ -72,27 +73,13 @@ cleanup () trap cleanup INT QUIT TERM EXIT ERR # Run two parallel nbdkit; to compare the logs and see what changes. -nbdkit -P blocksize1.pid -U blocksize1.sock \ +start_nbdkit -P blocksize1.pid -U blocksize1.sock \ --filter=log file logfile=blocksize1.log blocksize1.img -nbdkit -P blocksize2.pid -U blocksize2.sock --filter=blocksize \ +start_nbdkit -P blocksize2.pid -U blocksize2.sock --filter=blocksize \ --filter=log file logfile=blocksize2.log blocksize2.img \ minblock=1024 maxdata=512k maxlen=1M
If the first process starts but the second fails, then you have not set pid1=, and leak the first process. This is a regression from the old code, which managed to capture pid1 before triggering the trap to cleanup(), and thus killed the successful nbdkit. Several tests are impacted.
So one other idea I had is to have start_nbdkit create a trap function to clean up the nbdkit process (we'd still need trap functions in the main program for deleting temporary files). I don't know if this would work, but if it does it would fix the above problem.
Less importantly, the old code performed startup in parallel (kick off both nbdkit processes, then start the loop; if a pid file was not produced by either one of the nbdkit processes, the test determined that within 10 seconds); now startup is serial (you don't start a second nbdkit until the first one has succeeded). For a test with two nbdkit processes, that could expand typical test time from 1 second to 2 (if the pid file creation is not instantaneous, but the first sleep 1 was sufficient for both processes to be ready); and expand worst-case failure detection under heavy load from 10 into 20 seconds (if the first succeeded on the last iteration, but the second timed out). I don't see anything wrong with the change, although it might be worth a mention in the commit message as intentional.
I'm not too worried about this since it should cause only a minor increase in time.
I'm also seeing a common pattern of assigning pid=$(cat file.pid) right after calling start_nbdkt. Would it be worth changing the signature of start_nbdkit() to also populate a pid variable on success, as in:
start_nbdkit blocksize1.pid pid1 myarg1
start_nbdkit() { pidfile=$1 pidvar=$2 shift 2 ... pid=$(cat $pidfile) eval $pidvar=$pid
It was nice that start_nbdkit has the same "signature" as a regular nbdkit command.
which would also have the added benefit of avoiding the regression of pid1 not being set if the second nbdkit process doesn't start?
But overall, I like the idea of refactoring the common code for less repetition.
I'll have a think about the trap function stuff, but most likely tomorrow. 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
On 9/11/18 4:14 PM, Richard W.M. Jones wrote:
If the first process starts but the second fails, then you have not set pid1=, and leak the first process. This is a regression from the old code, which managed to capture pid1 before triggering the trap to cleanup(), and thus killed the successful nbdkit. Several tests are impacted.
So one other idea I had is to have start_nbdkit create a trap function to clean up the nbdkit process (we'd still need trap functions in the main program for deleting temporary files). I don't know if this would work, but if it does it would fix the above problem.
Calling 'trap' more than once overwrites the previous version of the trap; it's rather awkward to query trap to see what is already installed in order to append to that before reinstalling a new trap. But you COULD have sourcing the common stuff unconditionally install a generic trap handler: declare -a cleanup_hook trap cleanup INT QUIT TERM EXIT ERR cleanup() { for (( i=0; i < ${#cleanup_hook[@]}; i++ )); do ${cleanup_hook[i]} done } # Appends all arguments as a new command on the list of actions run at script exit cleanup_append() { cleanup_hook[${#cleanup_hook[@]}]="$@" } then have the various tests do: cleanup_append myfunc arg without having to call trap again. The arguments to cleanup_append will undergo word splitting during the cleanup function execution, but it's not a full eval so you can't rely on other shell metacharacters. Then again, because you can append shell function names with arguments as one of the commands on the list, it's fairly easy to encapsulate any test-specific cleanup in a function. In the case of start_nbdkit, it would mean you set up a secondary function that takes a pid file as a name, then call 'cleanup_append helper $pidfile' at the appropriate point, where only the helper function has to cat $pidfile if it exists. Then the test files won't have to track pid1= variables. So that looks even nicer than my idea of passing a variable name:
I'm also seeing a common pattern of assigning pid=$(cat file.pid) right after calling start_nbdkt. Would it be worth changing the signature of start_nbdkit() to also populate a pid variable on success, as in:
start_nbdkit blocksize1.pid pid1 myarg1
start_nbdkit() { pidfile=$1 pidvar=$2 shift 2 ... pid=$(cat $pidfile) eval $pidvar=$pid
It was nice that start_nbdkit has the same "signature" as a regular nbdkit command.
True, you do have that aspect, so continuing to hunt for -P may still be worthwhile, especially if the cleanup hook idea means you don't need to track a pid= variable. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
participants (2)
-
Eric Blake -
Richard W.M. Jones