[Qemu-devel] [PATCH v3 0/3] iotests: Fix test 039

[Qemu-devel] [PATCH v3 0/3] iotests: Fix test 039

[Max Reitz]

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 a new qemu-io command "sigraise" which invokes
raise(). Using this command to raise 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).


v3:
- Patch 1:
  - s/allows to raise/allows raising/ [Eric]
  - %s/Unix signal/signal/g [Markus]
  - Use SIGTERM instead of SIGKILL in the example, and don't hardcode
    its value [Markus]
- Patch 3:
  - Fix commit message (should have been "sigraise 9" instead of
    "abort -S 9", and now it's "sigraise $(kill -l KILL)") [Fam]
  - As hinted above, use "sigraise $(kill -l KILL)" instead of
    hardcoding the value 9 for SIGKILL [Markus]


git-backport-diff against v2:

Key:
[----] : patches are identical
[####] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively

001/3:[0008] [FC] 'qemu-io: Add sigraise command'
002/3:[----] [--] 'iotests: Filter for "Killed" in qemu-io output'
003/3:[0012] [FC] 'iotests: Fix test 039'


Max Reitz (3):
  qemu-io: Add sigraise command
  iotests: Filter for "Killed" in qemu-io output
  iotests: Fix test 039

 qemu-io-cmds.c                   | 46 ++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/039           | 18 +++++++++++-----
 tests/qemu-iotests/039.out       |  6 +++---
 tests/qemu-iotests/common.filter |  2 +-
 4 files changed, 63 insertions(+), 9 deletions(-)

-- 
1.9.3

[Qemu-devel] [PATCH v3 1/3] qemu-io: Add sigraise command

[Max Reitz]

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.

Thus, -c abort is not always useful to simulate a qemu-io crash;
therefore, this patch adds a new sigraise command which allows raising
a signal.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 qemu-io-cmds.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index d94fb1e..e708552 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -2048,6 +2048,51 @@ static const cmdinfo_t abort_cmd = {
        .oneline        = "simulate a program crash using abort(3)",
 };
 
+static void sigraise_help(void)
+{
+    printf(
+"\n"
+" raises the given signal\n"
+"\n"
+" Example:\n"
+" 'sigraise %i' - raises SIGTERM\n"
+"\n"
+" Invokes raise(signal), where \"signal\" is the mandatory integer argument\n"
+" given to sigraise.\n"
+"\n", SIGTERM);
+}
+
+static int sigraise_f(BlockDriverState *bs, int argc, char **argv);
+
+static const cmdinfo_t sigraise_cmd = {
+    .name       = "sigraise",
+    .cfunc      = sigraise_f,
+    .argmin     = 1,
+    .argmax     = 1,
+    .flags      = CMD_NOFILE_OK,
+    .args       = "signal",
+    .oneline    = "raises a signal",
+    .help       = sigraise_help,
+};
+
+static int sigraise_f(BlockDriverState *bs, int argc, char **argv)
+{
+    int sig = cvtnum(argv[1]);
+    if (sig < 0) {
+        printf("non-numeric signal number argument -- %s\n", argv[1]);
+        return 0;
+    }
+
+    /* Using raise() to kill this process does not necessarily flush all open
+     * streams. At least stdout and stderr (although the latter should be
+     * non-buffered anyway) should be flushed, though. */
+    fflush(stdout);
+    fflush(stderr);
+
+    raise(sig);
+    return 0;
+}
+
 static void sleep_cb(void *opaque)
 {
     bool *expired = opaque;
@@ -2202,4 +2247,5 @@ static void __attribute((constructor)) init_qemuio_commands(void)
     qemuio_add_command(&wait_break_cmd);
     qemuio_add_command(&abort_cmd);
     qemuio_add_command(&sleep_cmd);
+    qemuio_add_command(&sigraise_cmd);
 }
-- 
1.9.3

[Qemu-devel] [PATCH v3 2/3] iotests: Filter for "Killed" in qemu-io output

[Max Reitz]

_filter_qemu_io already filters out the process ID when qemu-io is
aborted; the same should be done when it is killed.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Fam Zheng <famz@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

[Qemu-devel] [PATCH v3 3/3] iotests: Fix test 039

[Max Reitz]

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 "sigraise $(kill -l KILL)" instead to have
it crash without a core dump.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/039     | 18 +++++++++++++-----
 tests/qemu-iotests/039.out |  6 +++---
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/tests/qemu-iotests/039 b/tests/qemu-iotests/039
index 84c9167..859705f 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,9 @@ 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 "sigraise $(kill -l KILL)" "$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 +109,9 @@ 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 "sigraise $(kill -l KILL)" "$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 +127,9 @@ 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 "sigraise $(kill -l KILL)" "$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

Re: [Qemu-devel] [PATCH v3 1/3] qemu-io: Add sigraise command

[Fam Zheng]

On Mon, 12/08 10:48, Max Reitz wrote:
> 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.
> 
> Thus, -c abort is not always useful to simulate a qemu-io crash;
> therefore, this patch adds a new sigraise command which allows raising
> a signal.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  qemu-io-cmds.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
> 
> diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
> index d94fb1e..e708552 100644
> --- a/qemu-io-cmds.c
> +++ b/qemu-io-cmds.c
> @@ -2048,6 +2048,51 @@ static const cmdinfo_t abort_cmd = {
>         .oneline        = "simulate a program crash using abort(3)",
>  };
>  
> +static void sigraise_help(void)
> +{
> +    printf(
> +"\n"
> +" raises the given signal\n"
> +"\n"
> +" Example:\n"
> +" 'sigraise %i' - raises SIGTERM\n"
> +"\n"
> +" Invokes raise(signal), where \"signal\" is the mandatory integer argument\n"
> +" given to sigraise.\n"
> +"\n", SIGTERM);
> +}
> +
> +static int sigraise_f(BlockDriverState *bs, int argc, char **argv);
> +
> +static const cmdinfo_t sigraise_cmd = {
> +    .name       = "sigraise",
> +    .cfunc      = sigraise_f,
> +    .argmin     = 1,
> +    .argmax     = 1,
> +    .flags      = CMD_NOFILE_OK,
> +    .args       = "signal",
> +    .oneline    = "raises a signal",
> +    .help       = sigraise_help,
> +};
> +
> +static int sigraise_f(BlockDriverState *bs, int argc, char **argv)
> +{
> +    int sig = cvtnum(argv[0]);
> +    if (sig < 0) {
> +        printf("non-numeric signal number argument -- %s\n", argv[1]);
> +        return 0;
> +    }
> +
> +    /* Using raise() to kill this process does not necessarily flush all open
> +     * streams. At least stdout and stderr (although the latter should be
> +     * non-buffered anyway) should be flushed, though. */
> +    fflush(stdout);
> +    fflush(stderr);
> +
> +    raise(sig);
> +    return 0;
> +}
> +
>  static void sleep_cb(void *opaque)
>  {
>      bool *expired = opaque;
> @@ -2202,4 +2247,5 @@ static void __attribute((constructor)) init_qemuio_commands(void)
>      qemuio_add_command(&wait_break_cmd);
>      qemuio_add_command(&abort_cmd);
>      qemuio_add_command(&sleep_cmd);
> +    qemuio_add_command(&sigraise_cmd);
>  }
> -- 
> 1.9.3
> 
> 

Reviewed-by: Fam Zheng <famz@redhat.com>

Re: [Qemu-devel] [PATCH v3 3/3] iotests: Fix test 039

[Fam Zheng]

On Mon, 12/08 10:48, Max Reitz wrote:
> 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 "sigraise $(kill -l KILL)" instead to have
> it crash without a core dump.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  tests/qemu-iotests/039     | 18 +++++++++++++-----
>  tests/qemu-iotests/039.out |  6 +++---
>  2 files changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/tests/qemu-iotests/039 b/tests/qemu-iotests/039
> index 84c9167..859705f 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,9 @@ 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 "sigraise $(kill -l KILL)" "$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 +109,9 @@ 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 "sigraise $(kill -l KILL)" "$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 +127,9 @@ 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 "sigraise $(kill -l KILL)" "$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
> 

Reviewed-by: Fam Zheng <famz@redhat.com>

Re: [Qemu-devel] [PATCH v3 0/3] iotests: Fix test 039

[Stefan Hajnoczi]

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

On Mon, Dec 08, 2014 at 10:48:09AM +0100, Max Reitz 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 a new qemu-io command "sigraise" which invokes
> raise(). Using this command to raise 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).
> 
> 
> v3:
> - Patch 1:
>   - s/allows to raise/allows raising/ [Eric]
>   - %s/Unix signal/signal/g [Markus]
>   - Use SIGTERM instead of SIGKILL in the example, and don't hardcode
>     its value [Markus]
> - Patch 3:
>   - Fix commit message (should have been "sigraise 9" instead of
>     "abort -S 9", and now it's "sigraise $(kill -l KILL)") [Fam]
>   - As hinted above, use "sigraise $(kill -l KILL)" instead of
>     hardcoding the value 9 for SIGKILL [Markus]
> 
> 
> git-backport-diff against v2:
> 
> Key:
> [----] : patches are identical
> [####] : number of functional differences between upstream/downstream patch
> [down] : patch is downstream-only
> The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively
> 
> 001/3:[0008] [FC] 'qemu-io: Add sigraise command'
> 002/3:[----] [--] 'iotests: Filter for "Killed" in qemu-io output'
> 003/3:[0012] [FC] 'iotests: Fix test 039'
> 
> 
> Max Reitz (3):
>   qemu-io: Add sigraise command
>   iotests: Filter for "Killed" in qemu-io output
>   iotests: Fix test 039
> 
>  qemu-io-cmds.c                   | 46 ++++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/039           | 18 +++++++++++-----
>  tests/qemu-iotests/039.out       |  6 +++---
>  tests/qemu-iotests/common.filter |  2 +-
>  4 files changed, 63 insertions(+), 9 deletions(-)
> 
> -- 
> 1.9.3
> 
> 

Thanks, applied to my block-next tree:
https://github.com/stefanha/qemu/commits/block-next

Stefan

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