|
From: | Stratos Psomadakis |
Subject: | [Qemu-devel] Possible bug in monitor code |
Date: | Wed, 22 Jan 2014 17:53:52 +0200 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130801 Thunderbird/17.0.8 |
Hi, we've encountered a weird issue regarding monitor (qmp and hmp) behavior with qemu-1.7 (and qemu-1.5). The following steps will reproduce the issue: After some investigation, we traced it down to the monitor_flush() function in monitor.c. Specifically, when a second client connects to the qmp (client B), while another one is already using it (client A), we get the following from stracing the second client (client B):1) Client A connects to qmp socket with socat 2) Client A gets greeting message {"QMP": {"version": ..} 3) Client A waits (select on the socket's fd) 4) Client B tries to connect to the *same* qmp socket with socat 5) Client B does *NOT* get any greating message 6) Client B waits (select on the socket's fd) 7) Client B closes connection (kill socat) 8) Client A quits too 9) Client C connects to qmp socket 10) Client C gets *two* greeting messages!!! connect(3, {sa_family=AF_FILE, path="foo.mon"}, 9) = 0So, the connect() syscall from client B succeeds, although client B connection has not yet been accepted by the qmp server (it's still in the backlog of the qmp listening socket). After killing client B and then client A, we see the following when stracing the qemu proc: 22363 accept4(6, {sa_family=AF_FILE, NULL}, [2], SOCK_CLOEXEC) = 9The qmp server / qemu accepts the connection from client B (who has now closed the connection) and tries to write the greeting message to the socket fd. This results in write returning an error (EPIPE). The monitor_flush() function doesn't seem to handle this case (write error). Instead, it adds a watch / handler to retry the write operation. Thus, mon->outbuf is not cleaned up properly, which results in duplicate greeting messages for the next client to connect. The following seems to do the trick. diff --git a/monitor.c b/monitor.c index 845f608..5622f20 100644 --- a/monitor.c +++ b/monitor.c @@ -288,8 +288,8 @@ void monitor_flush(Monitor *mon) if (len && !mon->mux_out) { rc = qemu_chr_fe_write(mon->chr, (const uint8_t *) buf, len); - if (rc == len) { - /* all flushed */ + if ((rc < 0 && errno != EAGAIN) || (rc == len)) { + /* all flushed or error */ QDECREF(mon->outbuf); mon->outbuf = qstring_new(); return; Comments? Thanks, Stratos -- Stratos Psomadakis <address@hidden> |
signature.asc
Description: OpenPGP digital signature
[Prev in Thread] | Current Thread | [Next in Thread] |