diff mbox series

[v2] ivshmem-server: Terminate also on SIGINT

Message ID 99c1a7bd-1876-66a2-4b8e-d5bc86116fe7@web.de
State New
Headers show
Series [v2] ivshmem-server: Terminate also on SIGINT | expand

Commit Message

Jan Kiszka Aug. 3, 2019, 1:22 p.m. UTC 
From: Jan Kiszka <jan.kiszka@siemens.com>

Allows to shutdown a foreground session via ctrl-c.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

Changes in v2:
 - adjust error message

 contrib/ivshmem-server/main.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

--
2.16.4

Comments

Claudio Fontana Aug. 4, 2019, 2 a.m. UTC | #1 
On 8/3/19 3:22 PM, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> Allows to shutdown a foreground session via ctrl-c.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
> 
> 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;
>      }
> 
> --
> 2.16.4
> 
> 
Reviewed-by: Claudio Fontana <claudio.fontana@suse.com>
Stefano Garzarella Aug. 5, 2019, 8:33 a.m. UTC | #2 
On Sat, Aug 03, 2019 at 03:22:04PM +0200, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> Allows to shutdown a foreground session via ctrl-c.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
> 
> 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 <sgarzare@redhat.com>

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?

Thanks,
Stefano
Jan Kiszka Aug. 5, 2019, 10:06 a.m. UTC | #3 
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 <jan.kiszka@siemens.com>
>>
>> Allows to shutdown a foreground session via ctrl-c.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>
>> 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 <sgarzare@redhat.com>

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
Jan Kiszka Nov. 8, 2019, 9:08 a.m. UTC | #4 
On 03.08.19 15:22, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> Allows to shutdown a foreground session via ctrl-c.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
> 
> 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;
>       }
> 
> --
> 2.16.4
> 
> 

...and this one for you as well, Markus?

Jan
Markus Armbruster Nov. 8, 2019, 3:15 p.m. UTC | #5 
Jan Kiszka <jan.kiszka@siemens.com> writes:

> On 03.08.19 15:22, Jan Kiszka wrote:
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> Allows to shutdown a foreground session via ctrl-c.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>
>> 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;
>>       }
>>
>> --
>> 2.16.4
>>
>>
>
> ...and this one for you as well, Markus?

Cc: qemu-trivial
Laurent Vivier Nov. 12, 2019, 9:37 a.m. UTC | #6 
Le 08/11/2019 à 16:15, Markus Armbruster a écrit :
> Jan Kiszka <jan.kiszka@siemens.com> writes:
> 
>> On 03.08.19 15:22, Jan Kiszka wrote:
>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>
>>> Allows to shutdown a foreground session via ctrl-c.
>>>
>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>> ---
>>>
>>> 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;
>>>       }
>>>
>>> --
>>> 2.16.4
>>>
>>>
>>
>> ...and this one for you as well, Markus?
> 
> Cc: qemu-trivial
> 
> 

Applied to my trivial-patches branch.

Thanks,
Laurent
diff mbox series

Patch

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;
     }