Libguestfs - Aug 2018 - [PATCH nbdkit] tests: Cancel trap in cleanup function to avoid recursive traps.

In these test functions, when a test fails, the bash ‘trap’ command
causes the cleanup function to be called.  However bash does not annul
or cancel the traps when cleanup is called, so at the end of the
cleanup function when ‘exit’ is called, cleanup is called recursively.

Avoid this by cancelling the traps at the top of the cleanup function.
Also an extra debugging message is emitted here giving the value of
$status which can be useful.
---
 tests/test-blocksize.sh       |  2 ++
 tests/test-cache.sh           |  2 ++
 tests/test-cow.sh             |  2 ++
 tests/test-fua.sh             |  2 ++
 tests/test-log.sh             |  2 ++
 tests/test-nozero.sh          |  2 ++
 tests/test-offset2.sh         |  2 ++
 tests/test-pattern-largest.sh |  2 ++
 tests/test-pattern.sh         |  2 ++
 tests/test-single.sh          |  2 ++
 tests/test-tls-psk.sh         |  2 ++
 tests/test-tls.sh             |  2 ++
 tests/test-zero.sh            | 13 ++++++++++++-
 13 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/tests/test-blocksize.sh b/tests/test-blocksize.sh
index 9cadf30..e008f5e 100755
--- a/tests/test-blocksize.sh
+++ b/tests/test-blocksize.sh
@@ -55,6 +55,8 @@ pid1= pid2 cleanup ()
 {
     status=$?
+    trap '' INT QUIT TERM EXIT ERR
+    echo $0: cleanup: exit code $status
 
     test "$pid1" && kill $pid1
     test "$pid2" && kill $pid2
diff --git a/tests/test-cache.sh b/tests/test-cache.sh
index c46e7be..da1f6d3 100755
--- a/tests/test-cache.sh
+++ b/tests/test-cache.sh
@@ -61,6 +61,8 @@ pid="$(cat cache.pid)"
 cleanup ()
 {
     status=$?
+    trap '' INT QUIT TERM EXIT ERR
+    echo $0: cleanup: exit code $status
 
     kill $pid
     rm -f $files
diff --git a/tests/test-cow.sh b/tests/test-cow.sh
index 01d2d04..7211958 100755
--- a/tests/test-cow.sh
+++ b/tests/test-cow.sh
@@ -62,6 +62,8 @@ pid="$(cat cow.pid)"
 cleanup ()
 {
     status=$?
+    trap '' INT QUIT TERM EXIT ERR
+    echo $0: cleanup: exit code $status
 
     kill $pid
     rm -f $files
diff --git a/tests/test-fua.sh b/tests/test-fua.sh
index 0ec9ef0..8a8c7fc 100755
--- a/tests/test-fua.sh
+++ b/tests/test-fua.sh
@@ -56,6 +56,8 @@ pid1= pid2= pid3= pid4 cleanup ()
 {
     status=$?
+    trap '' INT QUIT TERM EXIT ERR
+    echo $0: cleanup: exit code $status
 
     test "$pid1" && kill $pid1
     test "$pid2" && kill $pid2
diff --git a/tests/test-log.sh b/tests/test-log.sh
index 94d3960..8948c2c 100755
--- a/tests/test-log.sh
+++ b/tests/test-log.sh
@@ -64,6 +64,8 @@ pid="$(cat log.pid)"
 cleanup ()
 {
     status=$?
+    trap '' INT QUIT TERM EXIT ERR
+    echo $0: cleanup: exit code $status
 
     kill $pid
     # For easier debugging, dump the final log file before removing it.
diff --git a/tests/test-nozero.sh b/tests/test-nozero.sh
index 57c4452..77fda0b 100755
--- a/tests/test-nozero.sh
+++ b/tests/test-nozero.sh
@@ -66,6 +66,8 @@ pid1= pid2= pid3= pid4= pid5a= pid5b cleanup ()
 {
     status=$?
+    trap '' INT QUIT TERM EXIT ERR
+    echo $0: cleanup: exit code $status
 
     test "$pid1" && kill $pid1
     test "$pid2" && kill $pid2
diff --git a/tests/test-offset2.sh b/tests/test-offset2.sh
index a4f44cc..78bb690 100755
--- a/tests/test-offset2.sh
+++ b/tests/test-offset2.sh
@@ -70,6 +70,8 @@ pid="$(cat offset2.pid)"
 cleanup ()
 {
     status=$?
+    trap '' INT QUIT TERM EXIT ERR
+    echo $0: cleanup: exit code $status
 
     kill $pid
     rm -f $files
diff --git a/tests/test-pattern-largest.sh b/tests/test-pattern-largest.sh
index 5ff891f..9789675 100755
--- a/tests/test-pattern-largest.sh
+++ b/tests/test-pattern-largest.sh
@@ -68,6 +68,8 @@ pid="$(cat pattern-largest.pid)"
 cleanup ()
 {
     status=$?
+    trap '' INT QUIT TERM EXIT ERR
+    echo $0: cleanup: exit code $status
 
     kill $pid
     rm -f $files
diff --git a/tests/test-pattern.sh b/tests/test-pattern.sh
index 2db4e0c..c6e605d 100755
--- a/tests/test-pattern.sh
+++ b/tests/test-pattern.sh
@@ -70,6 +70,8 @@ pid="$(cat pattern.pid)"
 cleanup ()
 {
     status=$?
+    trap '' INT QUIT TERM EXIT ERR
+    echo $0: cleanup: exit code $status
 
     kill $pid
     rm -f $files
diff --git a/tests/test-single.sh b/tests/test-single.sh
index fdcfc62..b4aefcc 100755
--- a/tests/test-single.sh
+++ b/tests/test-single.sh
@@ -57,6 +57,8 @@ pid=$!
 cleanup ()
 {
     status=$?
+    trap '' INT QUIT TERM EXIT ERR
+    echo $0: cleanup: exit code $status
 
     kill $pid
     rm -f $files
diff --git a/tests/test-tls-psk.sh b/tests/test-tls-psk.sh
index 4f8111a..e7954bb 100755
--- a/tests/test-tls-psk.sh
+++ b/tests/test-tls-psk.sh
@@ -98,6 +98,8 @@ pid="$(cat tls-psk.pid)"
 cleanup ()
 {
     status=$?
+    trap '' INT QUIT TERM EXIT ERR
+    echo $0: cleanup: exit code $status
 
     kill $pid
     rm -f tls-psk.pid tls-psk.out
diff --git a/tests/test-tls.sh b/tests/test-tls.sh
index 7a6c949..dcf34a2 100755
--- a/tests/test-tls.sh
+++ b/tests/test-tls.sh
@@ -92,6 +92,8 @@ pid="$(cat tls.pid)"
 cleanup ()
 {
     status=$?
+    trap '' INT QUIT TERM EXIT ERR
+    echo $0: cleanup: exit code $status
 
     kill $pid
     rm -f tls.pid tls.out
diff --git a/tests/test-zero.sh b/tests/test-zero.sh
index b590345..1f4549c 100755
--- a/tests/test-zero.sh
+++ b/tests/test-zero.sh
@@ -42,7 +42,18 @@ fi
 
 files="test-zero.out"
 rm -f $files
-trap "rm $files" INT QUIT TERM EXIT ERR
+
+cleanup ()
+{
+    status=$?
+    trap '' INT QUIT TERM EXIT ERR
+    echo $0: cleanup: exit code $status
+
+    rm $files
+
+    exit $status
+}
+trap cleanup INT QUIT TERM EXIT ERR
 
 nbdkit -U - zero --run 'qemu-img convert $nbd test-zero.out'
 
-- 
2.18.0

On 08/01/2018 08:54 AM, Richard W.M. Jones wrote:
> In these test functions, when a test fails, the bash ‘trap’ command
> causes the cleanup function to be called.  However bash does not annul
> or cancel the traps when cleanup is called, so at the end of the
> cleanup function when ‘exit’ is called, cleanup is called recursively.
> 
> Avoid this by cancelling the traps at the top of the cleanup function.
> Also an extra debugging message is emitted here giving the value of
> $status which can be useful.
ACK

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