* [Qemu-devel] [PATCH 0/3] iotests: Fix test 039 @ 2014-11-25 10:09 Max Reitz 2014-11-25 10:09 ` [Qemu-devel] [PATCH 1/3] qemu-io: Let -c abort raise any signal Max Reitz ` (4 more replies) 0 siblings, 5 replies; 17+ messages in thread From: Max Reitz @ 2014-11-25 10:09 UTC (permalink / raw) To: qemu-devel Cc: Kevin Wolf, Fam Zheng, Michael Müller, Mao Chuan Li, Max Reitz, Stefan Hajnoczi Test 039 used to fail because qemu-io -c abort may generate core dumps even with ulimit -c 0 (and the output then contains "(core dumped)"). Fix this by adding an option to the "abort" command which allows specifying a signal number to raise(). Using SIGKILL for example does not result in a core dump, but it still badly crashes qemu-io (as desired). I am sending this series because we need all tests to work before adding the check-block target to "make check" (which we will hopefully do soon). Max Reitz (3): qemu-io: Let -c abort raise any signal iotests: Filter for "Killed" in qemu-io output iotests: Fix test 039 qemu-io-cmds.c | 59 ++++++++++++++++++++++++++++++++++++++-- tests/qemu-iotests/039 | 12 ++++---- tests/qemu-iotests/039.out | 6 ++-- tests/qemu-iotests/common.filter | 2 +- 4 files changed, 67 insertions(+), 12 deletions(-) -- 1.9.3 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH 1/3] qemu-io: Let -c abort raise any signal 2014-11-25 10:09 [Qemu-devel] [PATCH 0/3] iotests: Fix test 039 Max Reitz @ 2014-11-25 10:09 ` Max Reitz 2014-11-25 13:19 ` Markus Armbruster 2014-11-25 10:09 ` [Qemu-devel] [PATCH 2/3] iotests: Filter for "Killed" in qemu-io output Max Reitz ` (3 subsequent siblings) 4 siblings, 1 reply; 17+ messages in thread From: Max Reitz @ 2014-11-25 10:09 UTC (permalink / raw) To: qemu-devel Cc: Kevin Wolf, Fam Zheng, Michael Müller, Mao Chuan Li, Max Reitz, Stefan Hajnoczi abort() has the sometimes undesirable side-effect of generating a core dump. If that is not needed, SIGKILL has the same effect of abruptly crash qemu; without a core dump. Therefore, this patch allows to use the qemu-io abort command to raise any signal. Signed-off-by: Max Reitz <mreitz@redhat.com> --- qemu-io-cmds.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 56 insertions(+), 3 deletions(-) diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c index d94fb1e..5d39cf4 100644 --- a/qemu-io-cmds.c +++ b/qemu-io-cmds.c @@ -2036,18 +2036,71 @@ static const cmdinfo_t wait_break_cmd = { .oneline = "waits for the suspension of a request", }; -static int abort_f(BlockDriverState *bs, int argc, char **argv) + +static void abort_help(void) { - abort(); + printf( +"\n" +" simulates a program crash\n" +"\n" +" Invokes abort(), or raise(signal) if a signal number is specified.\n" +" -S, -- number of the signal to raise()\n" +"\n"); } +static int abort_f(BlockDriverState *bs, int argc, char **argv); + static const cmdinfo_t abort_cmd = { .name = "abort", .cfunc = abort_f, + .argmin = 0, + .argmax = 2, .flags = CMD_NOFILE_OK, - .oneline = "simulate a program crash using abort(3)", + .args = "[-S signal]", + .oneline = "simulate a program crash", + .help = abort_help, }; +static int abort_f(BlockDriverState *bs, int argc, char **argv) +{ + int c; + int sig = -1; + + while ((c = getopt(argc, argv, "S:")) != EOF) { + switch (c) { + case 'S': + sig = cvtnum(optarg); + if (sig < 0) { + printf("non-numeric signal number argument -- %s\n", optarg); + return 0; + } + break; + + default: + return qemuio_command_usage(&abort_cmd); + } + } + + if (optind != argc) { + return qemuio_command_usage(&abort_cmd); + } + + if (sig < 0) { + abort(); + } else { + /* While abort() does flush all open streams, using raise() to kill this + * process does not necessarily. At least stdout and stderr (although + * the latter should be non-buffered anyway) should be flushed, though. + */ + fflush(stdout); + fflush(stderr); + + raise(sig); + /* raise() may return */ + return 0; + } +} + static void sleep_cb(void *opaque) { bool *expired = opaque; -- 1.9.3 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] qemu-io: Let -c abort raise any signal 2014-11-25 10:09 ` [Qemu-devel] [PATCH 1/3] qemu-io: Let -c abort raise any signal Max Reitz @ 2014-11-25 13:19 ` Markus Armbruster 2014-11-25 13:31 ` Max Reitz 0 siblings, 1 reply; 17+ messages in thread From: Markus Armbruster @ 2014-11-25 13:19 UTC (permalink / raw) To: Max Reitz Cc: Kevin Wolf, Fam Zheng, Michael Müller, Mao Chuan Li, qemu-devel, Stefan Hajnoczi Max Reitz <mreitz@redhat.com> writes: > abort() has the sometimes undesirable side-effect of generating a core > dump. If that is not needed, SIGKILL has the same effect of abruptly > crash qemu; without a core dump. > > Therefore, this patch allows to use the qemu-io abort command to raise > any signal. > > Signed-off-by: Max Reitz <mreitz@redhat.com> > --- > qemu-io-cmds.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++--- > 1 file changed, 56 insertions(+), 3 deletions(-) > > diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c > index d94fb1e..5d39cf4 100644 > --- a/qemu-io-cmds.c > +++ b/qemu-io-cmds.c > @@ -2036,18 +2036,71 @@ static const cmdinfo_t wait_break_cmd = { > .oneline = "waits for the suspension of a request", > }; > > -static int abort_f(BlockDriverState *bs, int argc, char **argv) > + > +static void abort_help(void) > { > - abort(); > + printf( > +"\n" > +" simulates a program crash\n" > +"\n" > +" Invokes abort(), or raise(signal) if a signal number is specified.\n" > +" -S, -- number of the signal to raise()\n" > +"\n"); > } > > +static int abort_f(BlockDriverState *bs, int argc, char **argv); > + > static const cmdinfo_t abort_cmd = { > .name = "abort", > .cfunc = abort_f, > + .argmin = 0, > + .argmax = 2, > .flags = CMD_NOFILE_OK, > - .oneline = "simulate a program crash using abort(3)", > + .args = "[-S signal]", > + .oneline = "simulate a program crash", > + .help = abort_help, > }; This overloads the abort command with a kill command. Do we really need a way to send arbitrary signals? If yes, shouldn't we call it "kill" rather than "abort"? I suspect fooling around with signals isn't necessary, and a simple exit(1) would do. > > +static int abort_f(BlockDriverState *bs, int argc, char **argv) > +{ > + int c; > + int sig = -1; > + > + while ((c = getopt(argc, argv, "S:")) != EOF) { > + switch (c) { > + case 'S': > + sig = cvtnum(optarg); > + if (sig < 0) { > + printf("non-numeric signal number argument -- %s\n", optarg); > + return 0; > + } > + break; > + > + default: > + return qemuio_command_usage(&abort_cmd); > + } > + } > + > + if (optind != argc) { > + return qemuio_command_usage(&abort_cmd); > + } > + > + if (sig < 0) { > + abort(); > + } else { > + /* While abort() does flush all open streams, using raise() to kill this > + * process does not necessarily. At least stdout and stderr (although > + * the latter should be non-buffered anyway) should be flushed, though. > + */ > + fflush(stdout); > + fflush(stderr); Without -S, we flush all streams. With -S, we flush only stdout and stderr. The inconsistency is ugly. Could be avoided with fcloseall(), except it's a GNU extension. Or drop the non-signal path entirely, and raise(SIGABRT) instead of abort(). > + > + raise(sig); > + /* raise() may return */ > + return 0; > + } > +} > + > static void sleep_cb(void *opaque) > { > bool *expired = opaque; ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] qemu-io: Let -c abort raise any signal 2014-11-25 13:19 ` Markus Armbruster @ 2014-11-25 13:31 ` Max Reitz 0 siblings, 0 replies; 17+ messages in thread From: Max Reitz @ 2014-11-25 13:31 UTC (permalink / raw) To: Markus Armbruster Cc: Kevin Wolf, Fam Zheng, Michael Müller, Mao Chuan Li, qemu-devel, Stefan Hajnoczi On 2014-11-25 at 14:19, Markus Armbruster wrote: > Max Reitz <mreitz@redhat.com> writes: > >> abort() has the sometimes undesirable side-effect of generating a core >> dump. If that is not needed, SIGKILL has the same effect of abruptly >> crash qemu; without a core dump. >> >> Therefore, this patch allows to use the qemu-io abort command to raise >> any signal. >> >> Signed-off-by: Max Reitz <mreitz@redhat.com> >> --- >> qemu-io-cmds.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++--- >> 1 file changed, 56 insertions(+), 3 deletions(-) >> >> diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c >> index d94fb1e..5d39cf4 100644 >> --- a/qemu-io-cmds.c >> +++ b/qemu-io-cmds.c >> @@ -2036,18 +2036,71 @@ static const cmdinfo_t wait_break_cmd = { >> .oneline = "waits for the suspension of a request", >> }; >> >> -static int abort_f(BlockDriverState *bs, int argc, char **argv) >> + >> +static void abort_help(void) >> { >> - abort(); >> + printf( >> +"\n" >> +" simulates a program crash\n" >> +"\n" >> +" Invokes abort(), or raise(signal) if a signal number is specified.\n" >> +" -S, -- number of the signal to raise()\n" >> +"\n"); >> } >> >> +static int abort_f(BlockDriverState *bs, int argc, char **argv); >> + >> static const cmdinfo_t abort_cmd = { >> .name = "abort", >> .cfunc = abort_f, >> + .argmin = 0, >> + .argmax = 2, >> .flags = CMD_NOFILE_OK, >> - .oneline = "simulate a program crash using abort(3)", >> + .args = "[-S signal]", >> + .oneline = "simulate a program crash", >> + .help = abort_help, >> }; > This overloads the abort command with a kill command. abort() does a bit more than raise(SIGABRT), but all that it does more is basically "Make sure that raise(SIGABRT) actually works". So abort() basically is already a "kill me" command (kill -SIGABRT). I think overloading it is fine, but I wouldn't have that much of a problem to introduce another command, but it was just simpler this way. > Do we really need a way to send arbitrary signals? Why not? I'm implementing the functionality here for a single signal, it's not that hard to do it for arbitrary signals, so I did it. > If yes, shouldn't we > call it "kill" rather than "abort"? I'd call it "raise" (for obious reasons), but will not rename "abort". I can create a separate "raise" or "kill" command, though, obviously. But as "abort" is basically a "raise 6" (or "abort -S 6" with this version of the series), I think it's completely fine to overload "abort". > I suspect fooling around with signals isn't necessary, and a simple > exit(1) would do. No, because that would execute the atexit() functions. I don't know whether such are used in qemu, but if we want to simulate a crash, exit() is not the right function to do that. >> >> +static int abort_f(BlockDriverState *bs, int argc, char **argv) >> +{ >> + int c; >> + int sig = -1; >> + >> + while ((c = getopt(argc, argv, "S:")) != EOF) { >> + switch (c) { >> + case 'S': >> + sig = cvtnum(optarg); >> + if (sig < 0) { >> + printf("non-numeric signal number argument -- %s\n", optarg); >> + return 0; >> + } >> + break; >> + >> + default: >> + return qemuio_command_usage(&abort_cmd); >> + } >> + } >> + >> + if (optind != argc) { >> + return qemuio_command_usage(&abort_cmd); >> + } >> + >> + if (sig < 0) { >> + abort(); >> + } else { >> + /* While abort() does flush all open streams, using raise() to kill this >> + * process does not necessarily. At least stdout and stderr (although >> + * the latter should be non-buffered anyway) should be flushed, though. >> + */ >> + fflush(stdout); >> + fflush(stderr); > Without -S, we flush all streams. With -S, we flush only stdout and > stderr. The inconsistency is ugly. Could be avoided with fcloseall(), > except it's a GNU extension. Or drop the non-signal path entirely, and > raise(SIGABRT) instead of abort(). Except abort() does a bit more. Because I think not flushing any stream except for stdout and stderr is closer to a real crash, I think making sig = 6 the default and thus dropping the non-signal path is the better option. Max ^ permalink raw reply [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH 2/3] iotests: Filter for "Killed" in qemu-io output 2014-11-25 10:09 [Qemu-devel] [PATCH 0/3] iotests: Fix test 039 Max Reitz 2014-11-25 10:09 ` [Qemu-devel] [PATCH 1/3] qemu-io: Let -c abort raise any signal Max Reitz @ 2014-11-25 10:09 ` Max Reitz 2014-11-25 13:05 ` Markus Armbruster 2014-11-25 10:09 ` [Qemu-devel] [PATCH 3/3] iotests: Fix test 039 Max Reitz ` (2 subsequent siblings) 4 siblings, 1 reply; 17+ messages in thread From: Max Reitz @ 2014-11-25 10:09 UTC (permalink / raw) To: qemu-devel Cc: Kevin Wolf, Fam Zheng, Michael Müller, Mao Chuan Li, Max Reitz, Stefan Hajnoczi _filter_qemu_io already filters out the process ID when qemu-io is aborted; the same should be done if it is aborted. Signed-off-by: Max Reitz <mreitz@redhat.com> --- tests/qemu-iotests/common.filter | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter index dfb9726..6c14590 100644 --- a/tests/qemu-iotests/common.filter +++ b/tests/qemu-iotests/common.filter @@ -150,7 +150,7 @@ _filter_win32() _filter_qemu_io() { _filter_win32 | sed -e "s/[0-9]* ops\; [0-9/:. sec]* ([0-9/.inf]* [EPTGMKiBbytes]*\/sec and [0-9/.inf]* ops\/sec)/X ops\; XX:XX:XX.X (XXX YYY\/sec and XXX ops\/sec)/" \ - -e "s/: line [0-9][0-9]*: *[0-9][0-9]*\( Aborted\)/:\1/" \ + -e "s/: line [0-9][0-9]*: *[0-9][0-9]*\( Aborted\| Killed\)/:\1/" \ -e "s/qemu-io> //g" } -- 1.9.3 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] iotests: Filter for "Killed" in qemu-io output 2014-11-25 10:09 ` [Qemu-devel] [PATCH 2/3] iotests: Filter for "Killed" in qemu-io output Max Reitz @ 2014-11-25 13:05 ` Markus Armbruster 2014-11-25 13:06 ` Max Reitz 0 siblings, 1 reply; 17+ messages in thread From: Markus Armbruster @ 2014-11-25 13:05 UTC (permalink / raw) To: Max Reitz Cc: Kevin Wolf, Fam Zheng, Michael Müller, Mao Chuan Li, qemu-devel, Stefan Hajnoczi Max Reitz <mreitz@redhat.com> writes: > _filter_qemu_io already filters out the process ID when qemu-io is > aborted; the same should be done if it is aborted. "when it is killed". ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] iotests: Filter for "Killed" in qemu-io output 2014-11-25 13:05 ` Markus Armbruster @ 2014-11-25 13:06 ` Max Reitz 0 siblings, 0 replies; 17+ messages in thread From: Max Reitz @ 2014-11-25 13:06 UTC (permalink / raw) To: Markus Armbruster Cc: Kevin Wolf, Fam Zheng, Michael Müller, Mao Chuan Li, qemu-devel, Stefan Hajnoczi On 2014-11-25 at 14:05, Markus Armbruster wrote: > Max Reitz <mreitz@redhat.com> writes: > >> _filter_qemu_io already filters out the process ID when qemu-io is >> aborted; the same should be done if it is aborted. > "when it is killed". Oops, right. Max ^ permalink raw reply [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH 3/3] iotests: Fix test 039 2014-11-25 10:09 [Qemu-devel] [PATCH 0/3] iotests: Fix test 039 Max Reitz 2014-11-25 10:09 ` [Qemu-devel] [PATCH 1/3] qemu-io: Let -c abort raise any signal Max Reitz 2014-11-25 10:09 ` [Qemu-devel] [PATCH 2/3] iotests: Filter for "Killed" in qemu-io output Max Reitz @ 2014-11-25 10:09 ` Max Reitz 2014-11-25 12:21 ` [Qemu-devel] [PATCH 0/3] " Markus Armbruster 2014-11-25 16:48 ` Michael Mueller 4 siblings, 0 replies; 17+ messages in thread From: Max Reitz @ 2014-11-25 10:09 UTC (permalink / raw) To: qemu-devel Cc: Kevin Wolf, Fam Zheng, Michael Müller, Mao Chuan Li, Max Reitz, Stefan Hajnoczi Test 039 used qemu-io -c abort for simulating a qemu crash; however, abort() generally results in a core dump and ulimit -c 0 is no reliable way of preventing that. Use "abort -S 9" instead to have it crash without a core dump. Signed-off-by: Max Reitz <mreitz@redhat.com> --- tests/qemu-iotests/039 | 12 +++++++----- tests/qemu-iotests/039.out | 6 +++--- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/tests/qemu-iotests/039 b/tests/qemu-iotests/039 index 84c9167..813822c 100755 --- a/tests/qemu-iotests/039 +++ b/tests/qemu-iotests/039 @@ -47,9 +47,11 @@ _supported_os Linux _default_cache_mode "writethrough" _supported_cache_modes "writethrough" -_no_dump_exec() +_subshell_exec() { - (ulimit -c 0; exec "$@") + # Executing crashing commands in a subshell prevents information like the + # "Killed" line from being lost + (exec "$@") } size=128M @@ -72,7 +74,7 @@ echo "== Creating a dirty image file ==" IMGOPTS="compat=1.1,lazy_refcounts=on" _make_test_img $size -_no_dump_exec $QEMU_IO -c "write -P 0x5a 0 512" -c "abort" "$TEST_IMG" 2>&1 | _filter_qemu_io +_subshell_exec $QEMU_IO -c "write -P 0x5a 0 512" -c "abort -S 9" "$TEST_IMG" 2>&1 | _filter_qemu_io # The dirty bit must be set $PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features @@ -105,7 +107,7 @@ echo "== Opening a dirty image read/write should repair it ==" IMGOPTS="compat=1.1,lazy_refcounts=on" _make_test_img $size -_no_dump_exec $QEMU_IO -c "write -P 0x5a 0 512" -c "abort" "$TEST_IMG" 2>&1 | _filter_qemu_io +_subshell_exec $QEMU_IO -c "write -P 0x5a 0 512" -c "abort -S 9" "$TEST_IMG" 2>&1 | _filter_qemu_io # The dirty bit must be set $PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features @@ -121,7 +123,7 @@ echo "== Creating an image file with lazy_refcounts=off ==" IMGOPTS="compat=1.1,lazy_refcounts=off" _make_test_img $size -_no_dump_exec $QEMU_IO -c "write -P 0x5a 0 512" -c "abort" "$TEST_IMG" 2>&1 | _filter_qemu_io +_subshell_exec $QEMU_IO -c "write -P 0x5a 0 512" -c "abort -S 9" "$TEST_IMG" 2>&1 | _filter_qemu_io # The dirty bit must not be set since lazy_refcounts=off $PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features diff --git a/tests/qemu-iotests/039.out b/tests/qemu-iotests/039.out index 0adf153..35a04bd 100644 --- a/tests/qemu-iotests/039.out +++ b/tests/qemu-iotests/039.out @@ -11,7 +11,7 @@ No errors were found on the image. Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 wrote 512/512 bytes at offset 0 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -./039: Aborted ( ulimit -c 0; exec "$@" ) +./039: Killed ( exec "$@" ) incompatible_features 0x1 ERROR cluster 5 refcount=0 reference=1 ERROR OFLAG_COPIED data cluster: l2_entry=8000000000050000 refcount=0 @@ -46,7 +46,7 @@ read 512/512 bytes at offset 0 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 wrote 512/512 bytes at offset 0 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -./039: Aborted ( ulimit -c 0; exec "$@" ) +./039: Killed ( exec "$@" ) incompatible_features 0x1 ERROR cluster 5 refcount=0 reference=1 Rebuilding refcount structure @@ -60,7 +60,7 @@ incompatible_features 0x0 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 wrote 512/512 bytes at offset 0 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -./039: Aborted ( ulimit -c 0; exec "$@" ) +./039: Killed ( exec "$@" ) incompatible_features 0x0 No errors were found on the image. -- 1.9.3 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH 0/3] iotests: Fix test 039 2014-11-25 10:09 [Qemu-devel] [PATCH 0/3] iotests: Fix test 039 Max Reitz ` (2 preceding siblings ...) 2014-11-25 10:09 ` [Qemu-devel] [PATCH 3/3] iotests: Fix test 039 Max Reitz @ 2014-11-25 12:21 ` Markus Armbruster 2014-11-25 12:29 ` Max Reitz 2014-11-25 16:48 ` Michael Mueller 4 siblings, 1 reply; 17+ messages in thread From: Markus Armbruster @ 2014-11-25 12:21 UTC (permalink / raw) To: Max Reitz Cc: Kevin Wolf, Fam Zheng, Michael Müller, Mao Chuan Li, qemu-devel, Stefan Hajnoczi Max Reitz <mreitz@redhat.com> writes: > Test 039 used to fail I'm confused: "used to" suggests it doesn't anymore, but you sending a patches strongly suggests something's broken. > because qemu-io -c abort may generate core dumps > even with ulimit -c 0 (and the output then contains "(core dumped)"). How? [...] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH 0/3] iotests: Fix test 039 2014-11-25 12:21 ` [Qemu-devel] [PATCH 0/3] " Markus Armbruster @ 2014-11-25 12:29 ` Max Reitz 2014-11-25 13:20 ` Markus Armbruster 0 siblings, 1 reply; 17+ messages in thread From: Max Reitz @ 2014-11-25 12:29 UTC (permalink / raw) To: Markus Armbruster Cc: Kevin Wolf, Fam Zheng, Michael Müller, Mao Chuan Li, qemu-devel, Stefan Hajnoczi On 2014-11-25 at 13:21, Markus Armbruster wrote: > Max Reitz <mreitz@redhat.com> writes: > >> Test 039 used to fail > I'm confused: "used to" suggests it doesn't anymore, but you sending a > patches strongly suggests something's broken. Well, it used to fail before this series. :-P You're right, this sounds bad. Currently, 039 does fail, at least on any system with a /proc/sys/kernel/core_pattern passing the dump to another program. After this series, it does no longer. >> because qemu-io -c abort may generate core dumps >> even with ulimit -c 0 (and the output then contains "(core dumped)"). > How? See the patches[1][2] by Mao Chuan Li. If /proc/sys/kernel/core_pattern passes the dump to another program, ulimit -c 0 does not matter. [1] http://lists.nongnu.org/archive/html/qemu-devel/2014-11/msg02092.html [2] http://lists.nongnu.org/archive/html/qemu-devel/2014-11/msg02093.html The problem with those patches is that they require access to /proc/sys/kernel/core_pattern. I don't like having to run the iotests as root. Max ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH 0/3] iotests: Fix test 039 2014-11-25 12:29 ` Max Reitz @ 2014-11-25 13:20 ` Markus Armbruster 2014-11-25 13:22 ` Max Reitz 0 siblings, 1 reply; 17+ messages in thread From: Markus Armbruster @ 2014-11-25 13:20 UTC (permalink / raw) To: Max Reitz Cc: Kevin Wolf, Fam Zheng, Michael Müller, Mao Chuan Li, qemu-devel, Stefan Hajnoczi Max Reitz <mreitz@redhat.com> writes: > On 2014-11-25 at 13:21, Markus Armbruster wrote: >> Max Reitz <mreitz@redhat.com> writes: >> >>> Test 039 used to fail >> I'm confused: "used to" suggests it doesn't anymore, but you sending a >> patches strongly suggests something's broken. > > Well, it used to fail before this series. :-P > > You're right, this sounds bad. Currently, 039 does fail, at least on > any system with a /proc/sys/kernel/core_pattern passing the dump to > another program. After this series, it does no longer. > >>> because qemu-io -c abort may generate core dumps >>> even with ulimit -c 0 (and the output then contains "(core dumped)"). >> How? > > See the patches[1][2] by Mao Chuan Li. If > /proc/sys/kernel/core_pattern passes the dump to another program, > ulimit -c 0 does not matter. > > [1] http://lists.nongnu.org/archive/html/qemu-devel/2014-11/msg02092.html > [2] http://lists.nongnu.org/archive/html/qemu-devel/2014-11/msg02093.html > > The problem with those patches is that they require access to > /proc/sys/kernel/core_pattern. I don't like having to run the iotests > as root. To me, this sounds like a case of "doctor, it hurts when I do this". ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH 0/3] iotests: Fix test 039 2014-11-25 13:20 ` Markus Armbruster @ 2014-11-25 13:22 ` Max Reitz 2014-11-25 13:48 ` Markus Armbruster 0 siblings, 1 reply; 17+ messages in thread From: Max Reitz @ 2014-11-25 13:22 UTC (permalink / raw) To: Markus Armbruster Cc: Kevin Wolf, Fam Zheng, Michael Müller, Mao Chuan Li, qemu-devel, Stefan Hajnoczi On 2014-11-25 at 14:20, Markus Armbruster wrote: > Max Reitz <mreitz@redhat.com> writes: > >> On 2014-11-25 at 13:21, Markus Armbruster wrote: >>> Max Reitz <mreitz@redhat.com> writes: >>> >>>> Test 039 used to fail >>> I'm confused: "used to" suggests it doesn't anymore, but you sending a >>> patches strongly suggests something's broken. >> Well, it used to fail before this series. :-P >> >> You're right, this sounds bad. Currently, 039 does fail, at least on >> any system with a /proc/sys/kernel/core_pattern passing the dump to >> another program. After this series, it does no longer. >> >>>> because qemu-io -c abort may generate core dumps >>>> even with ulimit -c 0 (and the output then contains "(core dumped)"). >>> How? >> See the patches[1][2] by Mao Chuan Li. If >> /proc/sys/kernel/core_pattern passes the dump to another program, >> ulimit -c 0 does not matter. >> >> [1] http://lists.nongnu.org/archive/html/qemu-devel/2014-11/msg02092.html >> [2] http://lists.nongnu.org/archive/html/qemu-devel/2014-11/msg02093.html >> >> The problem with those patches is that they require access to >> /proc/sys/kernel/core_pattern. I don't like having to run the iotests >> as root. > To me, this sounds like a case of "doctor, it hurts when I do this". What do you mean? That I don't want the iotests to run as root? Or that I don't want to go the alternative of filtering out the "(core dumped)" message? 039 doesn't need a core dump. abort() generates a core dump. Why are we using abort()? Makes no sense. So don't use it. I can see that people may want to use qemu-io -c abort to generate a core dump, though, which is why I'm not simply replacing the abort() call by raise(SIGKILL). Max ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH 0/3] iotests: Fix test 039 2014-11-25 13:22 ` Max Reitz @ 2014-11-25 13:48 ` Markus Armbruster 2014-11-25 13:50 ` Max Reitz 0 siblings, 1 reply; 17+ messages in thread From: Markus Armbruster @ 2014-11-25 13:48 UTC (permalink / raw) To: Max Reitz Cc: Kevin Wolf, Fam Zheng, Michael Müller, Mao Chuan Li, qemu-devel, Stefan Hajnoczi Max Reitz <mreitz@redhat.com> writes: > On 2014-11-25 at 14:20, Markus Armbruster wrote: >> Max Reitz <mreitz@redhat.com> writes: >> >>> On 2014-11-25 at 13:21, Markus Armbruster wrote: >>>> Max Reitz <mreitz@redhat.com> writes: >>>> >>>>> Test 039 used to fail >>>> I'm confused: "used to" suggests it doesn't anymore, but you sending a >>>> patches strongly suggests something's broken. >>> Well, it used to fail before this series. :-P >>> >>> You're right, this sounds bad. Currently, 039 does fail, at least on >>> any system with a /proc/sys/kernel/core_pattern passing the dump to >>> another program. After this series, it does no longer. >>> >>>>> because qemu-io -c abort may generate core dumps >>>>> even with ulimit -c 0 (and the output then contains "(core dumped)"). >>>> How? >>> See the patches[1][2] by Mao Chuan Li. If >>> /proc/sys/kernel/core_pattern passes the dump to another program, >>> ulimit -c 0 does not matter. >>> >>> [1] http://lists.nongnu.org/archive/html/qemu-devel/2014-11/msg02092.html >>> [2] http://lists.nongnu.org/archive/html/qemu-devel/2014-11/msg02093.html >>> >>> The problem with those patches is that they require access to >>> /proc/sys/kernel/core_pattern. I don't like having to run the iotests >>> as root. >> To me, this sounds like a case of "doctor, it hurts when I do this". > > What do you mean? That I don't want the iotests to run as root? Or > that I don't want to go the alternative of filtering out the "(core > dumped)" message? I mean: Doctor, it hurts when I write weird stuff to /proc/sys/kernel/core_pattern. Don't do that then. If you want to be a nicer doc than me, go right ahead. [...] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH 0/3] iotests: Fix test 039 2014-11-25 13:48 ` Markus Armbruster @ 2014-11-25 13:50 ` Max Reitz 2014-11-25 14:04 ` Markus Armbruster 0 siblings, 1 reply; 17+ messages in thread From: Max Reitz @ 2014-11-25 13:50 UTC (permalink / raw) To: Markus Armbruster Cc: Kevin Wolf, Fam Zheng, Michael Müller, Mao Chuan Li, qemu-devel, Stefan Hajnoczi On 2014-11-25 at 14:48, Markus Armbruster wrote: > Max Reitz <mreitz@redhat.com> writes: > >> On 2014-11-25 at 14:20, Markus Armbruster wrote: >>> Max Reitz <mreitz@redhat.com> writes: >>> >>>> On 2014-11-25 at 13:21, Markus Armbruster wrote: >>>>> Max Reitz <mreitz@redhat.com> writes: >>>>> >>>>>> Test 039 used to fail >>>>> I'm confused: "used to" suggests it doesn't anymore, but you sending a >>>>> patches strongly suggests something's broken. >>>> Well, it used to fail before this series. :-P >>>> >>>> You're right, this sounds bad. Currently, 039 does fail, at least on >>>> any system with a /proc/sys/kernel/core_pattern passing the dump to >>>> another program. After this series, it does no longer. >>>> >>>>>> because qemu-io -c abort may generate core dumps >>>>>> even with ulimit -c 0 (and the output then contains "(core dumped)"). >>>>> How? >>>> See the patches[1][2] by Mao Chuan Li. If >>>> /proc/sys/kernel/core_pattern passes the dump to another program, >>>> ulimit -c 0 does not matter. >>>> >>>> [1] http://lists.nongnu.org/archive/html/qemu-devel/2014-11/msg02092.html >>>> [2] http://lists.nongnu.org/archive/html/qemu-devel/2014-11/msg02093.html >>>> >>>> The problem with those patches is that they require access to >>>> /proc/sys/kernel/core_pattern. I don't like having to run the iotests >>>> as root. >>> To me, this sounds like a case of "doctor, it hurts when I do this". >> What do you mean? That I don't want the iotests to run as root? Or >> that I don't want to go the alternative of filtering out the "(core >> dumped)" message? > I mean: > > Doctor, it hurts when I write weird stuff to > /proc/sys/kernel/core_pattern. > > Don't do that then. > > If you want to be a nicer doc than me, go right ahead. I don't write weird stuff there. My default system configuration does (and mine is not the only one): $ uname -r 3.17.3-200.fc20.x86_64 $ cat /proc/sys/kernel/core_pattern |/usr/sbin/chroot /proc/%P/root /usr/libexec/abrt-hook-ccpp %s %c %p %u %g %t e Max ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH 0/3] iotests: Fix test 039 2014-11-25 13:50 ` Max Reitz @ 2014-11-25 14:04 ` Markus Armbruster 2014-11-25 16:53 ` Michael Mueller 0 siblings, 1 reply; 17+ messages in thread From: Markus Armbruster @ 2014-11-25 14:04 UTC (permalink / raw) To: Max Reitz Cc: Kevin Wolf, Fam Zheng, Michael Müller, Mao Chuan Li, qemu-devel, Stefan Hajnoczi Max Reitz <mreitz@redhat.com> writes: > On 2014-11-25 at 14:48, Markus Armbruster wrote: >> Max Reitz <mreitz@redhat.com> writes: >> >>> On 2014-11-25 at 14:20, Markus Armbruster wrote: >>>> Max Reitz <mreitz@redhat.com> writes: >>>> >>>>> On 2014-11-25 at 13:21, Markus Armbruster wrote: >>>>>> Max Reitz <mreitz@redhat.com> writes: >>>>>> >>>>>>> Test 039 used to fail >>>>>> I'm confused: "used to" suggests it doesn't anymore, but you sending a >>>>>> patches strongly suggests something's broken. >>>>> Well, it used to fail before this series. :-P >>>>> >>>>> You're right, this sounds bad. Currently, 039 does fail, at least on >>>>> any system with a /proc/sys/kernel/core_pattern passing the dump to >>>>> another program. After this series, it does no longer. >>>>> >>>>>>> because qemu-io -c abort may generate core dumps >>>>>>> even with ulimit -c 0 (and the output then contains "(core dumped)"). >>>>>> How? >>>>> See the patches[1][2] by Mao Chuan Li. If >>>>> /proc/sys/kernel/core_pattern passes the dump to another program, >>>>> ulimit -c 0 does not matter. >>>>> >>>>> [1] http://lists.nongnu.org/archive/html/qemu-devel/2014-11/msg02092.html >>>>> [2] http://lists.nongnu.org/archive/html/qemu-devel/2014-11/msg02093.html >>>>> >>>>> The problem with those patches is that they require access to >>>>> /proc/sys/kernel/core_pattern. I don't like having to run the iotests >>>>> as root. >>>> To me, this sounds like a case of "doctor, it hurts when I do this". >>> What do you mean? That I don't want the iotests to run as root? Or >>> that I don't want to go the alternative of filtering out the "(core >>> dumped)" message? >> I mean: >> >> Doctor, it hurts when I write weird stuff to >> /proc/sys/kernel/core_pattern. >> >> Don't do that then. >> >> If you want to be a nicer doc than me, go right ahead. > > I don't write weird stuff there. My default system configuration does > (and mine is not the only one): > > $ uname -r > 3.17.3-200.fc20.x86_64 > $ cat /proc/sys/kernel/core_pattern > |/usr/sbin/chroot /proc/%P/root /usr/libexec/abrt-hook-ccpp %s %c %p > %u %g %t e abrt is one of the things I kill with prejudice on all my development machines. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH 0/3] iotests: Fix test 039 2014-11-25 14:04 ` Markus Armbruster @ 2014-11-25 16:53 ` Michael Mueller 0 siblings, 0 replies; 17+ messages in thread From: Michael Mueller @ 2014-11-25 16:53 UTC (permalink / raw) To: Markus Armbruster Cc: Kevin Wolf, Fam Zheng, Mao Chuan Li, qemu-devel, Max Reitz, Stefan Hajnoczi On Tue, 25 Nov 2014 15:04:38 +0100 Markus Armbruster <armbru@redhat.com> wrote: > Max Reitz <mreitz@redhat.com> writes: > > > On 2014-11-25 at 14:48, Markus Armbruster wrote: > >> Max Reitz <mreitz@redhat.com> writes: > >> > >>> On 2014-11-25 at 14:20, Markus Armbruster wrote: > >>>> Max Reitz <mreitz@redhat.com> writes: > >>>> > >>>>> On 2014-11-25 at 13:21, Markus Armbruster wrote: > >>>>>> Max Reitz <mreitz@redhat.com> writes: > >>>>>> > >>>>>>> Test 039 used to fail > >>>>>> I'm confused: "used to" suggests it doesn't anymore, but you sending a > >>>>>> patches strongly suggests something's broken. > >>>>> Well, it used to fail before this series. :-P > >>>>> > >>>>> You're right, this sounds bad. Currently, 039 does fail, at least on > >>>>> any system with a /proc/sys/kernel/core_pattern passing the dump to > >>>>> another program. After this series, it does no longer. > >>>>> > >>>>>>> because qemu-io -c abort may generate core dumps > >>>>>>> even with ulimit -c 0 (and the output then contains "(core dumped)"). > >>>>>> How? > >>>>> See the patches[1][2] by Mao Chuan Li. If > >>>>> /proc/sys/kernel/core_pattern passes the dump to another program, > >>>>> ulimit -c 0 does not matter. > >>>>> > >>>>> [1] http://lists.nongnu.org/archive/html/qemu-devel/2014-11/msg02092.html > >>>>> [2] http://lists.nongnu.org/archive/html/qemu-devel/2014-11/msg02093.html > >>>>> > >>>>> The problem with those patches is that they require access to > >>>>> /proc/sys/kernel/core_pattern. I don't like having to run the iotests > >>>>> as root. > >>>> To me, this sounds like a case of "doctor, it hurts when I do this". > >>> What do you mean? That I don't want the iotests to run as root? Or > >>> that I don't want to go the alternative of filtering out the "(core > >>> dumped)" message? > >> I mean: > >> > >> Doctor, it hurts when I write weird stuff to > >> /proc/sys/kernel/core_pattern. > >> > >> Don't do that then. > >> > >> If you want to be a nicer doc than me, go right ahead. > > > > I don't write weird stuff there. My default system configuration does > > (and mine is not the only one): > > > > $ uname -r > > 3.17.3-200.fc20.x86_64 > > $ cat /proc/sys/kernel/core_pattern > > |/usr/sbin/chroot /proc/%P/root /usr/libexec/abrt-hook-ccpp %s %c %p > > %u %g %t e > > abrt is one of the things I kill with prejudice on all my development > machines. Markus, we use non-personal test machines that shall automatically start tests like iotests when patches are committed, thus this series really helps... Thanks. Michael > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH 0/3] iotests: Fix test 039 2014-11-25 10:09 [Qemu-devel] [PATCH 0/3] iotests: Fix test 039 Max Reitz ` (3 preceding siblings ...) 2014-11-25 12:21 ` [Qemu-devel] [PATCH 0/3] " Markus Armbruster @ 2014-11-25 16:48 ` Michael Mueller 4 siblings, 0 replies; 17+ messages in thread From: Michael Mueller @ 2014-11-25 16:48 UTC (permalink / raw) To: Max Reitz Cc: Kevin Wolf, Mao Chuan Li, Fam Zheng, qemu-devel, Stefan Hajnoczi On Tue, 25 Nov 2014 11:09:35 +0100 Max Reitz <mreitz@redhat.com> wrote: > Test 039 used to fail because qemu-io -c abort may generate core dumps > even with ulimit -c 0 (and the output then contains "(core dumped)"). > Fix this by adding an option to the "abort" command which allows > specifying a signal number to raise(). Using SIGKILL for example does > not result in a core dump, but it still badly crashes qemu-io (as > desired). > > I am sending this series because we need all tests to work before adding > the check-block target to "make check" (which we will hopefully do > soon). > > > Max Reitz (3): > qemu-io: Let -c abort raise any signal > iotests: Filter for "Killed" in qemu-io output > iotests: Fix test 039 > > qemu-io-cmds.c | 59 ++++++++++++++++++++++++++++++++++++++-- > tests/qemu-iotests/039 | 12 ++++---- > tests/qemu-iotests/039.out | 6 ++-- > tests/qemu-iotests/common.filter | 2 +- > 4 files changed, 67 insertions(+), 12 deletions(-) > Hi Max, thanks a lot for picking this up to make our poor circumvention obsolete. Michael ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2014-11-25 16:54 UTC | newest] Thread overview: 17+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-11-25 10:09 [Qemu-devel] [PATCH 0/3] iotests: Fix test 039 Max Reitz 2014-11-25 10:09 ` [Qemu-devel] [PATCH 1/3] qemu-io: Let -c abort raise any signal Max Reitz 2014-11-25 13:19 ` Markus Armbruster 2014-11-25 13:31 ` Max Reitz 2014-11-25 10:09 ` [Qemu-devel] [PATCH 2/3] iotests: Filter for "Killed" in qemu-io output Max Reitz 2014-11-25 13:05 ` Markus Armbruster 2014-11-25 13:06 ` Max Reitz 2014-11-25 10:09 ` [Qemu-devel] [PATCH 3/3] iotests: Fix test 039 Max Reitz 2014-11-25 12:21 ` [Qemu-devel] [PATCH 0/3] " Markus Armbruster 2014-11-25 12:29 ` Max Reitz 2014-11-25 13:20 ` Markus Armbruster 2014-11-25 13:22 ` Max Reitz 2014-11-25 13:48 ` Markus Armbruster 2014-11-25 13:50 ` Max Reitz 2014-11-25 14:04 ` Markus Armbruster 2014-11-25 16:53 ` Michael Mueller 2014-11-25 16:48 ` Michael Mueller
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).