1. Patchew
  2. QEMU
  3. View series

[RFC PATCH] chardev: don't exit() straight away on C-a x

Alex Bennée posted 1 patch 2 years, 6 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20211018140226.838137-1-alex.bennee@linaro.org
There is a newer version of this series
chardev/char-mux.c                     | 3 ++-
stubs/{runstate-check.c => runstate.c} | 5 +++++
stubs/meson.build                      | 2 +-
3 files changed, 8 insertions(+), 2 deletions(-)
rename stubs/{runstate-check.c => runstate.c} (64%)
[RFC PATCH] chardev: don't exit() straight away on C-a x
Posted by Alex Bennée 2 years, 6 months ago 
While there are a number of uses in the code-base of the exit(0)
pattern it gets in the way of clean exit which can do all of it's
house-keeping. In particular it was reported that you can crash
plugins this way because TCG can still be running on other threads
when the atexit callback is called.

Use qemu_system_shutdown_request() instead. I did a gentle rename of
the runstate stub seeing as it now contains two functions.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reported-by: Lukas Jünger <lukas.junger@greensocs.com>
---
 chardev/char-mux.c                     | 3 ++-
 stubs/{runstate-check.c => runstate.c} | 5 +++++
 stubs/meson.build                      | 2 +-
 3 files changed, 8 insertions(+), 2 deletions(-)
 rename stubs/{runstate-check.c => runstate.c} (64%)

diff --git a/chardev/char-mux.c b/chardev/char-mux.c
index ada0c6866f..a46897fcd5 100644
--- a/chardev/char-mux.c
+++ b/chardev/char-mux.c
@@ -28,6 +28,7 @@
 #include "qemu/option.h"
 #include "chardev/char.h"
 #include "sysemu/block-backend.h"
+#include "sysemu/runstate.h"
 #include "chardev-internal.h"
 
 /* MUX driver for serial I/O splitting */
@@ -157,7 +158,7 @@ static int mux_proc_byte(Chardev *chr, MuxChardev *d, int ch)
             {
                  const char *term =  "QEMU: Terminated\n\r";
                  qemu_chr_write_all(chr, (uint8_t *)term, strlen(term));
-                 exit(0);
+                 qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
                  break;
             }
         case 's':
diff --git a/stubs/runstate-check.c b/stubs/runstate.c
similarity index 64%
rename from stubs/runstate-check.c
rename to stubs/runstate.c
index 2ccda2b70f..f47dbcd3e0 100644
--- a/stubs/runstate-check.c
+++ b/stubs/runstate.c
@@ -5,3 +5,8 @@ bool runstate_check(RunState state)
 {
     return state == RUN_STATE_PRELAUNCH;
 }
+
+void qemu_system_shutdown_request(ShutdownCause reason)
+{
+    return;
+}
diff --git a/stubs/meson.build b/stubs/meson.build
index f6aa3aa94f..8f6a9f17e5 100644
--- a/stubs/meson.build
+++ b/stubs/meson.build
@@ -35,7 +35,7 @@ stub_ss.add(files('qtest.c'))
 stub_ss.add(files('ram-block.c'))
 stub_ss.add(files('ramfb.c'))
 stub_ss.add(files('replay.c'))
-stub_ss.add(files('runstate-check.c'))
+stub_ss.add(files('runstate.c'))
 stub_ss.add(files('sysbus.c'))
 stub_ss.add(files('target-get-monitor-def.c'))
 stub_ss.add(files('target-monitor-defs.c'))
-- 
2.30.2
Re: [RFC PATCH] chardev: don't exit() straight away on C-a x
Posted by Marc-André Lureau 2 years, 6 months ago 
On Mon, Oct 18, 2021 at 6:06 PM Alex Bennée <alex.bennee@linaro.org> wrote:

