QEMU-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [Qemu-devel] [PATCH v2] ivshmem-server: Terminate also on SIGINT
@ 2019-08-03 13:22 Jan Kiszka
  2019-08-04  2:00 ` Claudio Fontana
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Jan Kiszka @ 2019-08-03 13:22 UTC (permalink / raw)
  To: qemu-devel; +Cc: Claudio Fontana

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] [PATCH v2] ivshmem-server: Terminate also on SIGINT
  2019-08-03 13:22 [Qemu-devel] [PATCH v2] ivshmem-server: Terminate also on SIGINT Jan Kiszka
@ 2019-08-04  2:00 ` Claudio Fontana
  2019-08-05  8:33 ` Stefano Garzarella
  2019-11-08  9:08 ` Jan Kiszka
  2 siblings, 0 replies; 7+ messages in thread
From: Claudio Fontana @ 2019-08-04  2:00 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: qemu-devel

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>


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] [PATCH v2] ivshmem-server: Terminate also on SIGINT
  2019-08-03 13:22 [Qemu-devel] [PATCH v2] ivshmem-server: Terminate also on SIGINT Jan Kiszka
  2019-08-04  2:00 ` Claudio Fontana
@ 2019-08-05  8:33 ` Stefano Garzarella
  2019-08-05 10:06   ` Jan Kiszka
  2019-11-08  9:08 ` Jan Kiszka
  2 siblings, 1 reply; 7+ messages in thread
From: Stefano Garzarella @ 2019-08-05  8:33 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: qemu-devel, Claudio Fontana

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] [PATCH v2] ivshmem-server: Terminate also on SIGINT
  2019-08-05  8:33 ` Stefano Garzarella
@ 2019-08-05 10:06   ` Jan Kiszka
  0 siblings, 0 replies; 7+ messages in thread
From: Jan Kiszka @ 2019-08-05 10:06 UTC (permalink / raw)
  To: Stefano Garzarella; +Cc: qemu-devel, Claudio Fontana

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



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] ivshmem-server: Terminate also on SIGINT
  2019-08-03 13:22 [Qemu-devel] [PATCH v2] ivshmem-server: Terminate also on SIGINT Jan Kiszka
  2019-08-04  2:00 ` Claudio Fontana
  2019-08-05  8:33 ` Stefano Garzarella
@ 2019-11-08  9:08 ` Jan Kiszka
  2019-11-08 15:15   ` Markus Armbruster
  2 siblings, 1 reply; 7+ messages in thread
From: Jan Kiszka @ 2019-11-08  9:08 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: qemu-devel, Claudio Fontana

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] ivshmem-server: Terminate also on SIGINT
  2019-11-08  9:08 ` Jan Kiszka
@ 2019-11-08 15:15   ` Markus Armbruster
  2019-11-12  9:37     ` Laurent Vivier
  0 siblings, 1 reply; 7+ messages in thread
From: Markus Armbruster @ 2019-11-08 15:15 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: qemu-trivial, qemu-devel, Claudio Fontana

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



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] ivshmem-server: Terminate also on SIGINT
  2019-11-08 15:15   ` Markus Armbruster
@ 2019-11-12  9:37     ` Laurent Vivier
  0 siblings, 0 replies; 7+ messages in thread
From: Laurent Vivier @ 2019-11-12  9:37 UTC (permalink / raw)
  To: Markus Armbruster, Jan Kiszka; +Cc: qemu-trivial, qemu-devel, Claudio Fontana

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-03 13:22 [Qemu-devel] [PATCH v2] ivshmem-server: Terminate also on SIGINT Jan Kiszka
2019-08-04  2:00 ` Claudio Fontana
2019-08-05  8:33 ` Stefano Garzarella
2019-08-05 10:06   ` Jan Kiszka
2019-11-08  9:08 ` Jan Kiszka
2019-11-08 15:15   ` Markus Armbruster
2019-11-12  9:37     ` Laurent Vivier

QEMU-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/qemu-devel/0 qemu-devel/git/0.git
	git clone --mirror https://lore.kernel.org/qemu-devel/1 qemu-devel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 qemu-devel qemu-devel/ https://lore.kernel.org/qemu-devel \
		qemu-devel@nongnu.org
	public-inbox-index qemu-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.nongnu.qemu-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git