Re: [Qemu-devel] [PATCH] display: stop using DT_NOGRAPHIC, use DT_NONE

[Paolo Bonzini <pbonzini@redhat.com>]

Il 28/06/2013 13:43, Michael Tokarev ha scritto:
>> > 
>> > All I'm saying is that I use -display none and I like that
>> > it defaults to "ctrl-c works and kills qemu" and I don't
>> > want you to break that :-)
> I'm not.  Code in qemu-char.c:
> 
> static bool stdio_allow_signal;
> 
> static void qemu_chr_set_echo_stdio(CharDriverState *chr, bool echo)
> {
>     ...
>     /* if graphical mode, we allow Ctrl-C handling */
>     if (!stdio_allow_signal)
>         tty.c_lflag &= ~ISIG;
>     ...
> }
> 
> static CharDriverState *qemu_chr_open_stdio(ChardevStdio *opts)
> {
>     ...
>     stdio_allow_signal = display_type != DT_NOGRAPHIC;
>     if (opts->has_signal) {
>         stdio_allow_signal = opts->signal;
>     }
>     ...
> }
> 
> 
> Note it is used only for stdio char device, like -serial stdio,
> so if you _just_ use -display none, without -serial stdio, you
> wont notice a change.
> 
> And note that original intention isn't exactly clear, either.
> How it is related with, say, curses display which needs raw
> keypresses?
> 
> Adding Paolo who wrote that code in bb002513.

I didn't, the code was already there:

    if (!qemu_opt_get_bool(opts, "signal", display_type != DT_NOGRAPHIC))
         tty.c_lflag &= ~ISIG;

I just did

-    if (!qemu_opt_get_bool(opts, "signal", display_type != DT_NOGRAPHIC))
+    if (!stdio_allow_signal)
          tty.c_lflag &= ~ISIG;
...
+    stdio_allow_signal = qemu_opt_get_bool(opts, "signal",
+                                           display_type != DT_NOGRAPHIC);

I think the idea is that "-nographic" sets up "-serial mon:stdio" so in
theory you do not need Ctrl-C and it is better to pass it to the VM.  Which
suggests this (untested) patch in turn:

-------------- 8< -----------------
From: Paolo Bonzini <pbonzini@redhat.com>
Subject: [PATCH] trap signals for "-serial mon:stdio"

With mon:stdio you can exit the VM by switching to the monitor and
sending the "quit" command.  It is then useful to pass Ctrl-C to the
VM instead of exiting.

This in turn lets us stop tying the default signal handling behavior
to -nographic, removing gratuitous differences between "-display none"
and "-nographic".

This patch changes behavior for "-display none -serial mon:stdio", as
expected, but not for "-display none -serial stdio".

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

diff --git a/qemu-char.c b/qemu-char.c
index a030e6b..dc42b58 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -955,7 +955,6 @@ static CharDriverState *qemu_chr_open_stdio(ChardevStdio *opts)
     chr = qemu_chr_open_fd(0, 1);
     chr->chr_close = qemu_chr_close_stdio;
     chr->chr_set_echo = qemu_chr_set_echo_stdio;
-    stdio_allow_signal = display_type != DT_NOGRAPHIC;
     if (opts->has_signal) {
         stdio_allow_signal = opts->signal;
     }
@@ -2929,7 +2928,10 @@ QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename)
 
     if (strstart(filename, "mon:", &p)) {
         filename = p;
-        qemu_opt_set(opts, "mux", "on");
+        qemu_opt_set(opts, "mux", "yes");
+        if (strcmp(filename, "stdio") == 0) {
+            qemu_opt_set(opts, "signal", "no");
+        }
     }
 
     if (strcmp(filename, "null")    == 0 ||
@@ -3058,8 +3060,7 @@ static void qemu_chr_parse_stdio(QemuOpts *opts, ChardevBackend *backend,
 {
     backend->stdio = g_new0(ChardevStdio, 1);
     backend->stdio->has_signal = true;
-    backend->stdio->signal =
-        qemu_opt_get_bool(opts, "signal", display_type != DT_NOGRAPHIC);
+    backend->stdio->signal = qemu_opt_get_bool(opts, "signal", true);
 }
 
 static void qemu_chr_parse_serial(QemuOpts *opts, ChardevBackend *backend,