> While there are a number of uses in the code-base of the exit(0)
> pattern it gets in the way of clean exit which can do all of it's
> house-keeping. In particular it was reported that you can crash
> plugins this way because TCG can still be running on other threads
> when the atexit callback is called.
>
> Use qemu_system_shutdown_request() instead. I did a gentle rename of
> the runstate stub seeing as it now contains two functions.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Reported-by: Lukas Jünger <lukas.junger@greensocs.com>
>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>  chardev/char-mux.c                     | 3 ++-
>  stubs/{runstate-check.c => runstate.c} | 5 +++++
>  stubs/meson.build                      | 2 +-
>  3 files changed, 8 insertions(+), 2 deletions(-)
>  rename stubs/{runstate-check.c => runstate.c} (64%)
>
> diff --git a/chardev/char-mux.c b/chardev/char-mux.c
> index ada0c6866f..a46897fcd5 100644
> --- a/chardev/char-mux.c
> +++ b/chardev/char-mux.c
> @@ -28,6 +28,7 @@
>  #include "qemu/option.h"
>  #include "chardev/char.h"
>  #include "sysemu/block-backend.h"
> +#include "sysemu/runstate.h"
>  #include "chardev-internal.h"
>
>  /* MUX driver for serial I/O splitting */
> @@ -157,7 +158,7 @@ static int mux_proc_byte(Chardev *chr, MuxChardev *d,
> int ch)
>              {
>                   const char *term =  "QEMU: Terminated\n\r";
>                   qemu_chr_write_all(chr, (uint8_t *)term, strlen(term));
> -                 exit(0);
> +
>  qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
>                   break;
>              }
>          case 's':
> diff --git a/stubs/runstate-check.c b/stubs/runstate.c
> similarity index 64%
> rename from stubs/runstate-check.c
> rename to stubs/runstate.c
> index 2ccda2b70f..f47dbcd3e0 100644
> --- a/stubs/runstate-check.c
> +++ b/stubs/runstate.c
> @@ -5,3 +5,8 @@ bool runstate_check(RunState state)
>  {
>      return state == RUN_STATE_PRELAUNCH;
>  }
> +
> +void qemu_system_shutdown_request(ShutdownCause reason)
> +{
> +    return;
> +}
> diff --git a/stubs/meson.build b/stubs/meson.build
> index f6aa3aa94f..8f6a9f17e5 100644
> --- a/stubs/meson.build
> +++ b/stubs/meson.build
> @@ -35,7 +35,7 @@ stub_ss.add(files('qtest.c'))
>  stub_ss.add(files('ram-block.c'))
>  stub_ss.add(files('ramfb.c'))
>  stub_ss.add(files('replay.c'))
> -stub_ss.add(files('runstate-check.c'))
> +stub_ss.add(files('runstate.c'))
>  stub_ss.add(files('sysbus.c'))
>  stub_ss.add(files('target-get-monitor-def.c'))
>  stub_ss.add(files('target-monitor-defs.c'))
> --
> 2.30.2
>
>
>

-- 
Marc-André Lureau
Re: [RFC PATCH] chardev: don't exit() straight away on C-a x
Posted by Paolo Bonzini 2 years, 6 months ago 
On 18/10/21 16:02, Alex Bennée wrote:
> While there are a number of uses in the code-base of the exit(0)
> pattern it gets in the way of clean exit which can do all of it's
> house-keeping. In particular it was reported that you can crash
> plugins this way because TCG can still be running on other threads
> when the atexit callback is called.
> 
> Use qemu_system_shutdown_request() instead. I did a gentle rename of
> the runstate stub seeing as it now contains two functions.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Reported-by: Lukas Jünger <lukas.junger@greensocs.com>

That won't work with -no-shutdown, but you can just call qmp_quit() instead.

Paolo

