1. Patchew
  2. QEMU
  3. View series

[Qemu-devel] [PATCH v2] qemu-io: fix EOF Ctrl-D handling in qemu-io readline code

Daniel P. Berrange posted 1 patch 8 years, 2 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20171208133416.28978-1-berrange@redhat.com
Test checkpatch passed
Test docker-build@min-glib passed
Test docker-mingw@fedora passed
Test docker-quick@centos6 passed
Test ppc passed
Test s390x passed
There is a newer version of this series
qemu-io.c | 26 +++++++++++++++++++++++++-
1 file changed, 25 insertions(+), 1 deletion(-)
[Qemu-devel] [PATCH v2] qemu-io: fix EOF Ctrl-D handling in qemu-io readline code
Posted by Daniel P. Berrange 8 years, 2 months ago 
qemu-io puts the TTY into non-canonical mode, which means no EOF processing is
done and thus getchar() will never return the EOF constant. Instead we have to
query the TTY attributes to determine the configured EOF character (usually
Ctrl-D / 0x4), and then explicitly check for that value. This fixes the
regression that prevented Ctrl-D from triggering an exit of qemu-io that has
existed since readline was first added in

  commit 0cf17e181798063c3824c8200ba46f25f54faa1a
  Author: Stefan Hajnoczi <stefanha@redhat.com>
  Date:   Thu Nov 14 11:54:17 2013 +0100

    qemu-io: use readline.c

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
Changed in v2:

  - Query termios settings for EOF character

 qemu-io.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/qemu-io.c b/qemu-io.c
index c70bde3eb1..fa4972d734 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -10,6 +10,9 @@
 #include "qemu/osdep.h"
 #include <getopt.h>
 #include <libgen.h>
+#ifndef _WIN32
+#include <termios.h>
+#endif
 
 #include "qapi/error.h"
 #include "qemu-io.h"
@@ -41,6 +44,26 @@ static bool imageOpts;
 
 static ReadLineState *readline_state;
 
