| Message ID | 20211018140226.838137-1-alex.bennee@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC] chardev: don't exit() straight away on C-a x | expand |
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 <div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Oct 18, 2021 at 6:06 PM Alex Bennée <<a href="mailto:alex.bennee@linaro.org">alex.bennee@linaro.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">While there are a number of uses in the code-base of the exit(0)<br> pattern it gets in the way of clean exit which can do all of it's<br> house-keeping. In particular it was reported that you can crash<br> plugins this way because TCG can still be running on other threads<br> when the atexit callback is called.<br> <br> Use qemu_system_shutdown_request() instead. I did a gentle rename of<br> the runstate stub seeing as it now contains two functions.<br> <br> Signed-off-by: Alex Bennée <<a href="mailto:alex.bennee@linaro.org" target="_blank">alex.bennee@linaro.org</a>><br> Reported-by: Lukas Jünger <<a href="mailto:lukas.junger@greensocs.com" target="_blank">lukas.junger@greensocs.com</a>><br></blockquote><div><br></div><div>Reviewed-by: Marc-André Lureau <<a href="mailto:marcandre.lureau@redhat.com">marcandre.lureau@redhat.com</a>> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> ---<br> chardev/char-mux.c | 3 ++-<br> stubs/{runstate-check.c => runstate.c} | 5 +++++<br> stubs/meson.build | 2 +-<br> 3 files changed, 8 insertions(+), 2 deletions(-)<br> rename stubs/{runstate-check.c => runstate.c} (64%)<br> <br> diff --git a/chardev/char-mux.c b/chardev/char-mux.c<br> index ada0c6866f..a46897fcd5 100644<br> --- a/chardev/char-mux.c<br> +++ b/chardev/char-mux.c<br> @@ -28,6 +28,7 @@<br> #include "qemu/option.h"<br> #include "chardev/char.h"<br> #include "sysemu/block-backend.h"<br> +#include "sysemu/runstate.h"<br> #include "chardev-internal.h"<br> <br> /* MUX driver for serial I/O splitting */<br> @@ -157,7 +158,7 @@ static int mux_proc_byte(Chardev *chr, MuxChardev *d, int ch)<br> {<br> const char *term = "QEMU: Terminated\n\r";<br> qemu_chr_write_all(chr, (uint8_t *)term, strlen(term));<br> - exit(0);<br> + qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);<br> break;<br> }<br> case 's':<br> diff --git a/stubs/runstate-check.c b/stubs/runstate.c<br> similarity index 64%<br> rename from stubs/runstate-check.c<br> rename to stubs/runstate.c<br> index 2ccda2b70f..f47dbcd3e0 100644<br> --- a/stubs/runstate-check.c<br> +++ b/stubs/runstate.c<br> @@ -5,3 +5,8 @@ bool runstate_check(RunState state)<br> {<br> return state == RUN_STATE_PRELAUNCH;<br> }<br> +<br> +void qemu_system_shutdown_request(ShutdownCause reason)<br> +{<br> + return;<br> +}<br> diff --git a/stubs/meson.build b/stubs/meson.build<br> index f6aa3aa94f..8f6a9f17e5 100644<br> --- a/stubs/meson.build<br> +++ b/stubs/meson.build<br> @@ -35,7 +35,7 @@ stub_ss.add(files('qtest.c'))<br> stub_ss.add(files('ram-block.c'))<br> stub_ss.add(files('ramfb.c'))<br> stub_ss.add(files('replay.c'))<br> -stub_ss.add(files('runstate-check.c'))<br> +stub_ss.add(files('runstate.c'))<br> stub_ss.add(files('sysbus.c'))<br> stub_ss.add(files('target-get-monitor-def.c'))<br> stub_ss.add(files('target-monitor-defs.c'))<br> -- <br> 2.30.2<br> <br> <br> </blockquote></div><br clear="all"><br>-- <br><div dir="ltr" class="gmail_signature">Marc-André Lureau<br></div></div>
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')) >
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
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
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')) >
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
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
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().
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
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'))
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%) -- 2.30.2