> ---
>   chardev/char-mux.c                     | 3 ++-
>   stubs/{runstate-check.c => runstate.c} | 5 +++++
>   stubs/meson.build                      | 2 +-
>   3 files changed, 8 insertions(+), 2 deletions(-)
>   rename stubs/{runstate-check.c => runstate.c} (64%)
> 
> diff --git a/chardev/char-mux.c b/chardev/char-mux.c
> index ada0c6866f..a46897fcd5 100644
> --- a/chardev/char-mux.c
> +++ b/chardev/char-mux.c
> @@ -28,6 +28,7 @@
>   #include "qemu/option.h"
>   #include "chardev/char.h"
>   #include "sysemu/block-backend.h"
> +#include "sysemu/runstate.h"
>   #include "chardev-internal.h"
>   
>   /* MUX driver for serial I/O splitting */
> @@ -157,7 +158,7 @@ static int mux_proc_byte(Chardev *chr, MuxChardev *d, int ch)
>               {
>                    const char *term =  "QEMU: Terminated\n\r";
>                    qemu_chr_write_all(chr, (uint8_t *)term, strlen(term));
> -                 exit(0);
> +                 qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
>                    break;
>               }
>           case 's':
> diff --git a/stubs/runstate-check.c b/stubs/runstate.c
> similarity index 64%
> rename from stubs/runstate-check.c
> rename to stubs/runstate.c
> index 2ccda2b70f..f47dbcd3e0 100644
> --- a/stubs/runstate-check.c
> +++ b/stubs/runstate.c
> @@ -5,3 +5,8 @@ bool runstate_check(RunState state)
>   {
>       return state == RUN_STATE_PRELAUNCH;
>   }
> +
> +void qemu_system_shutdown_request(ShutdownCause reason)
> +{
> +    return;
> +}
> diff --git a/stubs/meson.build b/stubs/meson.build
> index f6aa3aa94f..8f6a9f17e5 100644
> --- a/stubs/meson.build
> +++ b/stubs/meson.build
> @@ -35,7 +35,7 @@ stub_ss.add(files('qtest.c'))
>   stub_ss.add(files('ram-block.c'))
>   stub_ss.add(files('ramfb.c'))
>   stub_ss.add(files('replay.c'))
> -stub_ss.add(files('runstate-check.c'))
> +stub_ss.add(files('runstate.c'))
>   stub_ss.add(files('sysbus.c'))
>   stub_ss.add(files('target-get-monitor-def.c'))
>   stub_ss.add(files('target-monitor-defs.c'))
>
Re: [RFC PATCH] chardev: don't exit() straight away on C-a x
Posted by Alex Bennée 2 years, 6 months ago 
Paolo Bonzini <pbonzini@redhat.com> writes:

> On 18/10/21 16:02, Alex Bennée wrote:
>> While there are a number of uses in the code-base of the exit(0)
>> pattern it gets in the way of clean exit which can do all of it's
>> house-keeping. In particular it was reported that you can crash
>> plugins this way because TCG can still be running on other threads
>> when the atexit callback is called.
>> Use qemu_system_shutdown_request() instead. I did a gentle rename of
>> the runstate stub seeing as it now contains two functions.
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> Reported-by: Lukas Jünger <lukas.junger@greensocs.com>
>
> That won't work with -no-shutdown, but you can just call qmp_quit()
> instead.

How does calling qmp_quit() fix --no-shutdown? Isn't it just a thin
wrapper around the qemu_system_shutdown_request()?

