Patchwork [V2] Use clean shutdown request for ctrl-a x

login
register
mail settings
Submitter Fabien Chouteau
Date July 4, 2012, 11:04 a.m.
Message ID <1341399884-2135-1-git-send-email-chouteau@adacore.com>
Download mbox | patch
Permalink /patch/168951/
State New
Headers show

Comments

Fabien Chouteau - July 4, 2012, 11:04 a.m.
The goal is to make ctrl-a x to close Qemu in a clean way. The current
exit(0) skips a lot of cleanup/close functions, for example in block
drivers.

Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
---
 qemu-char.c |    2 +-
 sysemu.h    |    1 +
 vl.c        |    5 +++++
 3 files changed, 7 insertions(+), 1 deletion(-)
Fabien Chouteau - July 9, 2012, 10:19 a.m.
Any comment?

On 07/04/2012 01:04 PM, Fabien Chouteau wrote:
> The goal is to make ctrl-a x to close Qemu in a clean way. The current
> exit(0) skips a lot of cleanup/close functions, for example in block
> drivers.
> 
> Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
> ---
>  qemu-char.c |    2 +-
>  sysemu.h    |    1 +
>  vl.c        |    5 +++++
>  3 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/qemu-char.c b/qemu-char.c
> index c2aaaee..7732846 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -353,7 +353,7 @@ static int mux_proc_byte(CharDriverState *chr, MuxDriver *d, int ch)
>              {
>                   const char *term =  "QEMU: Terminated\n\r";
>                   chr->chr_write(chr,(uint8_t *)term,strlen(term));
> -                 exit(0);
> +                 qemu_system_force_shutdown();
>                   break;
>              }
>          case 's':
> diff --git a/sysemu.h b/sysemu.h
> index bc2c788..2b2c354 100644
> --- a/sysemu.h
> +++ b/sysemu.h
> @@ -50,6 +50,7 @@ void qemu_register_suspend_notifier(Notifier *notifier);
>  void qemu_system_wakeup_request(WakeupReason reason);
>  void qemu_system_wakeup_enable(WakeupReason reason, bool enabled);
>  void qemu_register_wakeup_notifier(Notifier *notifier);
> +void qemu_system_force_shutdown(void);
>  void qemu_system_shutdown_request(void);
>  void qemu_system_powerdown_request(void);
>  void qemu_system_debug_request(void);
> diff --git a/vl.c b/vl.c
> index 1329c30..bd27583 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1477,6 +1477,11 @@ void qemu_system_killed(int signal, pid_t pid)
>  {
>      shutdown_signal = signal;
>      shutdown_pid = pid;
> +    qemu_system_force_shutdown();
> +}
> +
> +void qemu_system_force_shutdown(void)
> +{
>      no_shutdown = 0;
>      qemu_system_shutdown_request();
>  }
>
Paolo Bonzini - July 9, 2012, 10:20 a.m.
Il 09/07/2012 12:19, Fabien Chouteau ha scritto:
> Any comment?

Looks good to me.  Blue, can you apply it while Anthony is on holiday?

Paolo

> On 07/04/2012 01:04 PM, Fabien Chouteau wrote:
>> The goal is to make ctrl-a x to close Qemu in a clean way. The current
>> exit(0) skips a lot of cleanup/close functions, for example in block
>> drivers.
>>
>> Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
>> ---
>>  qemu-char.c |    2 +-
>>  sysemu.h    |    1 +
>>  vl.c        |    5 +++++
>>  3 files changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/qemu-char.c b/qemu-char.c
>> index c2aaaee..7732846 100644
>> --- a/qemu-char.c
>> +++ b/qemu-char.c
>> @@ -353,7 +353,7 @@ static int mux_proc_byte(CharDriverState *chr, MuxDriver *d, int ch)
>>              {
>>                   const char *term =  "QEMU: Terminated\n\r";
>>                   chr->chr_write(chr,(uint8_t *)term,strlen(term));
>> -                 exit(0);
>> +                 qemu_system_force_shutdown();
>>                   break;
>>              }
>>          case 's':
>> diff --git a/sysemu.h b/sysemu.h
>> index bc2c788..2b2c354 100644
>> --- a/sysemu.h
>> +++ b/sysemu.h
>> @@ -50,6 +50,7 @@ void qemu_register_suspend_notifier(Notifier *notifier);
>>  void qemu_system_wakeup_request(WakeupReason reason);
>>  void qemu_system_wakeup_enable(WakeupReason reason, bool enabled);
>>  void qemu_register_wakeup_notifier(Notifier *notifier);
>> +void qemu_system_force_shutdown(void);
>>  void qemu_system_shutdown_request(void);
>>  void qemu_system_powerdown_request(void);
>>  void qemu_system_debug_request(void);
>> diff --git a/vl.c b/vl.c
>> index 1329c30..bd27583 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -1477,6 +1477,11 @@ void qemu_system_killed(int signal, pid_t pid)
>>  {
>>      shutdown_signal = signal;
>>      shutdown_pid = pid;
>> +    qemu_system_force_shutdown();
>> +}
>> +
>> +void qemu_system_force_shutdown(void)
>> +{
>>      no_shutdown = 0;
>>      qemu_system_shutdown_request();
>>  }
>>
> 
>
Kevin Wolf - July 9, 2012, 10:27 a.m.
Am 09.07.2012 12:19, schrieb Fabien Chouteau:
> Any comment?
> 
> On 07/04/2012 01:04 PM, Fabien Chouteau wrote:
>> The goal is to make ctrl-a x to close Qemu in a clean way. The current
>> exit(0) skips a lot of cleanup/close functions, for example in block
>> drivers.
>>
>> Signed-off-by: Fabien Chouteau <chouteau@adacore.com>

Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Andreas Färber - July 9, 2012, 12:18 p.m.
Am 09.07.2012 12:19, schrieb Fabien Chouteau:
> Any comment?
> 
> On 07/04/2012 01:04 PM, Fabien Chouteau wrote:
>> The goal is to make ctrl-a x to close Qemu in a clean way. The current
>> exit(0) skips a lot of cleanup/close functions, for example in block
>> drivers.
>>
>> Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
>> ---
>>  qemu-char.c |    2 +-
>>  sysemu.h    |    1 +
>>  vl.c        |    5 +++++
>>  3 files changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/qemu-char.c b/qemu-char.c
>> index c2aaaee..7732846 100644
>> --- a/qemu-char.c
>> +++ b/qemu-char.c
>> @@ -353,7 +353,7 @@ static int mux_proc_byte(CharDriverState *chr, MuxDriver *d, int ch)
>>              {
>>                   const char *term =  "QEMU: Terminated\n\r";
>>                   chr->chr_write(chr,(uint8_t *)term,strlen(term));
>> -                 exit(0);
>> +                 qemu_system_force_shutdown();
>>                   break;
>>              }
>>          case 's':

