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

[Qemu-devel] [PATCH v2 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).


v2:
- Rewrote patch 1: Overloading "abort" may be somehow technically
  justifyable, but there is no point in grasping at straws when it is
  much easier to just introduce a new command. Therefore, in this
  version, "abort" is no longer overloaded and instead a new command,
  "sigraise", is added (I thought long and hard about that name; I
  thought "raise" to sound too unspecific, so this is what I came up
  with) [Markus]
- Patch 2: Fixed commit message (s/if it is aborted/when it is killed/)
  [Markus]
- Patch 3: %s/abort -S 9/sigraise 9/


git-backport-diff against v1:

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:[down] 'qemu-io: Add sigraise command'
       ^^^^ wrong, should be:
      [0081] [FC]
002/3:[----] [--] 'iotests: Filter for "Killed" in qemu-io output'
003/3:[0006] [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           | 12 ++++++-----
 tests/qemu-iotests/039.out       |  6 +++---
 tests/qemu-iotests/common.filter |  2 +-
 4 files changed, 57 insertions(+), 9 deletions(-)

-- 
1.9.3

[Qemu-devel] [PATCH v2 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 to raise
any Unix 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..942b694 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 Unix signal\n"
+"\n"
+" Example:\n"
+" 'sigraise 9' - raises SIGKILL\n"
+"\n"
+" Invokes raise(signal), where \"signal\" is the mandatory integer argument\n"
+" given to sigraise.\n"
+"\n");
+}
+
+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 Unix 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 v2 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>
---
 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 v2 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 "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..51cb8f7 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 "sigraise 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 "sigraise 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 "sigraise 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

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

[Fam Zheng]

On Thu, 12/04 15:49, 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 to raise
> any Unix 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..942b694 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 Unix signal\n"
> +"\n"
> +" Example:\n"
> +" 'sigraise 9' - raises SIGKILL\n"
> +"\n"
> +" Invokes raise(signal), where \"signal\" is the mandatory integer argument\n"
> +" given to sigraise.\n"
> +"\n");
> +}
> +
> +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 Unix 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
> 
> 

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

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

[Fam Zheng]

On Thu, 12/04 15:49, Max Reitz wrote:
> _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>
> ---
>  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
> 
> 

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

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

[Fam Zheng]

On Thu, 12/04 15:49, 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 "abort -S 9" instead to have it crash
> without a core dump.

This needs to be fixed to "sigraise 9". Otherwise looks good. With the commit
message fixed,

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

> 
> 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..51cb8f7 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 "sigraise 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 "sigraise 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 "sigraise 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
> 
> 

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

[Max Reitz]

On 2014-12-05 at 08:08, Fam Zheng wrote:
> On Thu, 12/04 15:49, 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 "abort -S 9" instead to have it crash
>> without a core dump.
> This needs to be fixed to "sigraise 9".

Ah, right. Sorry. Will fix (after having waited for Markus's opinion on 
this v2).

> Otherwise looks good. With the commit
> message fixed,
>
> Reviewed-by: Fam Zheng <famz@redhat.com>

Thanks!

Max

>> 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..51cb8f7 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 "sigraise 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 "sigraise 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 "sigraise 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
>>
>>

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

[Markus Armbruster]

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.
>
> Thus, -c abort is not always useful to simulate a qemu-io crash;
> therefore, this patch adds a new sigraise command which allows to raise
> any Unix signal.

Nitpick: signals are ISO C, not just UNIX.  

>
> 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..942b694 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 Unix signal\n"
> +"\n"
> +" Example:\n"
> +" 'sigraise 9' - raises SIGKILL\n"

Assumes SIGKILL is encoded as 9, which is traditionally the case, but
not actually mandated by POSIX.

You could avoid hardcoding 9 with

    " 'sigraise %d' - raises SIGKILL\n"

with a SIGKILL as argument for %d.

But then you'd have to face the fact that SIGKILL is POSIX, not ISO C.
The ISO C signals are SIGABRT, SIGFPE, SIGILL, SIGINT, SIGSEGV, SIGTERM.
Of these, SIGINT and SIGTERM don't dump core, in case you care.

> +"\n"
> +" Invokes raise(signal), where \"signal\" is the mandatory integer argument\n"
> +" given to sigraise.\n"
> +"\n");
> +}
> +
> +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 Unix 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);
>  }

