* [Qemu-devel] [PATCH] Add 'maxqdepth' as an option to tty character devices.
@ 2013-05-06 13:43 John Baboval
2013-05-06 13:47 ` Paolo Bonzini
2013-05-06 14:43 ` Eric Blake
0 siblings, 2 replies; 10+ messages in thread
From: John Baboval @ 2013-05-06 13:43 UTC (permalink / raw)
To: qemu-devel; +Cc: John V. Baboval, John Baboval
From: "John V. Baboval" <john.baboval@virtualcomputer.com>
This parameter will cause writes to tty backed chardevs to return
-EAGAIN if the backing tty has buffered more than the specified
number of characters. When data is sent, the TIOCOUTQ ioctl is invoked
to determine the current TTY output buffer depth.
Background:
Some devices use DTR/DSR as flow control. (eg. Check/Receipt
printers with some POS software). When the device de-asserts
DTR, the guest OS notifies the application and new data is blocked.
When running on a QEMU serial port backed by a TTY, though the guest
stops transmitting, all the characters in the TTY output buffer are
still sent. The device buffer overflows and data is lost. In this
case the user could set maxqdepth=1.
Signed-off-by: John Baboval <john.baboval@citrix.com>
---
include/sysemu/char.h | 2 ++
qapi-schema.json | 5 ++++-
qemu-char.c | 40 +++++++++++++++++++++++++++++++++++++++-
qemu-options.hx | 4 ++--
4 files changed, 47 insertions(+), 4 deletions(-)
diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index 5e42c90..a94c1fb 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -43,6 +43,7 @@ typedef struct {
#define CHR_IOCTL_SERIAL_SET_TIOCM 13
#define CHR_IOCTL_SERIAL_GET_TIOCM 14
+#define CHR_IOCTL_SERIAL_TIOCOUTQ 15
#define CHR_TIOCM_CTS 0x020
#define CHR_TIOCM_CAR 0x040
@@ -77,6 +78,7 @@ struct CharDriverState {
int fe_open;
int explicit_fe_open;
int avail_connections;
+ uint32_t maxqdepth;
QemuOpts *opts;
QTAILQ_ENTRY(CharDriverState) next;
};
diff --git a/qapi-schema.json b/qapi-schema.json
index 7797400..029e7c9 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3182,11 +3182,14 @@
#
# @device: The name of the special file for the device,
# i.e. /dev/ttyS0 on Unix or COM1: on Windows
+# @maxqdepth: The maximum depth of the underlying tty
+ output queue (Unix)
# @type: What kind of device this is.
#
# Since: 1.4
##
-{ 'type': 'ChardevHostdev', 'data': { 'device' : 'str' } }
+{ 'type': 'ChardevHostdev', 'data': { 'device' : 'str',
+ 'maxqdepth' : 'int' } }
##
# @ChardevSocket:
diff --git a/qemu-char.c b/qemu-char.c
index 64e824d..e2e4217 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -782,6 +782,7 @@ typedef struct FDCharDriver {
GIOChannel *fd_in, *fd_out;
guint fd_in_tag;
int max_size;
+ int tiocoutq_failed;
QTAILQ_ENTRY(FDCharDriver) node;
} FDCharDriver;
@@ -1260,6 +1261,22 @@ static CharDriverState *qemu_chr_open_pty(const char *id,
return chr;
}
+static int tty_serial_write(CharDriverState *chr, const uint8_t *buf, int len)
+{
+ FDCharDriver *s = chr->opaque;
+ uint32_t inflight = 0;
+
+ qemu_chr_fe_ioctl(chr, CHR_IOCTL_SERIAL_TIOCOUTQ, &inflight);
+ if (inflight >= chr->maxqdepth)
+ return -EAGAIN;
+
+ if (inflight + len > chr->maxqdepth) {
+ len = chr->maxqdepth - inflight;
+ }
+
+ return io_channel_send(s->fd_out, buf, len);
+}
+
static void tty_serial_init(int fd, int speed,
int parity, int data_bits, int stop_bits)
{
@@ -1438,6 +1455,16 @@ static int tty_serial_ioctl(CharDriverState *chr, int cmd, void *arg)
ioctl(g_io_channel_unix_get_fd(s->fd_in), TIOCMSET, &targ);
}
break;
+ case CHR_IOCTL_SERIAL_TIOCOUTQ:
+ {
+ if (!s->tiocoutq_failed)
+ s->tiocoutq_failed = ioctl(g_io_channel_unix_get_fd(s->fd_in),
+ TIOCOUTQ, arg);
+
+ if (s->tiocoutq_failed)
+ *(unsigned int *)arg = 0;
+ }
+ break;
default:
return -ENOTSUP;
}
@@ -1466,6 +1493,7 @@ static CharDriverState *qemu_chr_open_tty_fd(int fd)
tty_serial_init(fd, 115200, 'N', 8, 1);
chr = qemu_chr_open_fd(fd, fd);
+ chr->chr_write = tty_serial_write;
chr->chr_ioctl = tty_serial_ioctl;
chr->chr_close = qemu_chr_close_tty;
return chr;
@@ -3172,6 +3200,8 @@ static void qemu_chr_parse_serial(QemuOpts *opts, ChardevBackend *backend,
}
backend->serial = g_new0(ChardevHostdev, 1);
backend->serial->device = g_strdup(device);
+ backend->serial->maxqdepth =
+ qemu_opt_get_number(opts, "maxqdepth", -1);
}
static void qemu_chr_parse_parallel(QemuOpts *opts, ChardevBackend *backend,
@@ -3575,6 +3605,9 @@ QemuOptsList qemu_chardev_opts = {
},{
.name = "size",
.type = QEMU_OPT_SIZE,
+ },{
+ .name = "maxqdepth",
+ .type = QEMU_OPT_NUMBER,
},
{ /* end of list */ }
},
@@ -3653,6 +3686,7 @@ static CharDriverState *qmp_chardev_open_serial(ChardevHostdev *serial,
Error **errp)
{
#ifdef HAVE_CHARDEV_TTY
+ CharDriverState *chr;
int fd;
fd = qmp_chardev_open_file_source(serial->device, O_RDWR, errp);
@@ -3660,7 +3694,11 @@ static CharDriverState *qmp_chardev_open_serial(ChardevHostdev *serial,
return NULL;
}
qemu_set_nonblock(fd);
- return qemu_chr_open_tty_fd(fd);
+ chr = qemu_chr_open_tty_fd(fd);
+ if (chr) {
+ chr->maxqdepth = serial->maxqdepth;
+ }
+ return chr;
#else
error_setg(errp, "character device backend type 'serial' not supported");
return NULL;
diff --git a/qemu-options.hx b/qemu-options.hx
index e86cc24..c522f13 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1792,8 +1792,8 @@ DEF("chardev", HAS_ARG, QEMU_OPTION_chardev,
#endif
#if defined(__linux__) || defined(__sun__) || defined(__FreeBSD__) \
|| defined(__NetBSD__) || defined(__OpenBSD__) || defined(__DragonFly__)
- "-chardev serial,id=id,path=path[,mux=on|off]\n"
- "-chardev tty,id=id,path=path[,mux=on|off]\n"
+ "-chardev serial,id=id,path=path[,mux=on|off][,maxqdepth=count]\n"
+ "-chardev tty,id=id,path=path[,mux=on|off][,maxqdepth=count]\n"
#endif
#if defined(__linux__) || defined(__FreeBSD__) || defined(__DragonFly__)
"-chardev parallel,id=id,path=path[,mux=on|off]\n"
--
1.7.9.5
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] Add 'maxqdepth' as an option to tty character devices.
2013-05-06 13:43 [Qemu-devel] [PATCH] Add 'maxqdepth' as an option to tty character devices John Baboval
@ 2013-05-06 13:47 ` Paolo Bonzini
2013-05-07 16:36 ` [Qemu-devel] (no subject) John Baboval
2013-05-06 14:43 ` Eric Blake
1 sibling, 1 reply; 10+ messages in thread
From: Paolo Bonzini @ 2013-05-06 13:47 UTC (permalink / raw)
To: John Baboval; +Cc: John V. Baboval, qemu-devel
Il 06/05/2013 15:43, John Baboval ha scritto:
> From: "John V. Baboval" <john.baboval@virtualcomputer.com>
>
> This parameter will cause writes to tty backed chardevs to return
> -EAGAIN if the backing tty has buffered more than the specified
> number of characters. When data is sent, the TIOCOUTQ ioctl is invoked
> to determine the current TTY output buffer depth.
>
> Background:
>
> Some devices use DTR/DSR as flow control. (eg. Check/Receipt
> printers with some POS software). When the device de-asserts
> DTR, the guest OS notifies the application and new data is blocked.
> When running on a QEMU serial port backed by a TTY, though the guest
> stops transmitting, all the characters in the TTY output buffer are
> still sent. The device buffer overflows and data is lost. In this
> case the user could set maxqdepth=1.
>
> Signed-off-by: John Baboval <john.baboval@citrix.com>
> ---
> include/sysemu/char.h | 2 ++
> qapi-schema.json | 5 ++++-
> qemu-char.c | 40 +++++++++++++++++++++++++++++++++++++++-
> qemu-options.hx | 4 ++--
> 4 files changed, 47 insertions(+), 4 deletions(-)
>
> diff --git a/include/sysemu/char.h b/include/sysemu/char.h
> index 5e42c90..a94c1fb 100644
> --- a/include/sysemu/char.h
> +++ b/include/sysemu/char.h
> @@ -43,6 +43,7 @@ typedef struct {
>
> #define CHR_IOCTL_SERIAL_SET_TIOCM 13
> #define CHR_IOCTL_SERIAL_GET_TIOCM 14
> +#define CHR_IOCTL_SERIAL_TIOCOUTQ 15
>
> #define CHR_TIOCM_CTS 0x020
> #define CHR_TIOCM_CAR 0x040
> @@ -77,6 +78,7 @@ struct CharDriverState {
> int fe_open;
> int explicit_fe_open;
> int avail_connections;
> + uint32_t maxqdepth;
> QemuOpts *opts;
> QTAILQ_ENTRY(CharDriverState) next;
> };
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 7797400..029e7c9 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3182,11 +3182,14 @@
> #
> # @device: The name of the special file for the device,
> # i.e. /dev/ttyS0 on Unix or COM1: on Windows
> +# @maxqdepth: The maximum depth of the underlying tty
> + output queue (Unix)
> # @type: What kind of device this is.
> #
> # Since: 1.4
> ##
> -{ 'type': 'ChardevHostdev', 'data': { 'device' : 'str' } }
> +{ 'type': 'ChardevHostdev', 'data': { 'device' : 'str',
> + 'maxqdepth' : 'int' } }
This needs to be optional for backwards compatibility. You can check
serial->has_maxqdepth and use a default value of -1 if it is true...
>
> ##
> # @ChardevSocket:
> diff --git a/qemu-char.c b/qemu-char.c
> index 64e824d..e2e4217 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -782,6 +782,7 @@ typedef struct FDCharDriver {
> GIOChannel *fd_in, *fd_out;
> guint fd_in_tag;
> int max_size;
> + int tiocoutq_failed;
> QTAILQ_ENTRY(FDCharDriver) node;
> } FDCharDriver;
>
> @@ -1260,6 +1261,22 @@ static CharDriverState *qemu_chr_open_pty(const char *id,
> return chr;
> }
>
> +static int tty_serial_write(CharDriverState *chr, const uint8_t *buf, int len)
> +{
> + FDCharDriver *s = chr->opaque;
> + uint32_t inflight = 0;
> +
> + qemu_chr_fe_ioctl(chr, CHR_IOCTL_SERIAL_TIOCOUTQ, &inflight);
> + if (inflight >= chr->maxqdepth)
> + return -EAGAIN;
> +
> + if (inflight + len > chr->maxqdepth) {
> + len = chr->maxqdepth - inflight;
> + }
> +
> + return io_channel_send(s->fd_out, buf, len);
> +}
> +
> static void tty_serial_init(int fd, int speed,
> int parity, int data_bits, int stop_bits)
> {
> @@ -1438,6 +1455,16 @@ static int tty_serial_ioctl(CharDriverState *chr, int cmd, void *arg)
> ioctl(g_io_channel_unix_get_fd(s->fd_in), TIOCMSET, &targ);
> }
> break;
> + case CHR_IOCTL_SERIAL_TIOCOUTQ:
> + {
> + if (!s->tiocoutq_failed)
> + s->tiocoutq_failed = ioctl(g_io_channel_unix_get_fd(s->fd_in),
> + TIOCOUTQ, arg);
> +
> + if (s->tiocoutq_failed)
> + *(unsigned int *)arg = 0;
> + }
> + break;
> default:
> return -ENOTSUP;
> }
> @@ -1466,6 +1493,7 @@ static CharDriverState *qemu_chr_open_tty_fd(int fd)
>
> tty_serial_init(fd, 115200, 'N', 8, 1);
> chr = qemu_chr_open_fd(fd, fd);
> + chr->chr_write = tty_serial_write;
> chr->chr_ioctl = tty_serial_ioctl;
> chr->chr_close = qemu_chr_close_tty;
> return chr;
> @@ -3172,6 +3200,8 @@ static void qemu_chr_parse_serial(QemuOpts *opts, ChardevBackend *backend,
> }
> backend->serial = g_new0(ChardevHostdev, 1);
> backend->serial->device = g_strdup(device);
> + backend->serial->maxqdepth =
> + qemu_opt_get_number(opts, "maxqdepth", -1);
... and also set has_maxqdepth here.
Thanks,
Paolo
> }
>
> static void qemu_chr_parse_parallel(QemuOpts *opts, ChardevBackend *backend,
> @@ -3575,6 +3605,9 @@ QemuOptsList qemu_chardev_opts = {
> },{
> .name = "size",
> .type = QEMU_OPT_SIZE,
> + },{
> + .name = "maxqdepth",
> + .type = QEMU_OPT_NUMBER,
> },
> { /* end of list */ }
> },
> @@ -3653,6 +3686,7 @@ static CharDriverState *qmp_chardev_open_serial(ChardevHostdev *serial,
> Error **errp)
> {
> #ifdef HAVE_CHARDEV_TTY
> + CharDriverState *chr;
> int fd;
>
> fd = qmp_chardev_open_file_source(serial->device, O_RDWR, errp);
> @@ -3660,7 +3694,11 @@ static CharDriverState *qmp_chardev_open_serial(ChardevHostdev *serial,
> return NULL;
> }
> qemu_set_nonblock(fd);
> - return qemu_chr_open_tty_fd(fd);
> + chr = qemu_chr_open_tty_fd(fd);
> + if (chr) {
> + chr->maxqdepth = serial->maxqdepth;
> + }
> + return chr;
> #else
> error_setg(errp, "character device backend type 'serial' not supported");
> return NULL;
> diff --git a/qemu-options.hx b/qemu-options.hx
> index e86cc24..c522f13 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1792,8 +1792,8 @@ DEF("chardev", HAS_ARG, QEMU_OPTION_chardev,
> #endif
> #if defined(__linux__) || defined(__sun__) || defined(__FreeBSD__) \
> || defined(__NetBSD__) || defined(__OpenBSD__) || defined(__DragonFly__)
> - "-chardev serial,id=id,path=path[,mux=on|off]\n"
> - "-chardev tty,id=id,path=path[,mux=on|off]\n"
> + "-chardev serial,id=id,path=path[,mux=on|off][,maxqdepth=count]\n"
> + "-chardev tty,id=id,path=path[,mux=on|off][,maxqdepth=count]\n"
> #endif
> #if defined(__linux__) || defined(__FreeBSD__) || defined(__DragonFly__)
> "-chardev parallel,id=id,path=path[,mux=on|off]\n"
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] (no subject)
2013-05-06 13:47 ` Paolo Bonzini
@ 2013-05-07 16:36 ` John Baboval
2013-05-07 16:36 ` [Qemu-devel] [PATCH] Add 'maxqdepth' as an option to tty character devices John Baboval
0 siblings, 1 reply; 10+ messages in thread
From: John Baboval @ 2013-05-07 16:36 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini
Thanks for the feedback. I've made the maxqdepth parameter optional as requested.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH] Add 'maxqdepth' as an option to tty character devices.
2013-05-07 16:36 ` [Qemu-devel] (no subject) John Baboval
@ 2013-05-07 16:36 ` John Baboval
2013-05-07 16:40 ` John Baboval
0 siblings, 1 reply; 10+ messages in thread
From: John Baboval @ 2013-05-07 16:36 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, John Baboval, John V. Baboval
From: "John V. Baboval" <john.baboval@virtualcomputer.com>
This parameter will cause writes to tty backed chardevs to return
-EAGAIN if the backing tty has buffered more than the specified
number of characters. When data is sent, the TIOCOUTQ ioctl is invoked
to determine the current TTY output buffer depth.
Background:
Some devices use DTR/DSR as flow control. (eg. Check/Receipt
printers with some POS software). When the device de-asserts
DTR, the guest OS notifies the application and new data is blocked.
When running on a QEMU serial port backed by a TTY, though the guest
stops transmitting, all the characters in the TTY output buffer are
still sent. The device buffer overflows and data is lost. In this
case the user could set maxqdepth=1.
Signed-off-by: John Baboval <john.baboval@citrix.com>
---
include/sysemu/char.h | 2 ++
qapi-schema.json | 5 ++++-
qemu-char.c | 40 +++++++++++++++++++++++++++++++++++++++-
qemu-options.hx | 4 ++--
4 files changed, 47 insertions(+), 4 deletions(-)
diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index 5e42c90..a94c1fb 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -43,6 +43,7 @@ typedef struct {
#define CHR_IOCTL_SERIAL_SET_TIOCM 13
#define CHR_IOCTL_SERIAL_GET_TIOCM 14
+#define CHR_IOCTL_SERIAL_TIOCOUTQ 15
#define CHR_TIOCM_CTS 0x020
#define CHR_TIOCM_CAR 0x040
@@ -77,6 +78,7 @@ struct CharDriverState {
int fe_open;
int explicit_fe_open;
int avail_connections;
+ uint32_t maxqdepth;
QemuOpts *opts;
QTAILQ_ENTRY(CharDriverState) next;
};
diff --git a/qapi-schema.json b/qapi-schema.json
index 7797400..029e7c9 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3182,11 +3182,14 @@
#
# @device: The name of the special file for the device,
# i.e. /dev/ttyS0 on Unix or COM1: on Windows
+# @maxqdepth: The maximum depth of the underlying tty
+ output queue (Unix)
# @type: What kind of device this is.
#
# Since: 1.4
##
-{ 'type': 'ChardevHostdev', 'data': { 'device' : 'str' } }
+{ 'type': 'ChardevHostdev', 'data': { 'device' : 'str',
+ 'maxqdepth' : 'int' } }
##
# @ChardevSocket:
diff --git a/qemu-char.c b/qemu-char.c
index 64e824d..e2e4217 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -782,6 +782,7 @@ typedef struct FDCharDriver {
GIOChannel *fd_in, *fd_out;
guint fd_in_tag;
int max_size;
+ int tiocoutq_failed;
QTAILQ_ENTRY(FDCharDriver) node;
} FDCharDriver;
@@ -1260,6 +1261,22 @@ static CharDriverState *qemu_chr_open_pty(const char *id,
return chr;
}
+static int tty_serial_write(CharDriverState *chr, const uint8_t *buf, int len)
+{
+ FDCharDriver *s = chr->opaque;
+ uint32_t inflight = 0;
+
+ qemu_chr_fe_ioctl(chr, CHR_IOCTL_SERIAL_TIOCOUTQ, &inflight);
+ if (inflight >= chr->maxqdepth)
+ return -EAGAIN;
+
+ if (inflight + len > chr->maxqdepth) {
+ len = chr->maxqdepth - inflight;
+ }
+
+ return io_channel_send(s->fd_out, buf, len);
+}
+
static void tty_serial_init(int fd, int speed,
int parity, int data_bits, int stop_bits)
{
@@ -1438,6 +1455,16 @@ static int tty_serial_ioctl(CharDriverState *chr, int cmd, void *arg)
ioctl(g_io_channel_unix_get_fd(s->fd_in), TIOCMSET, &targ);
}
break;
+ case CHR_IOCTL_SERIAL_TIOCOUTQ:
+ {
+ if (!s->tiocoutq_failed)
+ s->tiocoutq_failed = ioctl(g_io_channel_unix_get_fd(s->fd_in),
+ TIOCOUTQ, arg);
+
+ if (s->tiocoutq_failed)
+ *(unsigned int *)arg = 0;
+ }
+ break;
default:
return -ENOTSUP;
}
@@ -1466,6 +1493,7 @@ static CharDriverState *qemu_chr_open_tty_fd(int fd)
tty_serial_init(fd, 115200, 'N', 8, 1);
chr = qemu_chr_open_fd(fd, fd);
+ chr->chr_write = tty_serial_write;
chr->chr_ioctl = tty_serial_ioctl;
chr->chr_close = qemu_chr_close_tty;
return chr;
@@ -3172,6 +3200,8 @@ static void qemu_chr_parse_serial(QemuOpts *opts, ChardevBackend *backend,
}
backend->serial = g_new0(ChardevHostdev, 1);
backend->serial->device = g_strdup(device);
+ backend->serial->maxqdepth =
+ qemu_opt_get_number(opts, "maxqdepth", -1);
}
static void qemu_chr_parse_parallel(QemuOpts *opts, ChardevBackend *backend,
@@ -3575,6 +3605,9 @@ QemuOptsList qemu_chardev_opts = {
},{
.name = "size",
.type = QEMU_OPT_SIZE,
+ },{
+ .name = "maxqdepth",
+ .type = QEMU_OPT_NUMBER,
},
{ /* end of list */ }
},
@@ -3653,6 +3686,7 @@ static CharDriverState *qmp_chardev_open_serial(ChardevHostdev *serial,
Error **errp)
{
#ifdef HAVE_CHARDEV_TTY
+ CharDriverState *chr;
int fd;
fd = qmp_chardev_open_file_source(serial->device, O_RDWR, errp);
@@ -3660,7 +3694,11 @@ static CharDriverState *qmp_chardev_open_serial(ChardevHostdev *serial,
return NULL;
}
qemu_set_nonblock(fd);
- return qemu_chr_open_tty_fd(fd);
+ chr = qemu_chr_open_tty_fd(fd);
+ if (chr) {
+ chr->maxqdepth = serial->maxqdepth;
+ }
+ return chr;
#else
error_setg(errp, "character device backend type 'serial' not supported");
return NULL;
diff --git a/qemu-options.hx b/qemu-options.hx
index e86cc24..c522f13 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1792,8 +1792,8 @@ DEF("chardev", HAS_ARG, QEMU_OPTION_chardev,
#endif
#if defined(__linux__) || defined(__sun__) || defined(__FreeBSD__) \
|| defined(__NetBSD__) || defined(__OpenBSD__) || defined(__DragonFly__)
- "-chardev serial,id=id,path=path[,mux=on|off]\n"
- "-chardev tty,id=id,path=path[,mux=on|off]\n"
+ "-chardev serial,id=id,path=path[,mux=on|off][,maxqdepth=count]\n"
+ "-chardev tty,id=id,path=path[,mux=on|off][,maxqdepth=count]\n"
#endif
#if defined(__linux__) || defined(__FreeBSD__) || defined(__DragonFly__)
"-chardev parallel,id=id,path=path[,mux=on|off]\n"
--
1.7.9.5
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] Add 'maxqdepth' as an option to tty character devices.
2013-05-07 16:36 ` [Qemu-devel] [PATCH] Add 'maxqdepth' as an option to tty character devices John Baboval
@ 2013-05-07 16:40 ` John Baboval
0 siblings, 0 replies; 10+ messages in thread
From: John Baboval @ 2013-05-07 16:40 UTC (permalink / raw)
To: John Baboval; +Cc: pbonzini, qemu-devel, John V. Baboval
Hmm. I seem to have screwed up the "In-Reply-To" on this one. Sorry
about that.
On 05/07/2013 12:36 PM, John Baboval wrote:
> From: "John V. Baboval" <john.baboval@virtualcomputer.com>
>
> This parameter will cause writes to tty backed chardevs to return
> -EAGAIN if the backing tty has buffered more than the specified
> number of characters. When data is sent, the TIOCOUTQ ioctl is invoked
> to determine the current TTY output buffer depth.
>
> Background:
>
> Some devices use DTR/DSR as flow control. (eg. Check/Receipt
> printers with some POS software). When the device de-asserts
> DTR, the guest OS notifies the application and new data is blocked.
> When running on a QEMU serial port backed by a TTY, though the guest
> stops transmitting, all the characters in the TTY output buffer are
> still sent. The device buffer overflows and data is lost. In this
> case the user could set maxqdepth=1.
>
> Signed-off-by: John Baboval <john.baboval@citrix.com>
> ---
> include/sysemu/char.h | 2 ++
> qapi-schema.json | 5 ++++-
> qemu-char.c | 40 +++++++++++++++++++++++++++++++++++++++-
> qemu-options.hx | 4 ++--
> 4 files changed, 47 insertions(+), 4 deletions(-)
>
> diff --git a/include/sysemu/char.h b/include/sysemu/char.h
> index 5e42c90..a94c1fb 100644
> --- a/include/sysemu/char.h
> +++ b/include/sysemu/char.h
> @@ -43,6 +43,7 @@ typedef struct {
>
> #define CHR_IOCTL_SERIAL_SET_TIOCM 13
> #define CHR_IOCTL_SERIAL_GET_TIOCM 14
> +#define CHR_IOCTL_SERIAL_TIOCOUTQ 15
>
> #define CHR_TIOCM_CTS 0x020
> #define CHR_TIOCM_CAR 0x040
> @@ -77,6 +78,7 @@ struct CharDriverState {
> int fe_open;
> int explicit_fe_open;
> int avail_connections;
> + uint32_t maxqdepth;
> QemuOpts *opts;
> QTAILQ_ENTRY(CharDriverState) next;
> };
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 7797400..029e7c9 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3182,11 +3182,14 @@
> #
> # @device: The name of the special file for the device,
> # i.e. /dev/ttyS0 on Unix or COM1: on Windows
> +# @maxqdepth: The maximum depth of the underlying tty
> + output queue (Unix)
> # @type: What kind of device this is.
> #
> # Since: 1.4
> ##
> -{ 'type': 'ChardevHostdev', 'data': { 'device' : 'str' } }
> +{ 'type': 'ChardevHostdev', 'data': { 'device' : 'str',
> + 'maxqdepth' : 'int' } }
>
> ##
> # @ChardevSocket:
> diff --git a/qemu-char.c b/qemu-char.c
> index 64e824d..e2e4217 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -782,6 +782,7 @@ typedef struct FDCharDriver {
> GIOChannel *fd_in, *fd_out;
> guint fd_in_tag;
> int max_size;
> + int tiocoutq_failed;
> QTAILQ_ENTRY(FDCharDriver) node;
> } FDCharDriver;
>
> @@ -1260,6 +1261,22 @@ static CharDriverState *qemu_chr_open_pty(const char *id,
> return chr;
> }
>
> +static int tty_serial_write(CharDriverState *chr, const uint8_t *buf, int len)
> +{
> + FDCharDriver *s = chr->opaque;
> + uint32_t inflight = 0;
> +
> + qemu_chr_fe_ioctl(chr, CHR_IOCTL_SERIAL_TIOCOUTQ, &inflight);
> + if (inflight >= chr->maxqdepth)
> + return -EAGAIN;
> +
> + if (inflight + len > chr->maxqdepth) {
> + len = chr->maxqdepth - inflight;
> + }
> +
> + return io_channel_send(s->fd_out, buf, len);
> +}
> +
> static void tty_serial_init(int fd, int speed,
> int parity, int data_bits, int stop_bits)
> {
> @@ -1438,6 +1455,16 @@ static int tty_serial_ioctl(CharDriverState *chr, int cmd, void *arg)
> ioctl(g_io_channel_unix_get_fd(s->fd_in), TIOCMSET, &targ);
> }
> break;
> + case CHR_IOCTL_SERIAL_TIOCOUTQ:
> + {
> + if (!s->tiocoutq_failed)
> + s->tiocoutq_failed = ioctl(g_io_channel_unix_get_fd(s->fd_in),
> + TIOCOUTQ, arg);
> +
> + if (s->tiocoutq_failed)
> + *(unsigned int *)arg = 0;
> + }
> + break;
> default:
> return -ENOTSUP;
> }
> @@ -1466,6 +1493,7 @@ static CharDriverState *qemu_chr_open_tty_fd(int fd)
>
> tty_serial_init(fd, 115200, 'N', 8, 1);
> chr = qemu_chr_open_fd(fd, fd);
> + chr->chr_write = tty_serial_write;
> chr->chr_ioctl = tty_serial_ioctl;
> chr->chr_close = qemu_chr_close_tty;
> return chr;
> @@ -3172,6 +3200,8 @@ static void qemu_chr_parse_serial(QemuOpts *opts, ChardevBackend *backend,
> }
> backend->serial = g_new0(ChardevHostdev, 1);
> backend->serial->device = g_strdup(device);
> + backend->serial->maxqdepth =
> + qemu_opt_get_number(opts, "maxqdepth", -1);
> }
>
> static void qemu_chr_parse_parallel(QemuOpts *opts, ChardevBackend *backend,
> @@ -3575,6 +3605,9 @@ QemuOptsList qemu_chardev_opts = {
> },{
> .name = "size",
> .type = QEMU_OPT_SIZE,
> + },{
> + .name = "maxqdepth",
> + .type = QEMU_OPT_NUMBER,
> },
> { /* end of list */ }
> },
> @@ -3653,6 +3686,7 @@ static CharDriverState *qmp_chardev_open_serial(ChardevHostdev *serial,
> Error **errp)
> {
> #ifdef HAVE_CHARDEV_TTY
> + CharDriverState *chr;
> int fd;
>
> fd = qmp_chardev_open_file_source(serial->device, O_RDWR, errp);
> @@ -3660,7 +3694,11 @@ static CharDriverState *qmp_chardev_open_serial(ChardevHostdev *serial,
> return NULL;
> }
> qemu_set_nonblock(fd);
> - return qemu_chr_open_tty_fd(fd);
> + chr = qemu_chr_open_tty_fd(fd);
> + if (chr) {
> + chr->maxqdepth = serial->maxqdepth;
> + }
> + return chr;
> #else
> error_setg(errp, "character device backend type 'serial' not supported");
> return NULL;
> diff --git a/qemu-options.hx b/qemu-options.hx
> index e86cc24..c522f13 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1792,8 +1792,8 @@ DEF("chardev", HAS_ARG, QEMU_OPTION_chardev,
> #endif
> #if defined(__linux__) || defined(__sun__) || defined(__FreeBSD__) \
> || defined(__NetBSD__) || defined(__OpenBSD__) || defined(__DragonFly__)
> - "-chardev serial,id=id,path=path[,mux=on|off]\n"
> - "-chardev tty,id=id,path=path[,mux=on|off]\n"
> + "-chardev serial,id=id,path=path[,mux=on|off][,maxqdepth=count]\n"
> + "-chardev tty,id=id,path=path[,mux=on|off][,maxqdepth=count]\n"
> #endif
> #if defined(__linux__) || defined(__FreeBSD__) || defined(__DragonFly__)
> "-chardev parallel,id=id,path=path[,mux=on|off]\n"
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] Add 'maxqdepth' as an option to tty character devices.
2013-05-06 13:43 [Qemu-devel] [PATCH] Add 'maxqdepth' as an option to tty character devices John Baboval
2013-05-06 13:47 ` Paolo Bonzini
@ 2013-05-06 14:43 ` Eric Blake
2013-05-07 16:51 ` [Qemu-devel] [PATCH v3] " John Baboval
1 sibling, 1 reply; 10+ messages in thread
From: Eric Blake @ 2013-05-06 14:43 UTC (permalink / raw)
To: John Baboval; +Cc: John V. Baboval, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1684 bytes --]
On 05/06/2013 07:43 AM, John Baboval wrote:
> From: "John V. Baboval" <john.baboval@virtualcomputer.com>
>
> This parameter will cause writes to tty backed chardevs to return
> -EAGAIN if the backing tty has buffered more than the specified
> number of characters. When data is sent, the TIOCOUTQ ioctl is invoked
> to determine the current TTY output buffer depth.
>
Reviewing just the interface portion of the patch:
> +++ b/qapi-schema.json
> @@ -3182,11 +3182,14 @@
> #
> # @device: The name of the special file for the device,
> # i.e. /dev/ttyS0 on Unix or COM1: on Windows
> +# @maxqdepth: The maximum depth of the underlying tty
> + output queue (Unix)
Trailing whitespace. Run your patch through scripts/checkpatch.pl.
Since you are adding a new member, you should use a "(since 1.6)"
comment on this line. Also, most interfaces tend to use a blank line
between member documentation.
> # @type: What kind of device this is.
Hmm - we have a pre-existing documentation bug - this line probably
should have been deleted during commit d36b2b90.
> #
> # Since: 1.4
> ##
> -{ 'type': 'ChardevHostdev', 'data': { 'device' : 'str' } }
> +{ 'type': 'ChardevHostdev', 'data': { 'device' : 'str',
> + 'maxqdepth' : 'int' } }
Ouch - this says that maxqdepth is mandatory. But that is a
backwards-incompatible change with apps that target the 'chardev-add'
QMP command of qemu 1.4. You MUST make it optional, since older apps
will not be providing it.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH v3] Add 'maxqdepth' as an option to tty character devices.
2013-05-06 14:43 ` Eric Blake
@ 2013-05-07 16:51 ` John Baboval
2013-05-07 16:51 ` [Qemu-devel] [PATCH] " John Baboval
0 siblings, 1 reply; 10+ messages in thread
From: John Baboval @ 2013-05-07 16:51 UTC (permalink / raw)
To: qemu-devel
Includes changes requested by Eric Blake and Paolo Bonzini
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH] Add 'maxqdepth' as an option to tty character devices.
2013-05-07 16:51 ` [Qemu-devel] [PATCH v3] " John Baboval
@ 2013-05-07 16:51 ` John Baboval
2013-05-07 17:17 ` Eric Blake
0 siblings, 1 reply; 10+ messages in thread
From: John Baboval @ 2013-05-07 16:51 UTC (permalink / raw)
To: qemu-devel; +Cc: John V. Baboval, John Baboval
From: "John V. Baboval" <john.baboval@virtualcomputer.com>
This parameter will cause writes to tty backed chardevs to return
-EAGAIN if the backing tty has buffered more than the specified
number of characters. When data is sent, the TIOCOUTQ ioctl is invoked
to determine the current TTY output buffer depth.
Background:
Some devices use DTR/DSR as flow control. (eg. Check/Receipt
printers with some POS software). When the device de-asserts
DTR, the guest OS notifies the application and new data is blocked.
When running on a QEMU serial port backed by a TTY, though the guest
stops transmitting, all the characters in the TTY output buffer are
still sent. The device buffer overflows and data is lost. In this
case the user could set maxqdepth=1.
Signed-off-by: John Baboval <john.baboval@citrix.com>
---
include/sysemu/char.h | 2 ++
qapi-schema.json | 5 ++++-
qemu-char.c | 40 +++++++++++++++++++++++++++++++++++++++-
qemu-options.hx | 4 ++--
4 files changed, 47 insertions(+), 4 deletions(-)
diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index 5e42c90..a94c1fb 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -43,6 +43,7 @@ typedef struct {
#define CHR_IOCTL_SERIAL_SET_TIOCM 13
#define CHR_IOCTL_SERIAL_GET_TIOCM 14
+#define CHR_IOCTL_SERIAL_TIOCOUTQ 15
#define CHR_TIOCM_CTS 0x020
#define CHR_TIOCM_CAR 0x040
@@ -77,6 +78,7 @@ struct CharDriverState {
int fe_open;
int explicit_fe_open;
int avail_connections;
+ uint32_t maxqdepth;
QemuOpts *opts;
QTAILQ_ENTRY(CharDriverState) next;
};
diff --git a/qapi-schema.json b/qapi-schema.json
index 7797400..029e7c9 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3182,11 +3182,14 @@
#
# @device: The name of the special file for the device,
# i.e. /dev/ttyS0 on Unix or COM1: on Windows
+# @maxqdepth: The maximum depth of the underlying tty
+ output queue (Unix)
# @type: What kind of device this is.
#
# Since: 1.4
##
-{ 'type': 'ChardevHostdev', 'data': { 'device' : 'str' } }
+{ 'type': 'ChardevHostdev', 'data': { 'device' : 'str',
+ 'maxqdepth' : 'int' } }
##
# @ChardevSocket:
diff --git a/qemu-char.c b/qemu-char.c
index 64e824d..e2e4217 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -782,6 +782,7 @@ typedef struct FDCharDriver {
GIOChannel *fd_in, *fd_out;
guint fd_in_tag;
int max_size;
+ int tiocoutq_failed;
QTAILQ_ENTRY(FDCharDriver) node;
} FDCharDriver;
@@ -1260,6 +1261,22 @@ static CharDriverState *qemu_chr_open_pty(const char *id,
return chr;
}
+static int tty_serial_write(CharDriverState *chr, const uint8_t *buf, int len)
+{
+ FDCharDriver *s = chr->opaque;
+ uint32_t inflight = 0;
+
+ qemu_chr_fe_ioctl(chr, CHR_IOCTL_SERIAL_TIOCOUTQ, &inflight);
+ if (inflight >= chr->maxqdepth)
+ return -EAGAIN;
+
+ if (inflight + len > chr->maxqdepth) {
+ len = chr->maxqdepth - inflight;
+ }
+
+ return io_channel_send(s->fd_out, buf, len);
+}
+
static void tty_serial_init(int fd, int speed,
int parity, int data_bits, int stop_bits)
{
@@ -1438,6 +1455,16 @@ static int tty_serial_ioctl(CharDriverState *chr, int cmd, void *arg)
ioctl(g_io_channel_unix_get_fd(s->fd_in), TIOCMSET, &targ);
}
break;
+ case CHR_IOCTL_SERIAL_TIOCOUTQ:
+ {
+ if (!s->tiocoutq_failed)
+ s->tiocoutq_failed = ioctl(g_io_channel_unix_get_fd(s->fd_in),
+ TIOCOUTQ, arg);
+
+ if (s->tiocoutq_failed)
+ *(unsigned int *)arg = 0;
+ }
+ break;
default:
return -ENOTSUP;
}
@@ -1466,6 +1493,7 @@ static CharDriverState *qemu_chr_open_tty_fd(int fd)
tty_serial_init(fd, 115200, 'N', 8, 1);
chr = qemu_chr_open_fd(fd, fd);
+ chr->chr_write = tty_serial_write;
chr->chr_ioctl = tty_serial_ioctl;
chr->chr_close = qemu_chr_close_tty;
return chr;
@@ -3172,6 +3200,8 @@ static void qemu_chr_parse_serial(QemuOpts *opts, ChardevBackend *backend,
}
backend->serial = g_new0(ChardevHostdev, 1);
backend->serial->device = g_strdup(device);
+ backend->serial->maxqdepth =
+ qemu_opt_get_number(opts, "maxqdepth", -1);
}
static void qemu_chr_parse_parallel(QemuOpts *opts, ChardevBackend *backend,
@@ -3575,6 +3605,9 @@ QemuOptsList qemu_chardev_opts = {
},{
.name = "size",
.type = QEMU_OPT_SIZE,
+ },{
+ .name = "maxqdepth",
+ .type = QEMU_OPT_NUMBER,
},
{ /* end of list */ }
},
@@ -3653,6 +3686,7 @@ static CharDriverState *qmp_chardev_open_serial(ChardevHostdev *serial,
Error **errp)
{
#ifdef HAVE_CHARDEV_TTY
+ CharDriverState *chr;
int fd;
fd = qmp_chardev_open_file_source(serial->device, O_RDWR, errp);
@@ -3660,7 +3694,11 @@ static CharDriverState *qmp_chardev_open_serial(ChardevHostdev *serial,
return NULL;
}
qemu_set_nonblock(fd);
- return qemu_chr_open_tty_fd(fd);
+ chr = qemu_chr_open_tty_fd(fd);
+ if (chr) {
+ chr->maxqdepth = serial->maxqdepth;
+ }
+ return chr;
#else
error_setg(errp, "character device backend type 'serial' not supported");
return NULL;
diff --git a/qemu-options.hx b/qemu-options.hx
index e86cc24..c522f13 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1792,8 +1792,8 @@ DEF("chardev", HAS_ARG, QEMU_OPTION_chardev,
#endif
#if defined(__linux__) || defined(__sun__) || defined(__FreeBSD__) \
|| defined(__NetBSD__) || defined(__OpenBSD__) || defined(__DragonFly__)
- "-chardev serial,id=id,path=path[,mux=on|off]\n"
- "-chardev tty,id=id,path=path[,mux=on|off]\n"
+ "-chardev serial,id=id,path=path[,mux=on|off][,maxqdepth=count]\n"
+ "-chardev tty,id=id,path=path[,mux=on|off][,maxqdepth=count]\n"
#endif
#if defined(__linux__) || defined(__FreeBSD__) || defined(__DragonFly__)
"-chardev parallel,id=id,path=path[,mux=on|off]\n"
--
1.7.9.5
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] Add 'maxqdepth' as an option to tty character devices.
2013-05-07 16:51 ` [Qemu-devel] [PATCH] " John Baboval
@ 2013-05-07 17:17 ` Eric Blake
2013-05-07 22:24 ` John Baboval
0 siblings, 1 reply; 10+ messages in thread
From: Eric Blake @ 2013-05-07 17:17 UTC (permalink / raw)
To: John Baboval; +Cc: John V. Baboval, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1438 bytes --]
On 05/07/2013 10:51 AM, John Baboval wrote:
> From: "John V. Baboval" <john.baboval@virtualcomputer.com>
>
> This parameter will cause writes to tty backed chardevs to return
> -EAGAIN if the backing tty has buffered more than the specified
> number of characters. When data is sent, the TIOCOUTQ ioctl is invoked
> to determine the current TTY output buffer depth.
When submitting a v2 patch, please adjust the subject line to call out
that it is a v2, and send it as a top-level thread rather than
in-reply-to an earlier thread. For more details, see:
http://wiki.qemu.org/Contribute/SubmitAPatch
> +++ b/qapi-schema.json
> @@ -3182,11 +3182,14 @@
> #
> # @device: The name of the special file for the device,
> # i.e. /dev/ttyS0 on Unix or COM1: on Windows
> +# @maxqdepth: The maximum depth of the underlying tty
> + output queue (Unix)
> # @type: What kind of device this is.
> #
> # Since: 1.4
> ##
> -{ 'type': 'ChardevHostdev', 'data': { 'device' : 'str' } }
> +{ 'type': 'ChardevHostdev', 'data': { 'device' : 'str',
> + 'maxqdepth' : 'int' } }
You did not address any of my review concerns from v1, such as listing a
(since 1.6) label or marking maxqdepth as optional. Are you sure you
sent the right patch?
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] Add 'maxqdepth' as an option to tty character devices.
2013-05-07 17:17 ` Eric Blake
@ 2013-05-07 22:24 ` John Baboval
0 siblings, 0 replies; 10+ messages in thread
From: John Baboval @ 2013-05-07 22:24 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel
Sorry about fumbling with the tools. The wiki link is very helpful. I'll give it another go in a little while.
-----Original Message-----
From: Eric Blake [mailto:eblake@redhat.com]
Sent: Tuesday, May 07, 2013 1:17 PM
To: John Baboval
Cc: qemu-devel@nongnu.org; John Baboval
Subject: Re: [Qemu-devel] [PATCH] Add 'maxqdepth' as an option to tty character devices.
On 05/07/2013 10:51 AM, John Baboval wrote:
> From: "John V. Baboval" <john.baboval@virtualcomputer.com>
>
> This parameter will cause writes to tty backed chardevs to return
> -EAGAIN if the backing tty has buffered more than the specified number
> of characters. When data is sent, the TIOCOUTQ ioctl is invoked to
> determine the current TTY output buffer depth.
When submitting a v2 patch, please adjust the subject line to call out that it is a v2, and send it as a top-level thread rather than in-reply-to an earlier thread. For more details, see:
http://wiki.qemu.org/Contribute/SubmitAPatch
> +++ b/qapi-schema.json
> @@ -3182,11 +3182,14 @@
> #
> # @device: The name of the special file for the device,
> # i.e. /dev/ttyS0 on Unix or COM1: on Windows
> +# @maxqdepth: The maximum depth of the underlying tty
> + output queue (Unix)
> # @type: What kind of device this is.
> #
> # Since: 1.4
> ##
> -{ 'type': 'ChardevHostdev', 'data': { 'device' : 'str' } }
> +{ 'type': 'ChardevHostdev', 'data': { 'device' : 'str',
> + 'maxqdepth' : 'int' } }
You did not address any of my review concerns from v1, such as listing a (since 1.6) label or marking maxqdepth as optional. Are you sure you sent the right patch?
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2013-05-07 22:24 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-06 13:43 [Qemu-devel] [PATCH] Add 'maxqdepth' as an option to tty character devices John Baboval
2013-05-06 13:47 ` Paolo Bonzini
2013-05-07 16:36 ` [Qemu-devel] (no subject) John Baboval
2013-05-07 16:36 ` [Qemu-devel] [PATCH] Add 'maxqdepth' as an option to tty character devices John Baboval
2013-05-07 16:40 ` John Baboval
2013-05-06 14:43 ` Eric Blake
2013-05-07 16:51 ` [Qemu-devel] [PATCH v3] " John Baboval
2013-05-07 16:51 ` [Qemu-devel] [PATCH] " John Baboval
2013-05-07 17:17 ` Eric Blake
2013-05-07 22:24 ` John Baboval
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.