>
> Paolo
>
>> ---
>>   chardev/char-mux.c                     | 3 ++-
>>   stubs/{runstate-check.c => runstate.c} | 5 +++++
>>   stubs/meson.build                      | 2 +-
>>   3 files changed, 8 insertions(+), 2 deletions(-)
>>   rename stubs/{runstate-check.c => runstate.c} (64%)
>> diff --git a/chardev/char-mux.c b/chardev/char-mux.c
>> index ada0c6866f..a46897fcd5 100644
>> --- a/chardev/char-mux.c
>> +++ b/chardev/char-mux.c
>> @@ -28,6 +28,7 @@
>>   #include "qemu/option.h"
>>   #include "chardev/char.h"
>>   #include "sysemu/block-backend.h"
>> +#include "sysemu/runstate.h"
>>   #include "chardev-internal.h"
>>     /* MUX driver for serial I/O splitting */
>> @@ -157,7 +158,7 @@ static int mux_proc_byte(Chardev *chr, MuxChardev *d, int ch)
>>               {
>>                    const char *term =  "QEMU: Terminated\n\r";
>>                    qemu_chr_write_all(chr, (uint8_t *)term, strlen(term));
>> -                 exit(0);
>> +                 qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
>>                    break;
>>               }
>>           case 's':
>> diff --git a/stubs/runstate-check.c b/stubs/runstate.c
>> similarity index 64%
>> rename from stubs/runstate-check.c
>> rename to stubs/runstate.c
>> index 2ccda2b70f..f47dbcd3e0 100644
>> --- a/stubs/runstate-check.c
>> +++ b/stubs/runstate.c
>> @@ -5,3 +5,8 @@ bool runstate_check(RunState state)
>>   {
>>       return state == RUN_STATE_PRELAUNCH;
>>   }
>> +
>> +void qemu_system_shutdown_request(ShutdownCause reason)
>> +{
>> +    return;
>> +}
>> diff --git a/stubs/meson.build b/stubs/meson.build
>> index f6aa3aa94f..8f6a9f17e5 100644
>> --- a/stubs/meson.build
>> +++ b/stubs/meson.build
>> @@ -35,7 +35,7 @@ stub_ss.add(files('qtest.c'))
>>   stub_ss.add(files('ram-block.c'))
>>   stub_ss.add(files('ramfb.c'))
>>   stub_ss.add(files('replay.c'))
>> -stub_ss.add(files('runstate-check.c'))
>> +stub_ss.add(files('runstate.c'))
>>   stub_ss.add(files('sysbus.c'))
>>   stub_ss.add(files('target-get-monitor-def.c'))
>>   stub_ss.add(files('target-monitor-defs.c'))
>> 


-- 
Alex Bennée
Re: [RFC PATCH] chardev: don't exit() straight away on C-a x
Posted by Paolo Bonzini 2 years, 6 months ago 
On 18/10/21 16:53, Alex Bennée wrote:
> 
> Paolo Bonzini <pbonzini@redhat.com> writes:
> 
>> On 18/10/21 16:02, Alex Bennée wrote:
>>> While there are a number of uses in the code-base of the exit(0)
>>> pattern it gets in the way of clean exit which can do all of it's
>>> house-keeping. In particular it was reported that you can crash
>>> plugins this way because TCG can still be running on other threads
>>> when the atexit callback is called.
>>> Use qemu_system_shutdown_request() instead. I did a gentle rename of
>>> the runstate stub seeing as it now contains two functions.
>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>> Reported-by: Lukas Jünger <lukas.junger@greensocs.com>
>>
>> That won't work with -no-shutdown, but you can just call qmp_quit()
>> instead.
> 
> How does calling qmp_quit() fix --no-shutdown? Isn't it just a thin
> wrapper around the qemu_system_shutdown_request()?

It first undoes the effect of -no-shutdown:

void qmp_quit(Error **errp)
{
     shutdown_action = SHUTDOWN_ACTION_POWEROFF;
     qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_QMP_QUIT);
}

Paolo
Re: [RFC PATCH] chardev: don't exit() straight away on C-a x
Posted by Alex Bennée 2 years, 6 months ago 
Paolo Bonzini <pbonzini@redhat.com> writes:

