Make qemu_shutdown_requested signal-safe

PATCH In review Jan Kiszka 3 participants 4 messages Oct 26, 2014 – Oct 27, 2014

lore

Share & tools

lore

Jan Kiszka

author

2014-10-26 09:32 UTC #0 lore raw

PATCH 1/1 Make qemu_shutdown_requested signal-safe

From: Jan Kiszka <[email protected]>

qemu_shutdown_requested may be interrupted by qemu_system_killed. If the latter sets shutdown_requested after qemu_shutdown_requested has read it but before it was cleared, the shutdown event is lost. Fix this by using atomic_xchg.

Signed-off-by Jan Kiszka <[email protected]>

---
 vl.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

vl.c +1 -3

index 2f81384..f6b3546 100644
@@ -1609,9 +1609,7 @@ int qemu_reset_requested_get(void)
 
 static int qemu_shutdown_requested(void)
 {
-    int r = shutdown_requested;
-    shutdown_requested = 0;
-    return r;
+    return atomic_xchg(&shutdown_requested, 0);
 }
 
 static void qemu_kill_report(void)

Signature · 1 line

1.8.4.5

Gonglei

Reviewed-by on v1 1/1

2014-10-26 09:45 UTC #1 lore raw

Quoted 17 lines · Jan Kiszka

From: Jan Kiszka <[email protected]>
qemu_shutdown_requested may be interrupted by qemu_system_killed. If the latter sets shutdown_requested after qemu_shutdown_requested has read it but before it was cleared, the shutdown event is lost. Fix this by using atomic_xchg.
Signed-off-by Jan Kiszka <[email protected]>
---
 vl.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)
vl.c +0 -0
index 2f81384..f6b3546 100644

vl.c int qemu_reset_requested_get(void) · line 1615

Quoted 10 lines

vl.c +1 -3

@@ -1609,9 +1609,7 @@ int qemu_reset_requested_get(void)
 
 static int qemu_shutdown_requested(void)
 {
-    int r = shutdown_requested;
-    shutdown_requested = 0;
-    return r;
+    return atomic_xchg(&shutdown_requested, 0);
 }
 
 static void qemu_kill_report(void)

Looks good to me, so

Reviewed-by Gonglei <[email protected]>

Best regards,
-Gonglei

Paolo Bonzini

Reviewed-by on v1 1/1

2014-10-27 09:10 UTC #2 lore raw

In reply to Jan Kiszka

Quoted 14 lines · Jan Kiszka

From: Jan Kiszka <[email protected]>
qemu_shutdown_requested may be interrupted by qemu_system_killed. If the latter sets shutdown_requested after qemu_shutdown_requested has read it but before it was cleared, the shutdown event is lost. Fix this by using atomic_xchg.
Signed-off-by Jan Kiszka <[email protected]> --- vl.c | 4
+--- 1 file changed, 1 insertion(+), 3 deletions(-)
vl.c index 2f81384..f6b3546 100644 --- a/vl.c renamed from vl.c +1 -0
+++ b/vl.c @@ -1609,9 +1609,7 @@ int 
qemu_reset_requested_get(void)

Quoted 6 lines

static int qemu_shutdown_requested(void) { - int r =
shutdown_requested; - shutdown_requested = 0; - return r; +
return atomic_xchg(&shutdown_requested, 0); }
static void qemu_kill_report(void)

Cc [email protected]

Reviewed-by Paolo Bonzini <[email protected]>

Are you going to send a pull request yourself?

Jan Kiszka

author

2014-10-27 09:12 UTC #3 lore raw

Quoted 19 lines · Paolo Bonzini

Quoted 18 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
qemu_shutdown_requested may be interrupted by qemu_system_killed. If the latter sets shutdown_requested after qemu_shutdown_requested has read it but before it was cleared, the shutdown event is lost. Fix this by using atomic_xchg.
Signed-off-by Jan Kiszka <[email protected]> --- vl.c | 4
+--- 1 file changed, 1 insertion(+), 3 deletions(-)
vl.c index 2f81384..f6b3546 100644 --- a/vl.c renamed from vl.c +1 -0
+++ b/vl.c @@ -1609,9 +1609,7 @@ int 
qemu_reset_requested_get(void)
static int qemu_shutdown_requested(void) { - int r =
shutdown_requested; - shutdown_requested = 0; - return r; +
return atomic_xchg(&shutdown_requested, 0); }

Quoted 8 lines

Quoted 2 lines
static void qemu_kill_report(void)
Cc [email protected]
Reviewed-by Paolo Bonzini <[email protected]>
Are you going to send a pull request yourself?

I was assuming Peter would merge (and decide about the ordering).

Jan