Looks good otherwise.

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

[Max Reitz]

On 2014-12-05 at 10:52, 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.
>>
>> Thus, -c abort is not always useful to simulate a qemu-io crash;
>> therefore, this patch adds a new sigraise command which allows to raise
>> any Unix signal.
> Nitpick: signals are ISO C, not just UNIX.

Yes, but "Unix signal" is what the Wikipedia article is named, so... ;-)

>> 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..942b694 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 Unix signal\n"
>> +"\n"
>> +" Example:\n"
>> +" 'sigraise 9' - raises SIGKILL\n"
> Assumes SIGKILL is encoded as 9, which is traditionally the case, but
> not actually mandated by POSIX.

Yes, I know. The best would be to parse the signal like kill(1) does, 
but that would have been extra difficult and probably not worth the effort.

Furthermore, I know there is a song called "kill dash nine", so I 
guessed it would be enough (at least it'll have to be enough for test 
039, thanks to "_supported_os Linux").

> You could avoid hardcoding 9 with
>
>      " 'sigraise %d' - raises SIGKILL\n"
>
> with a SIGKILL as argument for %d.

Clever. Will do.

> But then you'd have to face the fact that SIGKILL is POSIX, not ISO C.
> The ISO C signals are SIGABRT, SIGFPE, SIGILL, SIGINT, SIGSEGV, SIGTERM.
> Of these, SIGINT and SIGTERM don't dump core, in case you care.

Good to know. I guess I'll just go with SIGKILL anyway, it's ubiquitous 
enough.

>> +"\n"
>> +" Invokes raise(signal), where \"signal\" is the mandatory integer argument\n"
>> +" given to sigraise.\n"
>> +"\n");
>> +}
>> +
>> +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 Unix 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);
>>   }
> Looks good otherwise.

Thanks :-)

Max

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

[Markus Armbruster]

Max Reitz <mreitz@redhat.com> writes:

> On 2014-12-05 at 10:52, 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.
>>>
>>> Thus, -c abort is not always useful to simulate a qemu-io crash;
>>> therefore, this patch adds a new sigraise command which allows to raise
>>> any Unix signal.
>> Nitpick: signals are ISO C, not just UNIX.
>
> Yes, but "Unix signal" is what the Wikipedia article is named, so... ;-)

If it's in Wikipedia, it must be right!  Quick, file a bug against the C
standard!

>>> 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..942b694 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 Unix signal\n"
>>> +"\n"
>>> +" Example:\n"
>>> +" 'sigraise 9' - raises SIGKILL\n"
>> Assumes SIGKILL is encoded as 9, which is traditionally the case, but
>> not actually mandated by POSIX.
>
> Yes, I know. The best would be to parse the signal like kill(1) does,
> but that would have been extra difficult and probably not worth the
> effort.

Agree.

> Furthermore, I know there is a song called "kill dash nine", so I
> guessed it would be enough (at least it'll have to be enough for test
> 039, thanks to "_supported_os Linux").

Bash can map signal names to numbers and back:

    $ kill -l 9
    KILL
    $ kill -l KILL
    9

Perhaps you can use that to avoid hardcoding.

>> You could avoid hardcoding 9 with
>>
>>      " 'sigraise %d' - raises SIGKILL\n"
>>
>> with a SIGKILL as argument for %d.
>
> Clever. Will do.
>
>> But then you'd have to face the fact that SIGKILL is POSIX, not ISO C.
>> The ISO C signals are SIGABRT, SIGFPE, SIGILL, SIGINT, SIGSEGV, SIGTERM.
>> Of these, SIGINT and SIGTERM don't dump core, in case you care.
>
> Good to know. I guess I'll just go with SIGKILL anyway, it's
> ubiquitous enough.

Might break the Windows compile.

>>> +"\n"
>>> +" Invokes raise(signal), where \"signal\" is the mandatory integer argument\n"
>>> +" given to sigraise.\n"
>>> +"\n");
>>> +}
>>> +
>>> +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 Unix 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);
>>>   }
>> Looks good otherwise.
>
> Thanks :-)

Looking forward to v3 :)

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

[Kevin Wolf]

