Patchwork make -qmp stdio usable

login
register
mail settings
Submitter Paolo Bonzini
Date Aug. 11, 2010, 9:56 p.m.
Message ID <1281563766-19391-1-git-send-email-pbonzini@redhat.com>
Download mbox | patch
Permalink /patch/61509/
State New
Headers show

Comments

Paolo Bonzini - Aug. 11, 2010, 9:56 p.m.
Currently -qmp stdio (or the equivalent -mon/-chardev combination) sets
up the terminal attributes even though it does not go through readline
to actually do I/O.  As a result, echo is disabled and you cannot see
anything you type.  This patch fixes it by adding a "cooked" option to
the stdio chardev backend, that when set will disable switching the tty
to raw mode.

Cc: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-char.c   |   26 ++++++++++++++------------
 qemu-config.c |    3 +++
 vl.c          |    3 +++
 3 files changed, 20 insertions(+), 12 deletions(-)
Anthony Liguori - Aug. 22, 2010, 9:50 p.m.
On 08/11/2010 04:56 PM, Paolo Bonzini wrote:
> Currently -qmp stdio (or the equivalent -mon/-chardev combination) sets
> up the terminal attributes even though it does not go through readline
> to actually do I/O.  As a result, echo is disabled and you cannot see
> anything you type.  This patch fixes it by adding a "cooked" option to
> the stdio chardev backend, that when set will disable switching the tty
> to raw mode.
>    

cooked doesn't make a whole lot of sense to me (and it's not documented 
anywhere).

Maybe raw=on|off or even echo=on|off would make more sense?

Regards,

Anthony Liguori

> Cc: Kevin Wolf<kwolf@redhat.com>
> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>
> ---
>   qemu-char.c   |   26 ++++++++++++++------------
>   qemu-config.c |    3 +++
>   vl.c          |    3 +++
>   3 files changed, 20 insertions(+), 12 deletions(-)
>
> diff --git a/qemu-char.c b/qemu-char.c
> index 6a3952c..15e1891 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -735,19 +735,21 @@ static void term_init(QemuOpts *opts)
>       oldtty = tty;
>       old_fd0_flags = fcntl(0, F_GETFL);
>
> -    tty.c_iflag&= ~(IGNBRK|BRKINT|PARMRK|ISTRIP
> +    if (!qemu_opt_get_bool(opts, "cooked", 0)) {
> +        tty.c_iflag&= ~(IGNBRK|BRKINT|PARMRK|ISTRIP
>                             |INLCR|IGNCR|ICRNL|IXON);
> -    tty.c_oflag |= OPOST;
> -    tty.c_lflag&= ~(ECHO|ECHONL|ICANON|IEXTEN);
> -    /* if graphical mode, we allow Ctrl-C handling */
> -    if (!qemu_opt_get_bool(opts, "signal", display_type != DT_NOGRAPHIC))
> -        tty.c_lflag&= ~ISIG;
> -    tty.c_cflag&= ~(CSIZE|PARENB);
> -    tty.c_cflag |= CS8;
> -    tty.c_cc[VMIN] = 1;
> -    tty.c_cc[VTIME] = 0;
> -
> -    tcsetattr (0, TCSANOW,&tty);
> +        tty.c_oflag |= OPOST;
> +        tty.c_lflag&= ~(ECHO|ECHONL|ICANON|IEXTEN);
> +        /* if graphical mode, we allow Ctrl-C handling */
> +        if (!qemu_opt_get_bool(opts, "signal", display_type != DT_NOGRAPHIC))
> +            tty.c_lflag&= ~ISIG;
> +        tty.c_cflag&= ~(CSIZE|PARENB);
> +        tty.c_cflag |= CS8;
> +        tty.c_cc[VMIN] = 1;
> +        tty.c_cc[VTIME] = 0;
> +
> +	tcsetattr (0, TCSANOW,&tty);
> +    }
>
>       if (!term_atexit_done++)
>           atexit(term_exit);
> diff --git a/qemu-config.c b/qemu-config.c
> index 95abe61..8c525b0 100644
> --- a/qemu-config.c
> +++ b/qemu-config.c
> @@ -146,6 +146,9 @@ QemuOptsList qemu_chardev_opts = {
>           },{
>               .name = "signal",
>               .type = QEMU_OPT_BOOL,
> +        },{
> +            .name = "cooked",
> +            .type = QEMU_OPT_BOOL,
>           },
>           { /* end if list */ }
>       },
> diff --git a/vl.c b/vl.c
> index b3e3676..be122e7 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1596,6 +1596,9 @@ static void monitor_parse(const char *optarg, const char *mode)
>               fprintf(stderr, "parse error: %s\n", optarg);
>               exit(1);
>           }
> +        if (!strcmp(mode, "control")) {
> +            qemu_opt_set(opts, "cooked", "on");
> +        }
>       }
>
>       opts = qemu_opts_create(&qemu_mon_opts, label, 1);
>
Paolo Bonzini - Aug. 23, 2010, 6:22 a.m.
On 08/22/2010 11:50 PM, Anthony Liguori wrote:
> On 08/11/2010 04:56 PM, Paolo Bonzini wrote:
>> Currently -qmp stdio (or the equivalent -mon/-chardev combination) sets
>> up the terminal attributes even though it does not go through readline
>> to actually do I/O. As a result, echo is disabled and you cannot see
>> anything you type. This patch fixes it by adding a "cooked" option to
>> the stdio chardev backend, that when set will disable switching the tty
>> to raw mode.
>
> cooked doesn't make a whole lot of sense to me (and it's not documented
> anywhere).

