| Message ID | 1272296853-30285-6-git-send-email-lcapitulino@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 04/26/2010 10:47 AM, Luiz Capitulino wrote: > The 'quit' Monitor command (implemented by do_quit()) calls > exit() directly, this is problematic under QMP because QEMU > exits before having a chance to send the ok response. > > Clients don't know if QEMU exited because of a problem or > because the 'quit' command has been executed. > > This commit fixes that by moving the exit() call to the main > loop, so that do_quit() requests the system to quit, instead > of calling exit() directly. > Does this also have the effect of printing out a (qemu) prompt after quit before an EOF appears on that socket? Regards, Anthony Liguori > Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com> > --- > monitor.c | 2 +- > sysemu.h | 2 ++ > vl.c | 18 ++++++++++++++++++ > 3 files changed, 21 insertions(+), 1 deletions(-) > > diff --git a/monitor.c b/monitor.c > index ef84298..611cbe9 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -1017,7 +1017,7 @@ static void do_info_cpu_stats(Monitor *mon) > */ > static int do_quit(Monitor *mon, const QDict *qdict, QObject **ret_data) > { > - exit(0); > + qemu_system_exit_request(); > return 0; > } > > diff --git a/sysemu.h b/sysemu.h > index d0effa0..fa921df 100644 > --- a/sysemu.h > +++ b/sysemu.h > @@ -45,9 +45,11 @@ void cpu_disable_ticks(void); > void qemu_system_reset_request(void); > void qemu_system_shutdown_request(void); > void qemu_system_powerdown_request(void); > +void qemu_system_exit_request(void); > int qemu_shutdown_requested(void); > int qemu_reset_requested(void); > int qemu_powerdown_requested(void); > +int qemu_exit_requested(void); > extern qemu_irq qemu_system_powerdown; > void qemu_system_reset(void); > > diff --git a/vl.c b/vl.c > index a5a0f41..9ef6f2c 100644 > --- a/vl.c > +++ b/vl.c > @@ -1697,6 +1697,7 @@ static int shutdown_requested; > static int powerdown_requested; > int debug_requested; > int vmstop_requested; > +static int exit_requested; > > int qemu_shutdown_requested(void) > { > @@ -1719,6 +1720,12 @@ int qemu_powerdown_requested(void) > return r; > } > > +int qemu_exit_requested(void) > +{ > + /* just return it, we'll exit() anyway */ > + return exit_requested; > +} > + > static int qemu_debug_requested(void) > { > int r = debug_requested; > @@ -1789,6 +1796,12 @@ void qemu_system_powerdown_request(void) > qemu_notify_event(); > } > > +void qemu_system_exit_request(void) > +{ > + exit_requested = 1; > + qemu_notify_event(); > +} > + > #ifdef _WIN32 > static void host_main_loop_wait(int *timeout) > { > @@ -1925,6 +1938,8 @@ static int vm_can_run(void) > return 0; > if (debug_requested) > return 0; > + if (exit_requested) > + return 0; > return 1; > } > > @@ -1977,6 +1992,9 @@ static void main_loop(void) > if ((r = qemu_vmstop_requested())) { > vm_stop(r); > } > + if (qemu_exit_requested()) { > + exit(0); > + } > } > pause_all_vcpus(); > } >
On Mon, 26 Apr 2010 12:49:40 -0500 Anthony Liguori <anthony@codemonkey.ws> wrote: > On 04/26/2010 10:47 AM, Luiz Capitulino wrote: > > The 'quit' Monitor command (implemented by do_quit()) calls > > exit() directly, this is problematic under QMP because QEMU > > exits before having a chance to send the ok response. > > > > Clients don't know if QEMU exited because of a problem or > > because the 'quit' command has been executed. > > > > This commit fixes that by moving the exit() call to the main > > loop, so that do_quit() requests the system to quit, instead > > of calling exit() directly. > > > > Does this also have the effect of printing out a (qemu) prompt after > quit before an EOF appears on that socket? Ah, right.. So, the easiest way to fix this is: if (user monitor) { exit(0); } else { go through main; } And, wrt to the pull, assuming you like the other patches, what's the best for you? Should I just drop this patch and ask you to pull again or can I do the fix, rebase, send it in this thread, and ping you?
On 04/26/2010 01:22 PM, Luiz Capitulino wrote: > On Mon, 26 Apr 2010 12:49:40 -0500 > Anthony Liguori<anthony@codemonkey.ws> wrote: > > >> On 04/26/2010 10:47 AM, Luiz Capitulino wrote: >> >>> The 'quit' Monitor command (implemented by do_quit()) calls >>> exit() directly, this is problematic under QMP because QEMU >>> exits before having a chance to send the ok response. >>> >>> Clients don't know if QEMU exited because of a problem or >>> because the 'quit' command has been executed. >>> >>> This commit fixes that by moving the exit() call to the main >>> loop, so that do_quit() requests the system to quit, instead >>> of calling exit() directly. >>> >>> >> Does this also have the effect of printing out a (qemu) prompt after >> quit before an EOF appears on that socket? >> > Ah, right.. > It's not necessarily a bad thing if it does. I just wanted to raise that because it's possible that someone depends on the behavior. I'm not sure it matters to me if we change this behavior though. > So, the easiest way to fix this is: > > if (user monitor) { > exit(0); > } else { > go through main; > } > > And, wrt to the pull, assuming you like the other patches, what's the best for you? > I'm happy to pull it, just wanted to see fi this issue was considered before I did. Regards, Anthony Liguori > Should I just drop this patch and ask you to pull again or can I do the fix, > rebase, send it in this thread, and ping you? >
diff --git a/monitor.c b/monitor.c index ef84298..611cbe9 100644 --- a/monitor.c +++ b/monitor.c @@ -1017,7 +1017,7 @@ static void do_info_cpu_stats(Monitor *mon) */ static int do_quit(Monitor *mon, const QDict *qdict, QObject **ret_data) { - exit(0); + qemu_system_exit_request(); return 0; } diff --git a/sysemu.h b/sysemu.h index d0effa0..fa921df 100644 --- a/sysemu.h +++ b/sysemu.h @@ -45,9 +45,11 @@ void cpu_disable_ticks(void); void qemu_system_reset_request(void); void qemu_system_shutdown_request(void); void qemu_system_powerdown_request(void); +void qemu_system_exit_request(void); int qemu_shutdown_requested(void); int qemu_reset_requested(void); int qemu_powerdown_requested(void); +int qemu_exit_requested(void); extern qemu_irq qemu_system_powerdown; void qemu_system_reset(void); diff --git a/vl.c b/vl.c index a5a0f41..9ef6f2c 100644 --- a/vl.c +++ b/vl.c @@ -1697,6 +1697,7 @@ static int shutdown_requested; static int powerdown_requested; int debug_requested; int vmstop_requested; +static int exit_requested; int qemu_shutdown_requested(void) { @@ -1719,6 +1720,12 @@ int qemu_powerdown_requested(void) return r; } +int qemu_exit_requested(void) +{ + /* just return it, we'll exit() anyway */ + return exit_requested; +} + static int qemu_debug_requested(void) { int r = debug_requested; @@ -1789,6 +1796,12 @@ void qemu_system_powerdown_request(void) qemu_notify_event(); } +void qemu_system_exit_request(void) +{ + exit_requested = 1; + qemu_notify_event(); +} + #ifdef _WIN32 static void host_main_loop_wait(int *timeout) { @@ -1925,6 +1938,8 @@ static int vm_can_run(void) return 0; if (debug_requested) return 0; + if (exit_requested) + return 0; return 1; } @@ -1977,6 +1992,9 @@ static void main_loop(void) if ((r = qemu_vmstop_requested())) { vm_stop(r); } + if (qemu_exit_requested()) { + exit(0); + } } pause_all_vcpus(); }
The 'quit' Monitor command (implemented by do_quit()) calls exit() directly, this is problematic under QMP because QEMU exits before having a chance to send the ok response. Clients don't know if QEMU exited because of a problem or because the 'quit' command has been executed. This commit fixes that by moving the exit() call to the main loop, so that do_quit() requests the system to quit, instead of calling exit() directly. Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> --- monitor.c | 2 +- sysemu.h | 2 ++ vl.c | 18 ++++++++++++++++++ 3 files changed, 21 insertions(+), 1 deletions(-)