Am 05.12.2014 um 13:23 hat Markus Armbruster geschrieben:
> Max Reitz <mreitz@redhat.com> writes:
> 
> > On 2014-12-05 at 10:52, 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.
> >>>
> >>> Thus, -c abort is not always useful to simulate a qemu-io crash;
> >>> therefore, this patch adds a new sigraise command which allows to raise
> >>> any Unix signal.
> >> Nitpick: signals are ISO C, not just UNIX.
> >
> > Yes, but "Unix signal" is what the Wikipedia article is named, so... ;-)
> 
> If it's in Wikipedia, it must be right!  Quick, file a bug against the C
> standard!

Well, if we're into nitpicking today: As you already said below, only a
small subset is in standard C.  Therefore, "allows to raise any Unix
signal" is more than "allows to raise any C 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..942b694 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 Unix signal\n"
> >>> +"\n"
> >>> +" Example:\n"
> >>> +" 'sigraise 9' - raises SIGKILL\n"
> >> Assumes SIGKILL is encoded as 9, which is traditionally the case, but
> >> not actually mandated by POSIX.
> >
> > Yes, I know. The best would be to parse the signal like kill(1) does,
> > but that would have been extra difficult and probably not worth the
> > effort.
> 
> Agree.
> 
> > Furthermore, I know there is a song called "kill dash nine", so I
> > guessed it would be enough (at least it'll have to be enough for test
> > 039, thanks to "_supported_os Linux").
> 
> Bash can map signal names to numbers and back:
> 
>     $ kill -l 9
>     KILL
>     $ kill -l KILL
>     9
> 
> Perhaps you can use that to avoid hardcoding.

Can we please immediately stop overengineering a simple debugging tool
that should do nothing but quit qemu without doing a clean shutdown?

If the literal 9 bothers you, kill the example from the help text and be
done with it.

Kevin

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

[Max Reitz]

On 2014-12-05 at 13:23, Markus Armbruster wrote:
> Max Reitz <mreitz@redhat.com> writes:
>
>> On 2014-12-05 at 10:52, 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.
>>>>
>>>> Thus, -c abort is not always useful to simulate a qemu-io crash;
>>>> therefore, this patch adds a new sigraise command which allows to raise
>>>> any Unix signal.
>>> Nitpick: signals are ISO C, not just UNIX.
>> Yes, but "Unix signal" is what the Wikipedia article is named, so... ;-)
> If it's in Wikipedia, it must be right!  Quick, file a bug against the C
> standard!

Yes, I know. But caling it "ISO C signals" or just "C signals" sounds 
strange, since C only defines raise() as far as I remember. Originally, 
I just called it "signals", of course, but that didn't sound so clear. 
"Unix signal" is an unambiguous way to name them.

>>>> 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..942b694 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 Unix signal\n"
>>>> +"\n"
>>>> +" Example:\n"
>>>> +" 'sigraise 9' - raises SIGKILL\n"
>>> Assumes SIGKILL is encoded as 9, which is traditionally the case, but
>>> not actually mandated by POSIX.
>> Yes, I know. The best would be to parse the signal like kill(1) does,
>> but that would have been extra difficult and probably not worth the
>> effort.
> Agree.
>
>> Furthermore, I know there is a song called "kill dash nine", so I
>> guessed it would be enough (at least it'll have to be enough for test
>> 039, thanks to "_supported_os Linux").
> Bash can map signal names to numbers and back:
>
>      $ kill -l 9
>      KILL
>      $ kill -l KILL
>      9
>
> Perhaps you can use that to avoid hardcoding.

I hope you're talking about doing this in the patch and not in qemu-io 
itself. *g*

Well, can do. There is no real reason to as long as the test is only 
supported under Linux anyway, but there's no reason not to do this in 
the respin (other than that I'll lose Fam's R-b...) either. Therefore, 
will do, thanks.

>>> You could avoid hardcoding 9 with
>>>
>>>       " 'sigraise %d' - raises SIGKILL\n"
>>>
>>> with a SIGKILL as argument for %d.
>> Clever. Will do.
>>
>>> But then you'd have to face the fact that SIGKILL is POSIX, not ISO C.
>>> The ISO C signals are SIGABRT, SIGFPE, SIGILL, SIGINT, SIGSEGV, SIGTERM.
>>> Of these, SIGINT and SIGTERM don't dump core, in case you care.
>> Good to know. I guess I'll just go with SIGKILL anyway, it's
>> ubiquitous enough.
> Might break the Windows compile.