"Cooked mode" is actually pretty common when talking about the Unix 
TTYs, I can find it for example in curses documentation:

     Normally,  the  tty  driver buffers typed characters until a
     newline or carriage return is typed.  The cbreak routine disables
     line  buffering and erase/kill character-processing (interrupt and
     flow control characters are unaffected), making characters typed by
     the  user  immediately available to the program.  The nocbreak
     routine returns the terminal to normal (cooked) mode.

> Maybe raw=on|off or even echo=on|off would make more sense?

raw would make sense, echo was my first choice but it has the problem 
that echo is done anyway when Qemu's readline is involved.

I'll redo the patch with raw.

Paolo

Patch

diff --git a/qemu-char.c b/qemu-char.c
index 6a3952c..15e1891 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -735,19 +735,21 @@  static void term_init(QemuOpts *opts)
     oldtty = tty;
     old_fd0_flags = fcntl(0, F_GETFL);
 
-    tty.c_iflag &= ~(IGNBRK|BRKINT|PARMRK|ISTRIP
+    if (!qemu_opt_get_bool(opts, "cooked", 0)) {
+        tty.c_iflag &= ~(IGNBRK|BRKINT|PARMRK|ISTRIP
                           |INLCR|IGNCR|ICRNL|IXON);
-    tty.c_oflag |= OPOST;
-    tty.c_lflag &= ~(ECHO|ECHONL|ICANON|IEXTEN);
-    /* if graphical mode, we allow Ctrl-C handling */
-    if (!qemu_opt_get_bool(opts, "signal", display_type != DT_NOGRAPHIC))
-        tty.c_lflag &= ~ISIG;
-    tty.c_cflag &= ~(CSIZE|PARENB);
-    tty.c_cflag |= CS8;
-    tty.c_cc[VMIN] = 1;
-    tty.c_cc[VTIME] = 0;
-
-    tcsetattr (0, TCSANOW, &tty);
+        tty.c_oflag |= OPOST;
+        tty.c_lflag &= ~(ECHO|ECHONL|ICANON|IEXTEN);
+        /* if graphical mode, we allow Ctrl-C handling */
+        if (!qemu_opt_get_bool(opts, "signal", display_type != DT_NOGRAPHIC))
+            tty.c_lflag &= ~ISIG;
+        tty.c_cflag &= ~(CSIZE|PARENB);
+        tty.c_cflag |= CS8;
+        tty.c_cc[VMIN] = 1;
+        tty.c_cc[VTIME] = 0;
+
+	tcsetattr (0, TCSANOW, &tty);
+    }
 
     if (!term_atexit_done++)
         atexit(term_exit);
diff --git a/qemu-config.c b/qemu-config.c
index 95abe61..8c525b0 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -146,6 +146,9 @@  QemuOptsList qemu_chardev_opts = {
         },{
             .name = "signal",
             .type = QEMU_OPT_BOOL,
+        },{
+            .name = "cooked",
+            .type = QEMU_OPT_BOOL,
         },
         { /* end if list */ }
     },
diff --git a/vl.c b/vl.c
index b3e3676..be122e7 100644
--- a/vl.c
+++ b/vl.c
@@ -1596,6 +1596,9 @@  static void monitor_parse(const char *optarg, const char *mode)
             fprintf(stderr, "parse error: %s\n", optarg);
             exit(1);
         }
+        if (!strcmp(mode, "control")) {
+            qemu_opt_set(opts, "cooked", "on");
+        }
     }
 
     opts = qemu_opts_create(&qemu_mon_opts, label, 1);