Re: [Qemu-devel] [PATCH] Add 'maxqdepth' as an option to tty character devices.

[John Baboval <john.baboval@citrix.com>]

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"