> On 18/10/21 16:53, Alex Bennée wrote:
>> Paolo Bonzini <pbonzini@redhat.com> writes:
>> 
>>> On 18/10/21 16:02, Alex Bennée wrote:
>>>> While there are a number of uses in the code-base of the exit(0)
>>>> pattern it gets in the way of clean exit which can do all of it's
>>>> house-keeping. In particular it was reported that you can crash
>>>> plugins this way because TCG can still be running on other threads
>>>> when the atexit callback is called.
>>>> Use qemu_system_shutdown_request() instead. I did a gentle rename of
>>>> the runstate stub seeing as it now contains two functions.
>>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>>> Reported-by: Lukas Jünger <lukas.junger@greensocs.com>
>>>
>>> That won't work with -no-shutdown, but you can just call qmp_quit()
>>> instead.
>> How does calling qmp_quit() fix --no-shutdown? Isn't it just a thin
>> wrapper around the qemu_system_shutdown_request()?
>
> It first undoes the effect of -no-shutdown:
>
> void qmp_quit(Error **errp)
> {
>     shutdown_action = SHUTDOWN_ACTION_POWEROFF;

I guess this is it? I couldn't follow the chain of qemu_opts to find
what sort of change -no-shutdown made to the shutdown_action.

>     qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_QMP_QUIT);
> }
>
> Paolo


-- 
Alex Bennée
Re: [RFC PATCH] chardev: don't exit() straight away on C-a x
Posted by Paolo Bonzini 2 years, 6 months ago 
On 18/10/21 19:20, Alex Bennée wrote:
>>      shutdown_action = SHUTDOWN_ACTION_POWEROFF;
> I guess this is it? I couldn't follow the chain of qemu_opts to find
> what sort of change -no-shutdown made to the shutdown_action.
> 

Yes, "-no-shutdown" is short for "-action shutdown=pause".  From there 
it goes process_runstate_actions -> qmp_marshal_set_action -> 
qmp_set_action, where it sets "shutdown_action = SHUTDOWN_ACTION_PAUSE".

Paolo
Re: [RFC PATCH] chardev: don't exit() straight away on C-a x
Posted by Philippe Mathieu-Daudé 2 years, 6 months ago 
On 10/18/21 16:02, Alex Bennée wrote:
> While there are a number of uses in the code-base of the exit(0)
> pattern it gets in the way of clean exit which can do all of it's
> house-keeping. In particular it was reported that you can crash
> plugins this way because TCG can still be running on other threads
> when the atexit callback is called.
> 
> Use qemu_system_shutdown_request() instead. I did a gentle rename of
> the runstate stub seeing as it now contains two functions.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Reported-by: Lukas Jünger <lukas.junger@greensocs.com>
> ---
>  chardev/char-mux.c                     | 3 ++-
>  stubs/{runstate-check.c => runstate.c} | 5 +++++
>  stubs/meson.build                      | 2 +-
>  3 files changed, 8 insertions(+), 2 deletions(-)
>  rename stubs/{runstate-check.c => runstate.c} (64%)
> 
> diff --git a/chardev/char-mux.c b/chardev/char-mux.c
> index ada0c6866f..a46897fcd5 100644
> --- a/chardev/char-mux.c
> +++ b/chardev/char-mux.c
> @@ -28,6 +28,7 @@
>  #include "qemu/option.h"
>  #include "chardev/char.h"
>  #include "sysemu/block-backend.h"
> +#include "sysemu/runstate.h"
>  #include "chardev-internal.h"
>  
>  /* MUX driver for serial I/O splitting */
> @@ -157,7 +158,7 @@ static int mux_proc_byte(Chardev *chr, MuxChardev *d, int ch)
>              {
>                   const char *term =  "QEMU: Terminated\n\r";
>                   qemu_chr_write_all(chr, (uint8_t *)term, strlen(term));
> -                 exit(0);
> +                 qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
>                   break;
>              }
>          case 's':
> diff --git a/stubs/runstate-check.c b/stubs/runstate.c
> similarity index 64%
> rename from stubs/runstate-check.c
> rename to stubs/runstate.c
> index 2ccda2b70f..f47dbcd3e0 100644
> --- a/stubs/runstate-check.c
> +++ b/stubs/runstate.c
> @@ -5,3 +5,8 @@ bool runstate_check(RunState state)
>  {
>      return state == RUN_STATE_PRELAUNCH;
>  }
> +
> +void qemu_system_shutdown_request(ShutdownCause reason)
> +{
> +    return;
> +}

