Hi,
The commit 812c1057f, Handle G_IO_HUP in tcp_chr_read for tcp chardev,
broke CloudStack. CloudStack was relying on fire-and-forget style
messaging across a unix socket to the VM. Because the host "fires" the
message and then closes the socket a HUP is present on the line when the
VM starts reading the socket. Commit 812c1057f ensured that the socket was
checked for a HUP prior to calling recv, causing recv never to be called
by the VM and no data to be read.
I've posted a patch, attached here, which moves the HUP detection to after
all data has been read, but only for Linux as I suspect windows requires
HUPs to be detected prior to reading data.
Could you comment on the validity of this assumption? I would be really
happy to have this issue solved as it stops us from upgrading to later
versions of qemu.
Amit also has concerns regarding the return values from the tcp_chr_read
function, which seem a bit odd as they are all TRUE, even for failure
paths.
All feedback very much appreciated.
Best Regards,
Nils Carlson
From pyssl...@ludd.ltu.se Thu Jul 16 01:01:31 2015
Date: Wed, 15 Jul 2015 23:00:23 +0000
From: pyssl...@ludd.ltu.se
To: pbonz...@redhat.com, qemu-devel@nongnu.org
Cc: Nils Carlson <pyssl...@ludd.ltu.se>
Subject: [Qemu-devel] [PATCH v2] qemu-char: Fix missed data on unix socket
From: Nils Carlson <pyssl...@ludd.ltu.se>
Commit 812c1057 introduced HUP detection on unix and tcp sockets prior
to a read in tcp_chr_read. This unfortunately broke CloudStack 4.2
which relied on the old behaviour where data on a socket was readable
even if a HUP was present.
On Linux a working solution seems to be to simply check the HUP after
reading all available data, i.e. recv returns a negative value,
while keeping the previous behaviour for Windows as it is known to
work.
Signed-off-by: Nils Carlson <pyssl...@ludd.ltu.se>
---
qemu-char.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/qemu-char.c b/qemu-char.c
index 617e034..1e9895e 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2847,11 +2847,13 @@ static gboolean tcp_chr_read(GIOChannel *chan, GIOCondition cond, void *opaque)
uint8_t buf[READ_BUF_LEN];
int len, size;
+#ifdef _WIN32
if (cond & G_IO_HUP) {
/* connection closed */
tcp_chr_disconnect(chr);
return TRUE;
}
+#endif
if (!s->connected || s->max_size <= 0) {
return TRUE;
@@ -2860,7 +2862,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 && (cond & G_IO_HUP))) {
/* connection closed */
tcp_chr_disconnect(chr);
} else if (size > 0) {
--
1.7.10.4
