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

[Markus Armbruster]

Max Reitz <address@hidden> writes:

> On 2014-12-05 at 13:23, Markus Armbruster wrote:
>> Max Reitz <address@hidden> writes:
>>
>>> On 2014-12-05 at 10:52, Markus Armbruster wrote:
>>>> Max Reitz <address@hidden> 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 <address@hidden>
>>>>> ---
>>>>>    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.