[Qemu-devel] [PATCH] iotests: disable core dumps in test 061

[Qemu-devel] [PATCH] iotests: disable core dumps in test 061

[Alberto Garcia]

Commit 934659c460 disabled the supression of segmentation faults in
bash tests. The new output of test 061, however, assumes that a core
dump will be produced if a program aborts. This is not necessarily the
case because core dumps can be disabled using ulimit.

We cannot guarantee that core dumps can be enabled in all cases, so we
should disable them completely and update the test output accordingly.

Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 tests/qemu-iotests/061     | 3 +++
 tests/qemu-iotests/061.out | 4 ++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/061 b/tests/qemu-iotests/061
index 1df887a..7d8415b 100755
--- a/tests/qemu-iotests/061
+++ b/tests/qemu-iotests/061
@@ -43,6 +43,9 @@ _supported_fmt qcow2
 _supported_proto file
 _supported_os Linux
 
+# Disable core dumps or they'll mess up the test output
+ulimit -c 0
+
 echo
 echo "=== Testing version downgrade with zero expansion ==="
 echo
diff --git a/tests/qemu-iotests/061.out b/tests/qemu-iotests/061.out
index a683f46..0499138 100644
--- a/tests/qemu-iotests/061.out
+++ b/tests/qemu-iotests/061.out
@@ -57,7 +57,7 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
 wrote 131072/131072 bytes at offset 0
 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-./common.config: Aborted                 (core dumped) ( exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@" )
+./common.config: Aborted                 ( exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@" )
 magic                     0x514649fb
 version                   3
 backing_file_offset       0x0
@@ -215,7 +215,7 @@ No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
 wrote 131072/131072 bytes at offset 0
 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-./common.config: Aborted                 (core dumped) ( exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@" )
+./common.config: Aborted                 ( exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@" )
 magic                     0x514649fb
 version                   3
 backing_file_offset       0x0
-- 
2.5.1

Re: [Qemu-devel] [PATCH] iotests: disable core dumps in test 061

[Max Reitz]

[-- Attachment #1: Type: text/plain, Size: 2919 bytes --]

On 23.09.2015 18:11, Alberto Garcia wrote:
> Commit 934659c460 disabled the supression of segmentation faults in
> bash tests. The new output of test 061, however, assumes that a core
> dump will be produced if a program aborts. This is not necessarily the
> case because core dumps can be disabled using ulimit.
> 
> We cannot guarantee that core dumps can be enabled in all cases, so we
> should disable them completely and update the test output accordingly.
> 
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> ---
>  tests/qemu-iotests/061     | 3 +++
>  tests/qemu-iotests/061.out | 4 ++--
>  2 files changed, 5 insertions(+), 2 deletions(-)

As noted in the commit message for
3f394472c5bca59de5cab9baafdff1984b0213a3, ulimit -c 0 does not work for
everyone (for instance, for me it fails, probably because I'm using
systemd's coredumpctl). Generally speaking, it'll only prevent a core
dump from being created if your /proc/sys/kernel/core_pattern points to
a file, but it won't if it points to a program for gathering the dump.

What we really want is to use "sigraise $(kill -l KILL)" instead of
"abort", because SIGKILL never creates a core dump, but will have
basically the same effect of crashing qemu-io.

Max

> 
> diff --git a/tests/qemu-iotests/061 b/tests/qemu-iotests/061
> index 1df887a..7d8415b 100755
> --- a/tests/qemu-iotests/061
> +++ b/tests/qemu-iotests/061
> @@ -43,6 +43,9 @@ _supported_fmt qcow2
>  _supported_proto file
>  _supported_os Linux
>  
> +# Disable core dumps or they'll mess up the test output
> +ulimit -c 0
> +
>  echo
>  echo "=== Testing version downgrade with zero expansion ==="
>  echo
> diff --git a/tests/qemu-iotests/061.out b/tests/qemu-iotests/061.out
> index a683f46..0499138 100644
> --- a/tests/qemu-iotests/061.out
> +++ b/tests/qemu-iotests/061.out
> @@ -57,7 +57,7 @@ No errors were found on the image.
>  Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
>  wrote 131072/131072 bytes at offset 0
>  128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -./common.config: Aborted                 (core dumped) ( exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@" )
> +./common.config: Aborted                 ( exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@" )
>  magic                     0x514649fb
>  version                   3
>  backing_file_offset       0x0
> @@ -215,7 +215,7 @@ No errors were found on the image.
>  Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
>  wrote 131072/131072 bytes at offset 0
>  128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -./common.config: Aborted                 (core dumped) ( exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@" )
> +./common.config: Aborted                 ( exec "$QEMU_IO_PROG" $QEMU_IO_OPTIONS "$@" )
>  magic                     0x514649fb
>  version                   3
>  backing_file_offset       0x0
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH] iotests: disable core dumps in test 061

[Markus Armbruster]

Max Reitz <mreitz@redhat.com> writes:

> On 23.09.2015 18:11, Alberto Garcia wrote:
>> Commit 934659c460 disabled the supression of segmentation faults in
>> bash tests. The new output of test 061, however, assumes that a core
>> dump will be produced if a program aborts. This is not necessarily the
>> case because core dumps can be disabled using ulimit.
>> 
>> We cannot guarantee that core dumps can be enabled in all cases, so we
>> should disable them completely and update the test output accordingly.
>> 
>> Signed-off-by: Alberto Garcia <berto@igalia.com>
>> ---
>>  tests/qemu-iotests/061     | 3 +++
>>  tests/qemu-iotests/061.out | 4 ++--
>>  2 files changed, 5 insertions(+), 2 deletions(-)
>
> As noted in the commit message for
> 3f394472c5bca59de5cab9baafdff1984b0213a3, ulimit -c 0 does not work for
> everyone (for instance, for me it fails, probably because I'm using
> systemd's coredumpctl). Generally speaking, it'll only prevent a core
> dump from being created if your /proc/sys/kernel/core_pattern points to
> a file, but it won't if it points to a program for gathering the dump.
>
> What we really want is to use "sigraise $(kill -l KILL)" instead of
> "abort", because SIGKILL never creates a core dump, but will have
> basically the same effect of crashing qemu-io.

No, we don't want that.  SIGABRT gives the user the option to have core
dumps.  By switching to SIGKILL, you'd take away that option.

Because modern systems have complicated the ways you can get and not get
core dumps, user qemu-iotests is having difficulties getting one
reliably.  Since it's just as fine with getting none reliably, and a
reliably way to ask for that still exists (ulimit -c 0), it could do
just that.

Inconvenience: when a test fails, you can't examine its core dump
anymore, but have to instrument the test to create one, or splice in
gdb, or whatever else it takes.  On the other hand, you don't have to
delete core dumps anymore.

Possible alternative: normalize the crash message differences before
diffing against golden output.

Re: [Qemu-devel] [PATCH] iotests: disable core dumps in test 061

[Kevin Wolf]

Am 24.09.2015 um 08:45 hat Markus Armbruster geschrieben:
> Max Reitz <mreitz@redhat.com> writes:
> 
> > On 23.09.2015 18:11, Alberto Garcia wrote:
> >> Commit 934659c460 disabled the supression of segmentation faults in
> >> bash tests. The new output of test 061, however, assumes that a core
> >> dump will be produced if a program aborts. This is not necessarily the
> >> case because core dumps can be disabled using ulimit.
> >> 
> >> We cannot guarantee that core dumps can be enabled in all cases, so we
> >> should disable them completely and update the test output accordingly.
> >> 
> >> Signed-off-by: Alberto Garcia <berto@igalia.com>
> >> ---
> >>  tests/qemu-iotests/061     | 3 +++
> >>  tests/qemu-iotests/061.out | 4 ++--
> >>  2 files changed, 5 insertions(+), 2 deletions(-)
> >
> > As noted in the commit message for
> > 3f394472c5bca59de5cab9baafdff1984b0213a3, ulimit -c 0 does not work for
> > everyone (for instance, for me it fails, probably because I'm using
> > systemd's coredumpctl). Generally speaking, it'll only prevent a core
> > dump from being created if your /proc/sys/kernel/core_pattern points to
> > a file, but it won't if it points to a program for gathering the dump.
> >
> > What we really want is to use "sigraise $(kill -l KILL)" instead of
> > "abort", because SIGKILL never creates a core dump, but will have
> > basically the same effect of crashing qemu-io.
> 
> No, we don't want that.  SIGABRT gives the user the option to have core
> dumps.  By switching to SIGKILL, you'd take away that option.

Why do you need a core dump for a test case that intentionally simulates
a crash without any actual misbehaviour causing it? Isn't it actually
annoying to get useless core dumps?

> Because modern systems have complicated the ways you can get and not get
> core dumps, user qemu-iotests is having difficulties getting one
> reliably.  Since it's just as fine with getting none reliably, and a
> reliably way to ask for that still exists (ulimit -c 0), it could do
> just that.

If you reread Max' email carefully, his very point is that 'ulimit -c 0'
is _not_ reliable.

> Inconvenience: when a test fails, you can't examine its core dump
> anymore, but have to instrument the test to create one, or splice in
> gdb, or whatever else it takes.  On the other hand, you don't have to
> delete core dumps anymore.

If we switched the intentional crash to SIGKILL, you could still get
core dumps for cases where there is actual misbehaviour without touching
the script. 'ulimit -c 0' in contrast, in addition to not being
reliable, is all or nothing.

> Possible alternative: normalize the crash message differences before
> diffing against golden output.

Extending _filter_qemu_io is another viable option to make the test
pass, yes. However, you would still need to manually delete core dumps
from the intentional crash.

Kevin