On Mon, Sep 4, 2023 at 11:23 AM Peter Maydell <[email protected]> wrote: > > On Wed, 23 Aug 2023 at 08:07, Clément Chigot <[email protected]> wrote: > > > > This replaces the exit(0) call by a shutdown request, ensuring a proper > > cleanup of Qemu. Otherwise, some connections could be broken without > > being correctly flushed. > > > > Signed-off-by: Clément Chigot <[email protected]> > > --- > > gdbstub/gdbstub.c | 3 +-- > > gdbstub/softmmu.c | 13 +++++++++++++ > > gdbstub/user.c | 2 ++ > > 3 files changed, 16 insertions(+), 2 deletions(-) > > > > diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c > > index 5f28d5cf57..358eed1935 100644 > > --- a/gdbstub/gdbstub.c > > +++ b/gdbstub/gdbstub.c > > @@ -1298,7 +1298,6 @@ static void handle_v_kill(GArray *params, void > > *user_ctx) > > gdb_put_packet("OK"); > > error_report("QEMU: Terminated via GDBstub"); > > gdb_exit(0); > > - exit(0); > > } > > > > static const GdbCmdParseEntry gdb_v_commands_table[] = { > > @@ -1818,7 +1817,7 @@ static int gdb_handle_packet(const char *line_buf) > > /* Kill the target */ > > error_report("QEMU: Terminated via GDBstub"); > > gdb_exit(0); > > - exit(0); > > + break; > > case 'D': > > { > > static const GdbCmdParseEntry detach_cmd_desc = { > > diff --git a/gdbstub/softmmu.c b/gdbstub/softmmu.c > > index f509b7285d..fa9b09537d 100644 > > --- a/gdbstub/softmmu.c > > +++ b/gdbstub/softmmu.c > > @@ -434,6 +434,19 @@ void gdb_exit(int code) > > } > > > > qemu_chr_fe_deinit(&gdbserver_system_state.chr, true); > > + > > + /* > > + * Shutdown request is a clean way to stop the QEMU, compared > > + * to a direct call to exit(). But we can't pass the exit code > > + * through it so avoid doing that when it can matter. > > + * As this function is also called during the cleanup process, > > + * avoid sending the request if one is already set. > > + */ > > + if (code) { > > + exit(code); > > + } else if (!qemu_shutdown_requested_get()) { > > + qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN); > > + } > > } > > This definitely doesn't look right. Either exit() is OK > to call, or it is not. We shouldn't be exiting one way > if the exit status is 0 and another way if it is non-0.
I do agree but AFAIK, this isn't possible to pass the exit code using qemu_system_shutdown_request. Would it make more sense to trigger a SHUTDOWN_CAUSE_GUEST_PANIC instead ? This would result in a non-0 exit code and the already available gdb_trace would show the real exit code if needed. Clément