+static int ttyEOF;
+
+static int get_eof_char(void)
+{
+#ifdef _WIN32
+    return 0x4; /* Ctrl-D */
+#else
+    struct termios tty;
+    if (tcgetattr(STDIN_FILENO, &tty) != 0) {
+        if (errno == ENOTTY) {
+            return 0x0; /* just expect read() == 0 */
+        } else {
+            return 0x4; /* Ctrl-D */
+        }
+    }
+
+    return tty.c_cc[VEOF];
+#endif
+}
+
 static int close_f(BlockBackend *blk, int argc, char **argv)
 {
     blk_unref(qemuio_blk);
@@ -322,7 +345,7 @@ static char *fetchline_readline(void)
     readline_start(readline_state, get_prompt(), 0, readline_func, &line);
     while (!line) {
         int ch = getchar();
-        if (ch == EOF) {
+        if (ttyEOF != 0x0 && ch == ttyEOF) {
             break;
         }
         readline_handle_byte(readline_state, ch);
@@ -592,6 +615,7 @@ int main(int argc, char **argv)
     qemuio_add_command(&close_cmd);
 
     if (isatty(STDIN_FILENO)) {
+        ttyEOF = get_eof_char();
         readline_state = readline_init(readline_printf_func,
                                        readline_flush_func,
                                        NULL,
-- 
2.14.3
Re: [Qemu-devel] [PATCH v2] qemu-io: fix EOF Ctrl-D handling in qemu-io readline code
Posted by Eric Blake 8 years, 2 months ago 
On 12/08/2017 07:34 AM, Daniel P. Berrange wrote:
> qemu-io puts the TTY into non-canonical mode, which means no EOF processing is
> done and thus getchar() will never return the EOF constant. Instead we have to
> query the TTY attributes to determine the configured EOF character (usually
> Ctrl-D / 0x4), and then explicitly check for that value. This fixes the
> regression that prevented Ctrl-D from triggering an exit of qemu-io that has
> existed since readline was first added in
> 
>   commit 0cf17e181798063c3824c8200ba46f25f54faa1a
>   Author: Stefan Hajnoczi <stefanha@redhat.com>
>   Date:   Thu Nov 14 11:54:17 2013 +0100
> 
>     qemu-io: use readline.c
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
> Changed in v2:

> +++ b/qemu-io.c
> @@ -10,6 +10,9 @@
>  #include "qemu/osdep.h"
>  #include <getopt.h>
>  #include <libgen.h>
> +#ifndef _WIN32
> +#include <termios.h>
> +#endif

Wouldn't a configure probe for the existence of <termios.h> be more
reliable than just hard-coding the list of platforms where it is
currently not found?

>  
>  #include "qapi/error.h"
>  #include "qemu-io.h"
> @@ -41,6 +44,26 @@ static bool imageOpts;
>  
>  static ReadLineState *readline_state;
>  
> +static int ttyEOF;
> +
> +static int get_eof_char(void)
> +{
> +#ifdef _WIN32

in which case this also should be #if HAVE_TERMIOS_H

But I guess unless someone complains, all other platforms that we care
about have termios.h, so a weak:
Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH v2] qemu-io: fix EOF Ctrl-D handling in qemu-io readline code
Posted by Daniel P. Berrange 8 years, 2 months ago 
On Fri, Dec 08, 2017 at 10:21:35AM -0600, Eric Blake wrote:
> On 12/08/2017 07:34 AM, Daniel P. Berrange wrote:
> > qemu-io puts the TTY into non-canonical mode, which means no EOF processing is
> > done and thus getchar() will never return the EOF constant. Instead we have to
> > query the TTY attributes to determine the configured EOF character (usually
> > Ctrl-D / 0x4), and then explicitly check for that value. This fixes the
> > regression that prevented Ctrl-D from triggering an exit of qemu-io that has
> > existed since readline was first added in
> > 
> >   commit 0cf17e181798063c3824c8200ba46f25f54faa1a
> >   Author: Stefan Hajnoczi <stefanha@redhat.com>
> >   Date:   Thu Nov 14 11:54:17 2013 +0100
> > 
> >     qemu-io: use readline.c
> > 
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > ---
> > Changed in v2:
> 
> > +++ b/qemu-io.c
> > @@ -10,6 +10,9 @@
> >  #include "qemu/osdep.h"
> >  #include <getopt.h>
> >  #include <libgen.h>
> > +#ifndef _WIN32
> > +#include <termios.h>
> > +#endif
> 
> Wouldn't a configure probe for the existence of <termios.h> be more
> reliable than just hard-coding the list of platforms where it is
> currently not found?
> 
> >  
> >  #include "qapi/error.h"
> >  #include "qemu-io.h"
> > @@ -41,6 +44,26 @@ static bool imageOpts;
> >  
> >  static ReadLineState *readline_state;
> >  
> > +static int ttyEOF;
> > +
> > +static int get_eof_char(void)
> > +{
> > +#ifdef _WIN32
> 
> in which case this also should be #if HAVE_TERMIOS_H
> 
> But I guess unless someone complains, all other platforms that we care
> about have termios.h, so a weak:

FYI, vl.c already includes termios.h, with merely  #ifndef _WIN32
protection, so that shows this is sufficient for our immediate
needs.

> Reviewed-by: Eric Blake <eblake@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [Qemu-devel] [PATCH v2] qemu-io: fix EOF Ctrl-D handling in qemu-io readline code
Posted by Daniel P. Berrangé 8 years ago 
ping, does any block maintainer want to queue this one ?

On Fri, Dec 08, 2017 at 01:34:16PM +0000, Daniel P. Berrange wrote:
> qemu-io puts the TTY into non-canonical mode, which means no EOF processing is
> done and thus getchar() will never return the EOF constant. Instead we have to
> query the TTY attributes to determine the configured EOF character (usually
> Ctrl-D / 0x4), and then explicitly check for that value. This fixes the
> regression that prevented Ctrl-D from triggering an exit of qemu-io that has
> existed since readline was first added in
> 
>   commit 0cf17e181798063c3824c8200ba46f25f54faa1a
>   Author: Stefan Hajnoczi <stefanha@redhat.com>
>   Date:   Thu Nov 14 11:54:17 2013 +0100
> 
>     qemu-io: use readline.c
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
> Changed in v2:
> 
>   - Query termios settings for EOF character
> 
>  qemu-io.c | 26 +++++++++++++++++++++++++-
>  1 file changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/qemu-io.c b/qemu-io.c
> index c70bde3eb1..fa4972d734 100644
> --- a/qemu-io.c
> +++ b/qemu-io.c
> @@ -10,6 +10,9 @@
>  #include "qemu/osdep.h"
>  #include <getopt.h>
>  #include <libgen.h>
> +#ifndef _WIN32
> +#include <termios.h>
> +#endif
>  
>  #include "qapi/error.h"
>  #include "qemu-io.h"
> @@ -41,6 +44,26 @@ static bool imageOpts;
>  
>  static ReadLineState *readline_state;
>  
> +static int ttyEOF;
> +
> +static int get_eof_char(void)
> +{
> +#ifdef _WIN32
> +    return 0x4; /* Ctrl-D */
> +#else
> +    struct termios tty;
> +    if (tcgetattr(STDIN_FILENO, &tty) != 0) {
> +        if (errno == ENOTTY) {
> +            return 0x0; /* just expect read() == 0 */
> +        } else {
> +            return 0x4; /* Ctrl-D */
> +        }
> +    }
> +
> +    return tty.c_cc[VEOF];
> +#endif
> +}
> +
>  static int close_f(BlockBackend *blk, int argc, char **argv)
>  {
>      blk_unref(qemuio_blk);
> @@ -322,7 +345,7 @@ static char *fetchline_readline(void)
>      readline_start(readline_state, get_prompt(), 0, readline_func, &line);
>      while (!line) {
>          int ch = getchar();
> -        if (ch == EOF) {
> +        if (ttyEOF != 0x0 && ch == ttyEOF) {
>              break;
>          }
>          readline_handle_byte(readline_state, ch);
> @@ -592,6 +615,7 @@ int main(int argc, char **argv)
>      qemuio_add_command(&close_cmd);
>  
>      if (isatty(STDIN_FILENO)) {
> +        ttyEOF = get_eof_char();
>          readline_state = readline_init(readline_printf_func,
>                                         readline_flush_func,
>                                         NULL,
> -- 
> 2.14.3
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [Qemu-devel] [PATCH v2] qemu-io: fix EOF Ctrl-D handling in qemu-io readline code
Posted by Daniel P. Berrangé 7 years, 12 months ago 
Re-ping.

On Thu, Jan 25, 2018 at 05:05:01PM +0000, Daniel P. Berrangé wrote:
> ping, does any block maintainer want to queue this one ?
> 
> On Fri, Dec 08, 2017 at 01:34:16PM +0000, Daniel P. Berrange wrote:
> > qemu-io puts the TTY into non-canonical mode, which means no EOF processing is
> > done and thus getchar() will never return the EOF constant. Instead we have to
> > query the TTY attributes to determine the configured EOF character (usually
> > Ctrl-D / 0x4), and then explicitly check for that value. This fixes the
> > regression that prevented Ctrl-D from triggering an exit of qemu-io that has
> > existed since readline was first added in
> > 
> >   commit 0cf17e181798063c3824c8200ba46f25f54faa1a
> >   Author: Stefan Hajnoczi <stefanha@redhat.com>
> >   Date:   Thu Nov 14 11:54:17 2013 +0100
> > 
> >     qemu-io: use readline.c
> > 
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > ---
> > Changed in v2:
> > 
> >   - Query termios settings for EOF character
> > 
> >  qemu-io.c | 26 +++++++++++++++++++++++++-
> >  1 file changed, 25 insertions(+), 1 deletion(-)
> > 
> > diff --git a/qemu-io.c b/qemu-io.c
> > index c70bde3eb1..fa4972d734 100644
> > --- a/qemu-io.c
> > +++ b/qemu-io.c
> > @@ -10,6 +10,9 @@
> >  #include "qemu/osdep.h"
> >  #include <getopt.h>
> >  #include <libgen.h>
> > +#ifndef _WIN32
> > +#include <termios.h>
> > +#endif
> >  
> >  #include "qapi/error.h"
> >  #include "qemu-io.h"
> > @@ -41,6 +44,26 @@ static bool imageOpts;
> >  
> >  static ReadLineState *readline_state;
> >  
> > +static int ttyEOF;
> > +
> > +static int get_eof_char(void)
> > +{
> > +#ifdef _WIN32
> > +    return 0x4; /* Ctrl-D */
> > +#else
> > +    struct termios tty;
> > +    if (tcgetattr(STDIN_FILENO, &tty) != 0) {
> > +        if (errno == ENOTTY) {
> > +            return 0x0; /* just expect read() == 0 */
> > +        } else {
> > +            return 0x4; /* Ctrl-D */
> > +        }
> > +    }
> > +
> > +    return tty.c_cc[VEOF];
> > +#endif
> > +}
> > +
> >  static int close_f(BlockBackend *blk, int argc, char **argv)
> >  {
> >      blk_unref(qemuio_blk);
> > @@ -322,7 +345,7 @@ static char *fetchline_readline(void)
> >      readline_start(readline_state, get_prompt(), 0, readline_func, &line);
> >      while (!line) {
> >          int ch = getchar();
> > -        if (ch == EOF) {
> > +        if (ttyEOF != 0x0 && ch == ttyEOF) {
> >              break;
> >          }
> >          readline_handle_byte(readline_state, ch);
> > @@ -592,6 +615,7 @@ int main(int argc, char **argv)
> >      qemuio_add_command(&close_cmd);
> >  
> >      if (isatty(STDIN_FILENO)) {
> > +        ttyEOF = get_eof_char();
> >          readline_state = readline_init(readline_printf_func,
> >                                         readline_flush_func,
> >                                         NULL,
> > -- 
> > 2.14.3
> > 
> 
> Regards,
> Daniel
> -- 
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [Qemu-devel] [PATCH v2] qemu-io: fix EOF Ctrl-D handling in qemu-io readline code
Posted by Kevin Wolf 7 years, 12 months ago 
Am 08.12.2017 um 14:34 hat Daniel P. Berrange geschrieben:
> qemu-io puts the TTY into non-canonical mode, which means no EOF processing is
> done and thus getchar() will never return the EOF constant. Instead we have to
> query the TTY attributes to determine the configured EOF character (usually
> Ctrl-D / 0x4), and then explicitly check for that value. This fixes the
> regression that prevented Ctrl-D from triggering an exit of qemu-io that has
> existed since readline was first added in
> 
>   commit 0cf17e181798063c3824c8200ba46f25f54faa1a
>   Author: Stefan Hajnoczi <stefanha@redhat.com>
>   Date:   Thu Nov 14 11:54:17 2013 +0100
> 
>     qemu-io: use readline.c
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

Thanks, applied to the block branch.

It would be nice if we also printed a newline when Ctrl-D is pressed. If
you want to send a v3 for this, I'll just update the patch in my queue.

Kevin

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.