Re: [Qemu-devel] [PATCH v2] ivshmem-server: Terminate also on SIGINT

[Jan Kiszka]

On 05.08.19 10:33, Stefano Garzarella wrote:
> On Sat, Aug 03, 2019 at 03:22:04PM +0200, Jan Kiszka wrote:
>> From: Jan Kiszka <address@hidden>
>>
>> Allows to shutdown a foreground session via ctrl-c.
>>
>> Signed-off-by: Jan Kiszka <address@hidden>
>> ---
>>
>> Changes in v2:
>>  - adjust error message
>>
>>  contrib/ivshmem-server/main.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/contrib/ivshmem-server/main.c b/contrib/ivshmem-server/main.c
>> index 197c79c57e..e4cd35f74c 100644
>> --- a/contrib/ivshmem-server/main.c
>> +++ b/contrib/ivshmem-server/main.c
>> @@ -223,8 +223,9 @@ main(int argc, char *argv[])
>>      sa_quit.sa_handler = ivshmem_server_quit_cb;
>>      sa_quit.sa_flags = 0;
>>      if (sigemptyset(&sa_quit.sa_mask) == -1 ||
>> -        sigaction(SIGTERM, &sa_quit, 0) == -1) {
>> -        perror("failed to add SIGTERM handler; sigaction");
>> +        sigaction(SIGTERM, &sa_quit, 0) == -1 ||
>> +        sigaction(SIGINT, &sa_quit, 0) == -1) {
>> +        perror("failed to add signal handler; sigaction");
>>          goto err;
>>      }
>
> Reviewed-by: Stefano Garzarella <address@hidden>

Thanks.

>
> Not related with this patch, but since I was looking at the code,
> I noticed the 'ivshmem_server_quit' variable, set in the signal handler,
> is not volatile.
> Should we define it volatile to avoid possible compiler optimizations?

Yes, would be better.

I'm sure there is more, possibly also on the current device model side. I only
started to dig through it while implementing a new revision of the protocol on
top and aside of it.

Jan