| Message ID | 1443024701-32184-1-git-send-email-berto@igalia.com |
|---|---|
| State | New |
| Headers | show |
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 >
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.
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
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
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(-)