[PATCH] ui/cocoa: Do not exit immediately after shutdown

[PATCH] ui/cocoa: Do not exit immediately after shutdown

[Akihiko Odaki]

ui/cocoa used to call exit immediately after calling
qemu_system_shutdown_request, which prevents QEMU from actually
perfoming system shutdown. Just sleep forever, and wait QEMU to call
exit and kill the Cocoa thread.

Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
---
 ui/cocoa.m | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index 13fba8103e1..65bb74134ca 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -1121,7 +1121,7 @@ - (void)applicationWillTerminate:(NSNotification *)aNotification
     COCOA_DEBUG("QemuCocoaAppController: applicationWillTerminate\n");
 
     qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_UI);
-    exit(0);
+    [NSThread sleepForTimeInterval:INFINITY];
 }
 
 - (BOOL)applicationShouldTerminateAfterLastWindowClosed:(NSApplication *)theApplication
-- 
2.24.3 (Apple Git-128)

Re: [PATCH] ui/cocoa: Do not exit immediately after shutdown

[Peter Maydell]

On Fri, 19 Feb 2021 at 08:46, Akihiko Odaki <akihiko.odaki@gmail.com> wrote:
>
> ui/cocoa used to call exit immediately after calling
> qemu_system_shutdown_request, which prevents QEMU from actually
> perfoming system shutdown. Just sleep forever, and wait QEMU to call
> exit and kill the Cocoa thread.
>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
> ---
>  ui/cocoa.m | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index 13fba8103e1..65bb74134ca 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -1121,7 +1121,7 @@ - (void)applicationWillTerminate:(NSNotification *)aNotification
>      COCOA_DEBUG("QemuCocoaAppController: applicationWillTerminate\n");
>
>      qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_UI);
> -    exit(0);
> +    [NSThread sleepForTimeInterval:INFINITY];

I think that as it stands this change is probably a bit risky,
because the QEMU action for "what do I do on a shutdown request"
is not necessarily "exit" -- the QMP 'set-action' command allows
a user to configure QEMU to just pause on a shutdown, which will
result in this loop going forever (or until OSX gets bored and
forcibly terminates the process).

It would also be useful to have a comment, something like:
 /*
  * Sleep here, because returning will cause OSX to kill us
  * immediately; the QEMU main loop will handle the shutdown
  * request and terminate the process.
  */

?

>  }

Gerd: I notice that the only ui frontends that try to do this
SHUTDOWN_CAUSE_HOST_UI thing are the Cocoa UI and the SDL2 UI.
The GTK UI does
        qmp_quit()
instead; the SDL2 UI does
        shutdown_action = SHUTDOWN_ACTION_POWEROFF;
        qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_UI);
(presumably to avoid the "maybe the user told us to 'pause'"
issue I mention above). None of the other UI frontends have
any shutdown related handling. Shouldn't we be consistent
about how we do this ?

thanks
-- PMM

[PATCH v2] ui/cocoa: Do not exit immediately after shutdown

[Akihiko Odaki]

ui/cocoa used to call exit immediately after calling
qemu_system_shutdown_request, which prevents QEMU from actually
perfoming system shutdown. Just sleep forever, and wait QEMU to call
exit and kill the Cocoa thread.

Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
---
 ui/cocoa.m | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index 13fba8103e1..cbd03667bd8 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -1121,7 +1121,13 @@ - (void)applicationWillTerminate:(NSNotification *)aNotification
     COCOA_DEBUG("QemuCocoaAppController: applicationWillTerminate\n");
 
     qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_UI);
-    exit(0);
+
+    /*
+     * Sleep here, because returning will cause OSX to kill us
+     * immediately; the QEMU main loop will handle the shutdown
+     * request and terminate the process.
+     */
+    [NSThread sleepForTimeInterval:INFINITY];
 }
 
 - (BOOL)applicationShouldTerminateAfterLastWindowClosed:(NSApplication *)theApplication
-- 
2.24.3 (Apple Git-128)

Re: [PATCH v2] ui/cocoa: Do not exit immediately after shutdown

[Gerd Hoffmann]

On Fri, Feb 19, 2021 at 08:16:52PM +0900, Akihiko Odaki wrote:
> ui/cocoa used to call exit immediately after calling
> qemu_system_shutdown_request, which prevents QEMU from actually
> perfoming system shutdown. Just sleep forever, and wait QEMU to call
> exit and kill the Cocoa thread.
> 
> Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>

Added to UI patch queue.

thanks,
  Gerd

Re: [PATCH] ui/cocoa: Do not exit immediately after shutdown

[Gerd Hoffmann]

> Gerd: I notice that the only ui frontends that try to do this
> SHUTDOWN_CAUSE_HOST_UI thing are the Cocoa UI and the SDL2 UI.
> The GTK UI does
>         qmp_quit()
> instead; the SDL2 UI does
>         shutdown_action = SHUTDOWN_ACTION_POWEROFF;
>         qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_UI);
> (presumably to avoid the "maybe the user told us to 'pause'"
> issue I mention above).

Seems so, qemu hangs in "[paused]" indeed when started with
-no-shutdown.

> None of the other UI frontends have
> any shutdown related handling. Shouldn't we be consistent
> about how we do this ?

Makes sense.  Probably best with a little 3-line helper called by all
UIs so we are consistent.

take care,
  Gerd