Right, the simplest way to do it then would be just to use SIGTERM 
instead of SIGKILL in the example and still use SIGKILL in 039. I'll do 
that if you don't disagree.

Max

>>>> +"\n"
>>>> +" Invokes raise(signal), where \"signal\" is the mandatory integer argument\n"
>>>> +" given to sigraise.\n"
>>>> +"\n");
>>>> +}
>>>> +
>>>> +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 Unix 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);
>>>>    }
>>> Looks good otherwise.
>> Thanks :-)
> Looking forward to v3 :)

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

[Markus Armbruster]

Max Reitz <mreitz@redhat.com> writes:

> On 2014-12-05 at 13:23, Markus Armbruster wrote:
>> Max Reitz <mreitz@redhat.com> writes:
>>
>>> On 2014-12-05 at 10:52, 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.
>>>>>
>>>>> Thus, -c abort is not always useful to simulate a qemu-io crash;
>>>>> therefore, this patch adds a new sigraise command which allows to raise
>>>>> any Unix signal.
>>>> Nitpick: signals are ISO C, not just UNIX.
>>> Yes, but "Unix signal" is what the Wikipedia article is named, so... ;-)
>> If it's in Wikipedia, it must be right!  Quick, file a bug against the C
>> standard!
>
> Yes, I know. But caling it "ISO C signals" or just "C signals" sounds
> strange, since C only defines raise() as far as I
> remember. Originally, I just called it "signals", of course, but that
> didn't sound so clear. "Unix signal" is an unambiguous way to name
> them.

I'd write "allows to raise a signal", since I trust folks that have a
need for this feature to know what signal means in this context.  But if
you'd rather write "raise a Unix signal", then that's clearly covered by
your artistic license.

>>>>> 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..942b694 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 Unix signal\n"
>>>>> +"\n"
>>>>> +" Example:\n"
>>>>> +" 'sigraise 9' - raises SIGKILL\n"
>>>> Assumes SIGKILL is encoded as 9, which is traditionally the case, but
>>>> not actually mandated by POSIX.
>>> Yes, I know. The best would be to parse the signal like kill(1) does,
>>> but that would have been extra difficult and probably not worth the
>>> effort.
>> Agree.
>>
>>> Furthermore, I know there is a song called "kill dash nine", so I
>>> guessed it would be enough (at least it'll have to be enough for test
>>> 039, thanks to "_supported_os Linux").
>> Bash can map signal names to numbers and back:
>>
>>      $ kill -l 9
>>      KILL
>>      $ kill -l KILL
>>      9
>>
>> Perhaps you can use that to avoid hardcoding.
>
> I hope you're talking about doing this in the patch and not in qemu-io
> itself. *g*

Dear god, no!  %-)

> Well, can do. There is no real reason to as long as the test is only
> supported under Linux anyway, but there's no reason not to do this in
> the respin (other than that I'll lose Fam's R-b...) either. Therefore,
> will do, thanks.
>
>>>> You could avoid hardcoding 9 with
>>>>
>>>>       " 'sigraise %d' - raises SIGKILL\n"
>>>>
>>>> with a SIGKILL as argument for %d.
>>> Clever. Will do.
>>>
>>>> But then you'd have to face the fact that SIGKILL is POSIX, not ISO C.
>>>> The ISO C signals are SIGABRT, SIGFPE, SIGILL, SIGINT, SIGSEGV, SIGTERM.
>>>> Of these, SIGINT and SIGTERM don't dump core, in case you care.
>>> Good to know. I guess I'll just go with SIGKILL anyway, it's
>>> ubiquitous enough.
>> Might break the Windows compile.
>
> Right, the simplest way to do it then would be just to use SIGTERM
> instead of SIGKILL in the example and still use SIGKILL in 039. I'll
> do that if you don't disagree.

Sounds sensible to me.

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

[Eric Blake]

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

On 12/05/2014 06:59 AM, Markus Armbruster wrote:

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

> I'd write "allows to raise a signal", since I trust folks that have a
> need for this feature to know what signal means in this context.  But if
> you'd rather write "raise a Unix signal", then that's clearly covered by
> your artistic license.

Actually, I'd write "allows raising a signal"; since "allows to" is an
awkward construction not usually heard in native English.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


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