diff mbox series

[v5,2/6] iotests: exclude killed processes from running under Valgrind

Message ID 1563553816-148827-3-git-send-email-andrey.shinkevich@virtuozzo.com
State New
Headers show
Series Allow Valgrind checking all QEMU processes | expand

Commit Message

Andrey Shinkevich July 19, 2019, 4:30 p.m. UTC
The Valgrind tool fails to manage its termination when QEMU raises the
 signal SIGKILL in the multi-threaded process. The bug has been
 reported to the Valgrind maintainers and was registered as Bug 409141.
 Let's exclude such test cases from running under the Valgrind until
 new release of it because checking for the memory issues is covered by
 other test cases.

Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
 tests/qemu-iotests/039 | 5 +++++
 tests/qemu-iotests/061 | 2 ++
 tests/qemu-iotests/137 | 1 +
 3 files changed, 8 insertions(+)

Comments

John Snow Aug. 16, 2019, 12:40 a.m. UTC | #1
On 7/19/19 12:30 PM, Andrey Shinkevich wrote:
>  The Valgrind tool fails to manage its termination when QEMU raises the
>  signal SIGKILL in the multi-threaded process. The bug has been
>  reported to the Valgrind maintainers and was registered as Bug 409141.
>  Let's exclude such test cases from running under the Valgrind until
>  new release of it because checking for the memory issues is covered by
>  other test cases.
> 

Link to the bug in the commit message, please:
https://bugs.kde.org/show_bug.cgi?id=409141

...Hey! It looks like they may have fixed it. However, we don't know if
the user has a properly fixed version of valgrind or not.

How long do we keep these workarounds in-tree?

> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---
>  tests/qemu-iotests/039 | 5 +++++
>  tests/qemu-iotests/061 | 2 ++
>  tests/qemu-iotests/137 | 1 +
>  3 files changed, 8 insertions(+)
> 
> diff --git a/tests/qemu-iotests/039 b/tests/qemu-iotests/039
> index 0d4e963..95115e2 100755
> --- a/tests/qemu-iotests/039
> +++ b/tests/qemu-iotests/039
> @@ -65,6 +65,7 @@ echo "== Creating a dirty image file =="
>  IMGOPTS="compat=1.1,lazy_refcounts=on"
>  _make_test_img $size
>  
> +VALGRIND_QEMU="" \
>  $QEMU_IO -c "write -P 0x5a 0 512" \
>           -c "sigraise $(kill -l KILL)" "$TEST_IMG" 2>&1 \
>      | _filter_qemu_io
> @@ -100,6 +101,7 @@ echo "== Opening a dirty image read/write should repair it =="
>  IMGOPTS="compat=1.1,lazy_refcounts=on"
>  _make_test_img $size
>  

This seems like the sort of thing that's going to get forgotten about
quickly. It's not clear (if you're reading the test source) why we're
setting VALGRIND_QEMU="" before these invocations.

Is it possible to create some kind of _NO_VALGRIND() shim that has some
comment in it, like:

# Valgrind bug #409141 https://bugs.kde.org/show_bug.cgi?id=409141
# Until valgrind 3.16+ is ubiquitous, we must work around a hang in
# valgrind when issuing sigkill. Disable valgrind for this invocation.

VALGRIND_QEMU="" exec "$@"

(something like that.)

This way:

(1) The workaround is being clearly demonstrated as a bit of a hack
(2) The shim explains what the hack is for
(3) When we decide we no longer need the hack, we can pull it out of a
central location and easily grep to find callers of the shim.


> +VALGRIND_QEMU="" \
>  $QEMU_IO -c "write -P 0x5a 0 512" \
>           -c "sigraise $(kill -l KILL)" "$TEST_IMG" 2>&1 \
>      | _filter_qemu_io
> @@ -118,6 +120,7 @@ echo "== Creating an image file with lazy_refcounts=off =="
>  IMGOPTS="compat=1.1,lazy_refcounts=off"
>  _make_test_img $size
>  
> +VALGRIND_QEMU="" \
>  $QEMU_IO -c "write -P 0x5a 0 512" \
>           -c "sigraise $(kill -l KILL)" "$TEST_IMG" 2>&1 \
>      | _filter_qemu_io
> @@ -151,6 +154,7 @@ echo "== Changing lazy_refcounts setting at runtime =="
>  IMGOPTS="compat=1.1,lazy_refcounts=off"
>  _make_test_img $size
>  
> +VALGRIND_QEMU="" \
>  $QEMU_IO -c "reopen -o lazy-refcounts=on" \
>           -c "write -P 0x5a 0 512" \
>           -c "sigraise $(kill -l KILL)" "$TEST_IMG" 2>&1 \
> @@ -163,6 +167,7 @@ _check_test_img
>  IMGOPTS="compat=1.1,lazy_refcounts=on"
>  _make_test_img $size
>  
> +VALGRIND_QEMU="" \
>  $QEMU_IO -c "reopen -o lazy-refcounts=off" \
>           -c "write -P 0x5a 0 512" \
>           -c "sigraise $(kill -l KILL)" "$TEST_IMG" 2>&1 \
> diff --git a/tests/qemu-iotests/061 b/tests/qemu-iotests/061
> index d7dbd7e..5d0724c 100755
> --- a/tests/qemu-iotests/061
> +++ b/tests/qemu-iotests/061
> @@ -73,6 +73,7 @@ echo
>  echo "=== Testing dirty version downgrade ==="
>  echo
>  IMGOPTS="compat=1.1,lazy_refcounts=on" _make_test_img 64M
> +VALGRIND_QEMU="" \
>  $QEMU_IO -c "write -P 0x2a 0 128k" -c flush \
>           -c "sigraise $(kill -l KILL)" "$TEST_IMG" 2>&1 | _filter_qemu_io
>  $PYTHON qcow2.py "$TEST_IMG" dump-header
> @@ -107,6 +108,7 @@ echo
>  echo "=== Testing dirty lazy_refcounts=off ==="
>  echo
>  IMGOPTS="compat=1.1,lazy_refcounts=on" _make_test_img 64M
> +VALGRIND_QEMU="" \
>  $QEMU_IO -c "write -P 0x2a 0 128k" -c flush \
>           -c "sigraise $(kill -l KILL)" "$TEST_IMG" 2>&1 | _filter_qemu_io
>  $PYTHON qcow2.py "$TEST_IMG" dump-header
> diff --git a/tests/qemu-iotests/137 b/tests/qemu-iotests/137
> index 0c3d2a1..a442fc8 100755
> --- a/tests/qemu-iotests/137
> +++ b/tests/qemu-iotests/137
> @@ -130,6 +130,7 @@ echo
>  
>  # Whether lazy-refcounts was actually enabled can easily be tested: Check if
>  # the dirty bit is set after a crash
> +VALGRIND_QEMU="" \
>  $QEMU_IO \
>      -c "reopen -o lazy-refcounts=on,overlap-check=blubb" \
>      -c "write -P 0x5a 0 512" \
>
diff mbox series

