[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] char: fix repeated registration of tcp chardev
From: |
Mark Cave-Ayland |
Subject: |
Re: [Qemu-devel] [PATCH] char: fix repeated registration of tcp chardev I/O handlers |
Date: |
Mon, 8 Feb 2016 21:53:33 +0000 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Icedove/38.5.0 |
On 08/02/16 13:55, Daniel P. Berrange wrote:
> In previous commit:
>
> commit f2001a7e0555b66d6db25a3ff1801540814045bb
> Author: Daniel P. Berrange <address@hidden>
> Date: Tue Jan 19 11:14:30 2016 +0000
>
> char: don't assume telnet initialization will not block
>
> The code which writes the telnet initialization sequence moved
> to an event loop callback. If the TCP chardev is opened as a
> server in blocking mode (ie -serial telnet:0.0.0.0:3000,server,wait)
> this results in a state where the TCP chardev is connected, but not
> yet ready to send/recv data when virtual hardware is created.
>
> When the virtual hardware initialization registers its chardev
> callbacks, it triggers tcp_chr_update_read_handler, which will
> add I/O watches to the connection.
>
> When the telnet initialization finally runs, it will then call
> tcp_chr_connect to finish the connection setup. This will in
> turn add I/O watches to the connection too.
>
> There are now two sets of I/O watches registered on the same
> connection. This ultimately causes data loss on the connection,
> for example, when typing into the telnet console only every
> second byte is echoed back to the client.
>
> The same flaw can affect channels running with TLS encryption
> too, since they also have delayed connection setup completion.
>
> The fix is to update tcp_chr_update_read_handler so that it
> avoids registering watches if the connection is not fully
> setup yet.
>
> Signed-off-by: Daniel P. Berrange <address@hidden>
> ---
> qemu-char.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/qemu-char.c b/qemu-char.c
> index 927c47e..9060f8a 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -2856,6 +2856,10 @@ static void
> tcp_chr_update_read_handler(CharDriverState *chr)
> {
> TCPCharDriver *s = chr->opaque;
>
> + if (!s->connected) {
> + return;
> + }
> +
> remove_fd_in_watch(chr);
> if (s->ioc) {
> chr->fd_in_tag = io_add_watch_poll(s->ioc,
>
Hi Daniel,
Yes I can confirm that this patch fixes the issue for me - thanks a lot
for debugging!
ATB,
Mark.