* [Qemu-devel] [PATCH v2] qemu-io: fix EOF Ctrl-D handling in qemu-io readline code
@ 2017-12-08 13:34 Daniel P. Berrange
2017-12-08 16:21 ` Eric Blake
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Daniel P. Berrange @ 2017-12-08 13:34 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Kevin Wolf, Eric Blake, Max Reitz,
Dr. David Alan Gilbert, Daniel P. Berrange
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2] qemu-io: fix EOF Ctrl-D handling in qemu-io readline code
2017-12-08 13:34 [Qemu-devel] [PATCH v2] qemu-io: fix EOF Ctrl-D handling in qemu-io readline code Daniel P. Berrange
@ 2017-12-08 16:21 ` Eric Blake
2017-12-08 16:23 ` Daniel P. Berrange
2018-01-25 17:05 ` Daniel P. Berrangé
2018-02-12 18:17 ` Kevin Wolf
2 siblings, 1 reply; 6+ messages in thread
From: Eric Blake @ 2017-12-08 16:21 UTC (permalink / raw)
To: Daniel P. Berrange, qemu-devel
Cc: qemu-block, Kevin Wolf, Max Reitz, Dr. David Alan Gilbert
[-- Attachment #1: Type: text/plain, Size: 1685 bytes --]
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
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2] qemu-io: fix EOF Ctrl-D handling in qemu-io readline code
2017-12-08 16:21 ` Eric Blake
@ 2017-12-08 16:23 ` Daniel P. Berrange
0 siblings, 0 replies; 6+ messages in thread
From: Daniel P. Berrange @ 2017-12-08 16:23 UTC (permalink / raw)
To: Eric Blake
Cc: qemu-devel, qemu-block, Kevin Wolf, Max Reitz,
Dr. David Alan Gilbert
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 :|
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2] qemu-io: fix EOF Ctrl-D handling in qemu-io readline code
2017-12-08 13:34 [Qemu-devel] [PATCH v2] qemu-io: fix EOF Ctrl-D handling in qemu-io readline code Daniel P. Berrange
2017-12-08 16:21 ` Eric Blake
@ 2018-01-25 17:05 ` Daniel P. Berrangé
2018-02-12 17:49 ` Daniel P. Berrangé
2018-02-12 18:17 ` Kevin Wolf
2 siblings, 1 reply; 6+ messages in thread
From: Daniel P. Berrangé @ 2018-01-25 17:05 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Kevin Wolf, Eric Blake, Max Reitz,
Dr. David Alan Gilbert
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 :|
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2] qemu-io: fix EOF Ctrl-D handling in qemu-io readline code
2018-01-25 17:05 ` Daniel P. Berrangé
@ 2018-02-12 17:49 ` Daniel P. Berrangé
0 siblings, 0 replies; 6+ messages in thread
From: Daniel P. Berrangé @ 2018-02-12 17:49 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Dr. David Alan Gilbert, qemu-block, Max Reitz
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 :|
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2] qemu-io: fix EOF Ctrl-D handling in qemu-io readline code
2017-12-08 13:34 [Qemu-devel] [PATCH v2] qemu-io: fix EOF Ctrl-D handling in qemu-io readline code Daniel P. Berrange
2017-12-08 16:21 ` Eric Blake
2018-01-25 17:05 ` Daniel P. Berrangé
@ 2018-02-12 18:17 ` Kevin Wolf
2 siblings, 0 replies; 6+ messages in thread
From: Kevin Wolf @ 2018-02-12 18:17 UTC (permalink / raw)
To: Daniel P. Berrange
Cc: qemu-devel, qemu-block, Eric Blake, Max Reitz,
Dr. David Alan Gilbert
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-02-12 18:17 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-12-08 13:34 [Qemu-devel] [PATCH v2] qemu-io: fix EOF Ctrl-D handling in qemu-io readline code Daniel P. Berrange
2017-12-08 16:21 ` Eric Blake
2017-12-08 16:23 ` Daniel P. Berrange
2018-01-25 17:05 ` Daniel P. Berrangé
2018-02-12 17:49 ` Daniel P. Berrangé
2018-02-12 18:17 ` Kevin Wolf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).