FWIW there was a recent patch by Hervé that exposed further occurrences
of exit(), probably all would need to be reviewed and fixed.

Andreas
Fabien Chouteau - July 10, 2012, 10:16 a.m.
On 07/09/2012 02:18 PM, Andreas Färber wrote:
> Am 09.07.2012 12:19, schrieb Fabien Chouteau:
>> Any comment?
>>
>> On 07/04/2012 01:04 PM, Fabien Chouteau wrote:
>>> The goal is to make ctrl-a x to close Qemu in a clean way. The current
>>> exit(0) skips a lot of cleanup/close functions, for example in block
>>> drivers.
>>>
>>> Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
>>> ---
>>>  qemu-char.c |    2 +-
>>>  sysemu.h    |    1 +
>>>  vl.c        |    5 +++++
>>>  3 files changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/qemu-char.c b/qemu-char.c
>>> index c2aaaee..7732846 100644
>>> --- a/qemu-char.c
>>> +++ b/qemu-char.c
>>> @@ -353,7 +353,7 @@ static int mux_proc_byte(CharDriverState *chr, MuxDriver *d, int ch)
>>>              {
>>>                   const char *term =  "QEMU: Terminated\n\r";
>>>                   chr->chr_write(chr,(uint8_t *)term,strlen(term));
>>> -                 exit(0);
>>> +                 qemu_system_force_shutdown();
>>>                   break;
>>>              }
>>>          case 's':
> 
> FWIW there was a recent patch by Hervé that exposed further occurrences
> of exit(), probably all would need to be reviewed and fixed.
> 

There's a lot of exit() in Qemu (~660 with a quick grep), but it doesn't
always make sense to change them all.

In my opinion this one is a clean user request to shutdown the system,
as opposed to an error state that requires exit().

Patch

diff --git a/qemu-char.c b/qemu-char.c
index c2aaaee..7732846 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -353,7 +353,7 @@  static int mux_proc_byte(CharDriverState *chr, MuxDriver *d, int ch)
             {
                  const char *term =  "QEMU: Terminated\n\r";
                  chr->chr_write(chr,(uint8_t *)term,strlen(term));
-                 exit(0);
+                 qemu_system_force_shutdown();
                  break;
             }
         case 's':
diff --git a/sysemu.h b/sysemu.h
index bc2c788..2b2c354 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -50,6 +50,7 @@  void qemu_register_suspend_notifier(Notifier *notifier);
 void qemu_system_wakeup_request(WakeupReason reason);
 void qemu_system_wakeup_enable(WakeupReason reason, bool enabled);
 void qemu_register_wakeup_notifier(Notifier *notifier);
+void qemu_system_force_shutdown(void);
 void qemu_system_shutdown_request(void);
 void qemu_system_powerdown_request(void);
 void qemu_system_debug_request(void);
diff --git a/vl.c b/vl.c
index 1329c30..bd27583 100644
--- a/vl.c
+++ b/vl.c
@@ -1477,6 +1477,11 @@  void qemu_system_killed(int signal, pid_t pid)
 {
     shutdown_signal = signal;
     shutdown_pid = pid;
+    qemu_system_force_shutdown();
+}
+
+void qemu_system_force_shutdown(void)
+{
     no_shutdown = 0;
     qemu_system_shutdown_request();
 }