Amit Shah <amit.s...@redhat.com> wrote:
> If the char device we're connected to is overwhelmed with data and it
> can't accept any more, signal to the virtio-serial-bus to stop sending
> us more data till we tell otherwise.
>
> If the current buffer being processed hasn't been completely written out
> to the char device, we have to keep it around and re-try sending it
> since the virtio-serial-bus code assumes we consume the entire buffer.
>
> Allow the chardev backends to return -EAGAIN; we're ready with a
> callback handler that will flush the remainder of the buffer.
>
> Also register with savevm so that we save/restore such a buffer across
> migration.
>
> Signed-off-by: Amit Shah <amit.s...@redhat.com>
> ---
> hw/virtio-console.c | 126 +++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 files changed, 123 insertions(+), 3 deletions(-)
>
> diff --git a/hw/virtio-console.c b/hw/virtio-console.c
> index 749ed59..7eb6aa1 100644
> --- a/hw/virtio-console.c
> +++ b/hw/virtio-console.c
> @@ -13,18 +13,92 @@
> #include "qemu-char.h"
> #include "virtio-serial.h"
>
> +typedef struct Buffer {
> + uint8_t *buf;
> + size_t rem_len;
> + size_t offset;
> +} Buffer;
> +
> typedef struct VirtConsole {
> VirtIOSerialPort port;
> CharDriverState *chr;
> + Buffer *unflushed_buf;
This part of teh struct can be static instead of a pointer, and adjust
rest of code accordingly. for the rest, it looks ok.
For this to work, I think that you will have to change rem_len for
total_len (or whatever) and adjust code around. This makes trivial to
check for buffer used/no, just see if len = 0.
> +static int buffered_write_to_chardev(VirtConsole *vcon, const uint8_t *buf,
> + size_t len)
> +{
> + size_t written;
> + ssize_t ret;
> +
> + written = 0;
> + do {
> + ret = qemu_chr_write_nb(vcon->chr, buf + written, len - written);
> + if (ret < 0) {
> + if (vcon->unflushed_buf) {
> + vcon->unflushed_buf->offset += written;
> + vcon->unflushed_buf->rem_len -= written;
> + } else {
> + virtio_serial_throttle_port(&vcon->port, true);
> + add_unflushed_buf(vcon, buf + written, len - written);
> + }
> +
> + return -EAGAIN;
You can "return ret" instead of "inventing" a new error :)
> +static void virtio_console_port_save(QEMUFile *f, void *opaque)
> +{
> + VirtConsole *vcon = opaque;
> + uint32_t have_buffer;
> +
> + have_buffer = vcon->unflushed_buf ? true : false;
So, here you can just write vcon->unflushed_buf.len if you go with my
proposal, what will make VMState transition easier.
Rest of patch is good. I also like the series, specially the change of
qemu_chr_add_handlers() to use one struct to pass its arguments.
Later, Juan.