Hmm this isn't a stub anymore, this is the user-mode implementation.

I'd rather have some shared user/ or meanwhile duplicate it in
both bsd-user/linux-user (even if the implementation is empty)
instead of a stub.

> diff --git a/stubs/meson.build b/stubs/meson.build
> index f6aa3aa94f..8f6a9f17e5 100644
> --- a/stubs/meson.build
> +++ b/stubs/meson.build
> @@ -35,7 +35,7 @@ stub_ss.add(files('qtest.c'))
>  stub_ss.add(files('ram-block.c'))
>  stub_ss.add(files('ramfb.c'))
>  stub_ss.add(files('replay.c'))
> -stub_ss.add(files('runstate-check.c'))
> +stub_ss.add(files('runstate.c'))
>  stub_ss.add(files('sysbus.c'))
>  stub_ss.add(files('target-get-monitor-def.c'))
>  stub_ss.add(files('target-monitor-defs.c'))
>
Re: [RFC PATCH] chardev: don't exit() straight away on C-a x
Posted by Alex Bennée 2 years, 6 months ago 
Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> On 10/18/21 16:02, Alex Bennée wrote:
>> While there are a number of uses in the code-base of the exit(0)
>> pattern it gets in the way of clean exit which can do all of it's
>> house-keeping. In particular it was reported that you can crash
>> plugins this way because TCG can still be running on other threads
>> when the atexit callback is called.
>> 
>> Use qemu_system_shutdown_request() instead. I did a gentle rename of
>> the runstate stub seeing as it now contains two functions.
>> 
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> Reported-by: Lukas Jünger <lukas.junger@greensocs.com>
>> ---
>>  chardev/char-mux.c                     | 3 ++-
>>  stubs/{runstate-check.c => runstate.c} | 5 +++++
>>  stubs/meson.build                      | 2 +-
>>  3 files changed, 8 insertions(+), 2 deletions(-)
>>  rename stubs/{runstate-check.c => runstate.c} (64%)
>> 
>> diff --git a/chardev/char-mux.c b/chardev/char-mux.c
>> index ada0c6866f..a46897fcd5 100644
>> --- a/chardev/char-mux.c
>> +++ b/chardev/char-mux.c
>> @@ -28,6 +28,7 @@
>>  #include "qemu/option.h"
>>  #include "chardev/char.h"
>>  #include "sysemu/block-backend.h"
>> +#include "sysemu/runstate.h"
>>  #include "chardev-internal.h"
>>  
>>  /* MUX driver for serial I/O splitting */
>> @@ -157,7 +158,7 @@ static int mux_proc_byte(Chardev *chr, MuxChardev *d, int ch)
>>              {
>>                   const char *term =  "QEMU: Terminated\n\r";
>>                   qemu_chr_write_all(chr, (uint8_t *)term, strlen(term));
>> -                 exit(0);
>> +                 qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
>>                   break;
>>              }
>>          case 's':
>> diff --git a/stubs/runstate-check.c b/stubs/runstate.c
>> similarity index 64%
>> rename from stubs/runstate-check.c
>> rename to stubs/runstate.c
>> index 2ccda2b70f..f47dbcd3e0 100644
>> --- a/stubs/runstate-check.c
>> +++ b/stubs/runstate.c
>> @@ -5,3 +5,8 @@ bool runstate_check(RunState state)
>>  {
>>      return state == RUN_STATE_PRELAUNCH;
>>  }
>> +
>> +void qemu_system_shutdown_request(ShutdownCause reason)
>> +{
>> +    return;
>> +}
>
> Hmm this isn't a stub anymore, this is the user-mode implementation.

It is? I don't think any of the chardev code touches user-mode, I had to
add this because apparently other binaries link the libchardev code.

