[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/7] virtio-console: Also throttle when less was
From: |
Amit Shah |
Subject: |
Re: [Qemu-devel] [PATCH 1/7] virtio-console: Also throttle when less was written then requested |
Date: |
Tue, 2 Apr 2013 16:10:04 +0530 |
On (Mon) 01 Apr 2013 [18:00:55], Hans de Goede wrote:
> Hi,
>
> On 03/29/2013 10:31 AM, Amit Shah wrote:
> >On (Thu) 28 Mar 2013 [14:28:11], Hans de Goede wrote:
> >>This is necessary so that we get properly woken up to write the rest.
> >>
> >>Signed-off-by: Hans de Goede <address@hidden>
> >>Acked-by: Amit Shah <address@hidden>
> >>---
> >> hw/virtio-console.c | 5 +++--
> >> 1 file changed, 3 insertions(+), 2 deletions(-)
> >>
> >>diff --git a/hw/virtio-console.c b/hw/virtio-console.c
> >>index 284180f..015947c 100644
> >>--- a/hw/virtio-console.c
> >>+++ b/hw/virtio-console.c
> >>@@ -47,7 +47,7 @@ static ssize_t flush_buf(VirtIOSerialPort *port, const
> >>uint8_t *buf, size_t len)
> >> ret = qemu_chr_fe_write(vcon->chr, buf, len);
> >> trace_virtio_console_flush_buf(port->id, len, ret);
> >>
> >>- if (ret <= 0) {
> >>+ if (ret < len) {
> >
> >Oops, there's a conversion bug here: len is size_t, and ret is
> >ssize_t. Both need to be the same type.
> >
> >Since we're not returning negative, ret should be changed to size_t.
> >The function signature changes too, but that's alright.
>
> Erm changing ret to a size_t will break things, since qemu_chr_fe_write can
> return < 0 and storing that into an unsigned will change it into a very
> large value instead ...
Oh, definitely. I meant changing the return type of flush_buf(), but
that doesn't help matters much...
> A better fix would be to change len to ssize_t. Although that will cause
> issues if we ever get a single buf which is larger then 2^31 - 1. Note
> we already have that problem since qemu_chr_fe_write already cannot handle
> such large buffers...
Yep. Do you want to do that as part of this series?
Amit