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

[Laurent Vivier]

Le 08/11/2019 à 16:15, Markus Armbruster a écrit :
> Jan Kiszka  writes:
> 
>> On 03.08.19 15:22, Jan Kiszka wrote:
>>> From: Jan Kiszka 
>>>
>>> Allows to shutdown a foreground session via ctrl-c.
>>>
>>> Signed-off-by: Jan Kiszka 
>>> ---
>>>
>>> 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(_quit.sa_mask) == -1 ||
>>> -sigaction(SIGTERM, _quit, 0) == -1) {
>>> -perror("failed to add SIGTERM handler; sigaction");
>>> +sigaction(SIGTERM, _quit, 0) == -1 ||
>>> +sigaction(SIGINT, _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

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

[Markus Armbruster]

Jan Kiszka  writes:

> On 03.08.19 15:22, Jan Kiszka wrote:
>> From: Jan Kiszka 
>>
>> Allows to shutdown a foreground session via ctrl-c.
>>
>> Signed-off-by: Jan Kiszka 
>> ---
>>
>> 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(_quit.sa_mask) == -1 ||
>> -sigaction(SIGTERM, _quit, 0) == -1) {
>> -perror("failed to add SIGTERM handler; sigaction");
>> +sigaction(SIGTERM, _quit, 0) == -1 ||
>> +sigaction(SIGINT, _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

[Jan Kiszka]

On 03.08.19 15:22, Jan Kiszka wrote:

From: Jan Kiszka 

Allows to shutdown a foreground session via ctrl-c.

Signed-off-by: Jan Kiszka 
---

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(_quit.sa_mask) == -1 ||
-sigaction(SIGTERM, _quit, 0) == -1) {
-perror("failed to add SIGTERM handler; sigaction");
+sigaction(SIGTERM, _quit, 0) == -1 ||
+sigaction(SIGINT, _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: [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 
>>
>> Allows to shutdown a foreground session via ctrl-c.
>>
>> Signed-off-by: Jan Kiszka 
>> ---
>>
>> 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(_quit.sa_mask) == -1 ||
>> -sigaction(SIGTERM, _quit, 0) == -1) {
>> -perror("failed to add SIGTERM handler; sigaction");
>> +sigaction(SIGTERM, _quit, 0) == -1 ||
>> +sigaction(SIGINT, _quit, 0) == -1) {
>> +perror("failed to add signal handler; sigaction");
>>  goto err;
>>  }
>
> Reviewed-by: Stefano Garzarella 

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: [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 
> 
> Allows to shutdown a foreground session via ctrl-c.
> 
> Signed-off-by: Jan Kiszka 
> ---
> 
> 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(_quit.sa_mask) == -1 ||
> -sigaction(SIGTERM, _quit, 0) == -1) {
> -perror("failed to add SIGTERM handler; sigaction");
> +sigaction(SIGTERM, _quit, 0) == -1 ||
> +sigaction(SIGINT, _quit, 0) == -1) {
> +perror("failed to add signal handler; sigaction");
>  goto err;
>  }

Reviewed-by: Stefano Garzarella 

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

[Claudio Fontana]

On 8/3/19 3:22 PM, Jan Kiszka wrote:
> From: Jan Kiszka 
> 
> Allows to shutdown a foreground session via ctrl-c.
> 
> Signed-off-by: Jan Kiszka 
> ---
> 
> 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(_quit.sa_mask) == -1 ||
> -sigaction(SIGTERM, _quit, 0) == -1) {
> -perror("failed to add SIGTERM handler; sigaction");
> +sigaction(SIGTERM, _quit, 0) == -1 ||
> +sigaction(SIGINT, _quit, 0) == -1) {
> +perror("failed to add signal handler; sigaction");
>  goto err;
>  }
> 
> --
> 2.16.4
> 
> 
Reviewed-by: Claudio Fontana 

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

[Jan Kiszka]

From: Jan Kiszka 

Allows to shutdown a foreground session via ctrl-c.

Signed-off-by: Jan Kiszka 
---

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(_quit.sa_mask) == -1 ||
-sigaction(SIGTERM, _quit, 0) == -1) {
-perror("failed to add SIGTERM handler; sigaction");
+sigaction(SIGTERM, _quit, 0) == -1 ||
+sigaction(SIGINT, _quit, 0) == -1) {
+perror("failed to add signal handler; sigaction");
 goto err;
 }

--
2.16.4