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

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

[Jan Kiszka]

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

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

[Claudio Fontana]

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>

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

[Stefano Garzarella]

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

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

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

[Jan Kiszka]

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

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

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

[Markus Armbruster]

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

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

[Laurent Vivier]

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