Patch

diff --git a/tests/qemu-iotests/039 b/tests/qemu-iotests/039
index 0d4e963..95115e2 100755
--- a/tests/qemu-iotests/039
+++ b/tests/qemu-iotests/039
@@ -65,6 +65,7 @@  echo "== Creating a dirty image file =="
 IMGOPTS="compat=1.1,lazy_refcounts=on"
 _make_test_img $size
 
+VALGRIND_QEMU="" \
 $QEMU_IO -c "write -P 0x5a 0 512" \
          -c "sigraise $(kill -l KILL)" "$TEST_IMG" 2>&1 \
     | _filter_qemu_io
@@ -100,6 +101,7 @@  echo "== Opening a dirty image read/write should repair it =="
 IMGOPTS="compat=1.1,lazy_refcounts=on"
 _make_test_img $size
 
+VALGRIND_QEMU="" \
 $QEMU_IO -c "write -P 0x5a 0 512" \
          -c "sigraise $(kill -l KILL)" "$TEST_IMG" 2>&1 \
     | _filter_qemu_io
@@ -118,6 +120,7 @@  echo "== Creating an image file with lazy_refcounts=off =="
 IMGOPTS="compat=1.1,lazy_refcounts=off"
 _make_test_img $size
 
+VALGRIND_QEMU="" \
 $QEMU_IO -c "write -P 0x5a 0 512" \
          -c "sigraise $(kill -l KILL)" "$TEST_IMG" 2>&1 \
     | _filter_qemu_io
@@ -151,6 +154,7 @@  echo "== Changing lazy_refcounts setting at runtime =="
 IMGOPTS="compat=1.1,lazy_refcounts=off"
 _make_test_img $size
 
+VALGRIND_QEMU="" \
 $QEMU_IO -c "reopen -o lazy-refcounts=on" \
          -c "write -P 0x5a 0 512" \
          -c "sigraise $(kill -l KILL)" "$TEST_IMG" 2>&1 \
@@ -163,6 +167,7 @@  _check_test_img
 IMGOPTS="compat=1.1,lazy_refcounts=on"
 _make_test_img $size
 
+VALGRIND_QEMU="" \
 $QEMU_IO -c "reopen -o lazy-refcounts=off" \
          -c "write -P 0x5a 0 512" \
          -c "sigraise $(kill -l KILL)" "$TEST_IMG" 2>&1 \
diff --git a/tests/qemu-iotests/061 b/tests/qemu-iotests/061
index d7dbd7e..5d0724c 100755
--- a/tests/qemu-iotests/061
+++ b/tests/qemu-iotests/061
@@ -73,6 +73,7 @@  echo
 echo "=== Testing dirty version downgrade ==="
 echo
 IMGOPTS="compat=1.1,lazy_refcounts=on" _make_test_img 64M
+VALGRIND_QEMU="" \
 $QEMU_IO -c "write -P 0x2a 0 128k" -c flush \
          -c "sigraise $(kill -l KILL)" "$TEST_IMG" 2>&1 | _filter_qemu_io
 $PYTHON qcow2.py "$TEST_IMG" dump-header
@@ -107,6 +108,7 @@  echo
 echo "=== Testing dirty lazy_refcounts=off ==="
 echo
 IMGOPTS="compat=1.1,lazy_refcounts=on" _make_test_img 64M
+VALGRIND_QEMU="" \
 $QEMU_IO -c "write -P 0x2a 0 128k" -c flush \
          -c "sigraise $(kill -l KILL)" "$TEST_IMG" 2>&1 | _filter_qemu_io
 $PYTHON qcow2.py "$TEST_IMG" dump-header
diff --git a/tests/qemu-iotests/137 b/tests/qemu-iotests/137
index 0c3d2a1..a442fc8 100755
--- a/tests/qemu-iotests/137
+++ b/tests/qemu-iotests/137
@@ -130,6 +130,7 @@  echo
 
 # Whether lazy-refcounts was actually enabled can easily be tested: Check if
 # the dirty bit is set after a crash
+VALGRIND_QEMU="" \
 $QEMU_IO \
     -c "reopen -o lazy-refcounts=on,overlap-check=blubb" \
     -c "write -P 0x5a 0 512" \