> I'd rather have some shared user/ or meanwhile duplicate it in
> both bsd-user/linux-user (even if the implementation is empty)
> instead of a stub.
>
>> diff --git a/stubs/meson.build b/stubs/meson.build
>> index f6aa3aa94f..8f6a9f17e5 100644
>> --- a/stubs/meson.build
>> +++ b/stubs/meson.build
>> @@ -35,7 +35,7 @@ stub_ss.add(files('qtest.c'))
>>  stub_ss.add(files('ram-block.c'))
>>  stub_ss.add(files('ramfb.c'))
>>  stub_ss.add(files('replay.c'))
>> -stub_ss.add(files('runstate-check.c'))
>> +stub_ss.add(files('runstate.c'))
>>  stub_ss.add(files('sysbus.c'))
>>  stub_ss.add(files('target-get-monitor-def.c'))
>>  stub_ss.add(files('target-monitor-defs.c'))
>> 


-- 
Alex Bennée
Re: [RFC PATCH] chardev: don't exit() straight away on C-a x
Posted by Philippe Mathieu-Daudé 2 years, 6 months ago 
On 10/18/21 18:14, Alex Bennée wrote:
> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
>> On 10/18/21 16:02, Alex Bennée wrote:
>>> While there are a number of uses in the code-base of the exit(0)
>>> pattern it gets in the way of clean exit which can do all of it's
>>> house-keeping. In particular it was reported that you can crash
>>> plugins this way because TCG can still be running on other threads
>>> when the atexit callback is called.
>>>
>>> Use qemu_system_shutdown_request() instead. I did a gentle rename of
>>> the runstate stub seeing as it now contains two functions.
>>>
>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>> Reported-by: Lukas Jünger <lukas.junger@greensocs.com>
>>> ---
>>>  chardev/char-mux.c                     | 3 ++-
>>>  stubs/{runstate-check.c => runstate.c} | 5 +++++
>>>  stubs/meson.build                      | 2 +-
>>>  3 files changed, 8 insertions(+), 2 deletions(-)
>>>  rename stubs/{runstate-check.c => runstate.c} (64%)
>>>
>>> diff --git a/chardev/char-mux.c b/chardev/char-mux.c
>>> index ada0c6866f..a46897fcd5 100644
>>> --- a/chardev/char-mux.c
>>> +++ b/chardev/char-mux.c
>>> @@ -28,6 +28,7 @@
>>>  #include "qemu/option.h"
>>>  #include "chardev/char.h"
>>>  #include "sysemu/block-backend.h"
>>> +#include "sysemu/runstate.h"
>>>  #include "chardev-internal.h"
>>>  
>>>  /* MUX driver for serial I/O splitting */
>>> @@ -157,7 +158,7 @@ static int mux_proc_byte(Chardev *chr, MuxChardev *d, int ch)
>>>              {
>>>                   const char *term =  "QEMU: Terminated\n\r";
>>>                   qemu_chr_write_all(chr, (uint8_t *)term, strlen(term));
>>> -                 exit(0);
>>> +                 qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
>>>                   break;
>>>              }
>>>          case 's':
>>> diff --git a/stubs/runstate-check.c b/stubs/runstate.c
>>> similarity index 64%
>>> rename from stubs/runstate-check.c
>>> rename to stubs/runstate.c
>>> index 2ccda2b70f..f47dbcd3e0 100644
>>> --- a/stubs/runstate-check.c
>>> +++ b/stubs/runstate.c
>>> @@ -5,3 +5,8 @@ bool runstate_check(RunState state)
>>>  {
>>>      return state == RUN_STATE_PRELAUNCH;
>>>  }
>>> +
>>> +void qemu_system_shutdown_request(ShutdownCause reason)
>>> +{
>>> +    return;
>>> +}
>>
>> Hmm this isn't a stub anymore, this is the user-mode implementation.
> 
> It is? I don't think any of the chardev code touches user-mode, I had to
> add this because apparently other binaries link the libchardev code.

If it isn't then is should call g_assert_not_reached().

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.