[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] qemu-char: Do not disconnect when there's da
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v2] qemu-char: Do not disconnect when there's data for reading |
Date: |
Thu, 18 Sep 2014 14:11:23 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) |
Zifei Tong <address@hidden> writes:
> After commit 812c1057f6175ac9a9829fa2920a2b5783814193 (Handle G_IO_HUP
> in tcp_chr_read for tcp chardev), connections are disconnected when in
> G_IO_HUP condition.
>
> However, it's possible that there is still data for reading in the channel.
> In that case, the remaining data is not handled.
>
> I saw a related bug when running socat in write-only mode, after
>
> $ echo "quit" | socat -u - UNIX-CONNECT:qemu-monitor
>
> the monitor won't not run the 'quit' command.
>
> Instead of GIOCondition, this patch uses the return value of tcp_chr_recv()
> to check the state of connection as suggested by Kirill.
>
> Cc: Kirill Batuzov <address@hidden>
> Cc: Nikolay Nikolaev <address@hidden>
> Cc: Markus Armbruster <address@hidden>
> Cc: Anthony Liguori <address@hidden>
> Signed-off-by: Zifei Tong <address@hidden>
> ---
> qemu-char.c | 8 +-------
> 1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/qemu-char.c b/qemu-char.c
> index 2a3cb9f..6cc69fa 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -2692,12 +2692,6 @@ static gboolean tcp_chr_read(GIOChannel *chan,
> GIOCondition cond, void *opaque)
> uint8_t buf[READ_BUF_LEN];
> int len, size;
>
> - if (cond & G_IO_HUP) {
> - /* connection closed */
> - tcp_chr_disconnect(chr);
> - return TRUE;
> - }
> -
> if (!s->connected || s->max_size <= 0) {
> return TRUE;
> }
> @@ -2705,7 +2699,7 @@ static gboolean tcp_chr_read(GIOChannel *chan,
> GIOCondition cond, void *opaque)
> if (len > s->max_size)
> len = s->max_size;
> size = tcp_chr_recv(chr, (void *)buf, len);
> - if (size == 0) {
> + if (size == 0 || (size < 0 && !(errno == EAGAIN || errno == EINTR))) {
> /* connection closed */
> tcp_chr_disconnect(chr);
> } else if (size > 0) {
What about EWOULDBLOCK?
The /* connection closed */ comment is now more inaccurate :)
If size == 0, the peer has shut down its sending end of the connection.
If size < 0, our attempt to receive encountered an error.
I'd just drop the comment.