From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48860) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W60Tl-0005eT-2v for qemu-devel@nongnu.org; Wed, 22 Jan 2014 11:16:44 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1W60Td-0004KK-M6 for qemu-devel@nongnu.org; Wed, 22 Jan 2014 11:16:37 -0500 Received: from averel.grnet-hq.admin.grnet.gr ([195.251.29.3]:21384) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W60Td-0004Gs-93 for qemu-devel@nongnu.org; Wed, 22 Jan 2014 11:16:29 -0500 Message-ID: <52DFE990.9000403@grnet.gr> Date: Wed, 22 Jan 2014 17:53:52 +0200 From: Stratos Psomadakis MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="EDtDk9I24eEoh8dVLVmtX8ejTikJEmaWp" Subject: [Qemu-devel] Possible bug in monitor code List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Synnefo Development , Stratos Psomadakis , Ganeti Development , Dimitris Aragiorgis This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --EDtDk9I24eEoh8dVLVmtX8ejTikJEmaWp Content-Type: multipart/alternative; boundary="------------000603050805080500040709" This is a multi-part message in MIME format. --------------000603050805080500040709 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable 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 issu= e: 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!!! 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): connect(3, {sa_family=3DAF_FILE, path=3D"foo.mon"}, 9) =3D 0 getsockname(3, {sa_family=3DAF_FILE, NULL}, [2]) =3D 0 select(4, [0 3], [1 3], [], NULL) =3D 2 (out [1 3]) select(4, [0 3], [], [], NULL So, 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=3DAF_FILE, NULL}, [2], SOCK_CLOEXEC) =3D = 9 22363 fcntl(9, F_GETFL) =3D 0x2 (flags O_RDWR) 22363 fcntl(9, F_SETFL, O_RDWR|O_NONBLOCK) =3D 0 22363 fstat(9, {st_mode=3DS_IFSOCK|0777, st_size=3D0, ...}) =3D 0 22363 fcntl(9, F_GETFL) =3D 0x802 (flags O_RDWR|O_NONBLOCK) 22363 write(9, "{\"QMP\": {\"version\": {\"qemu\": {\"m"..., 127) =3D= -1 EPIPE (Broken pipe) 22363 --- SIGPIPE (Broken pipe) @ 0 (0) --- The 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) =20 if (len && !mon->mux_out) { rc =3D qemu_chr_fe_write(mon->chr, (const uint8_t *) buf, len); - if (rc =3D=3D len) { - /* all flushed */ + if ((rc < 0 && errno !=3D EAGAIN) || (rc =3D=3D len)) { + /* all flushed or error */ QDECREF(mon->outbuf); mon->outbuf =3D qstring_new(); return; Comments? Thanks, Stratos --=20 Stratos Psomadakis --------------000603050805080500040709 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable 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:
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 *<=
/span>NOT* get any greating messag=
e
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!!!
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):<= br>
connect(3, {sa_family=3DAF_FILE, path=3D"foo.mon"}, 9) =3D= 0
getsockname(3, {sa_family=3DAF_FILE, NULL}, [2]) =3D 0
select(4, [0 3], [1 3], [], NULL)=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =3D 2 (out [1 3])
select(4, [0 3], [], [], NULL
So, 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=3DAF_FILE, NULL}, [2], SOCK_CLOEXEC) =3D 9
22363 fcntl(9, F_GETFL)=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 =3D 0x2 (flags O_RDWR= )
22363 fcntl(9, F_SETFL, O_RDWR|O_NONBLOCK) =3D 0
22363 fstat(9, {st_mode=3DS_IFSOCK|0777, st_size=3D0, ...}) =3D 0 22363 fcntl(9, F_GETFL)=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 =3D 0x802 (flags O_RDWR|O_NONBLOCK)
22363 write(9, "{\"QMP\": {\"version\": {\"qemu\": {\"m"..., 127) =3D -1 EPIPE (Broken pipe)
22363 --- SIGPIPE (Broken pipe) @ 0 (0) ---
The 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)
=C2=A0
=C2=A0=C2=A0=C2=A0=C2=A0 if (len && !mon->mux_out) {
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 rc =3D qemu_chr_fe_w= rite(mon->chr, (const uint8_t *) buf, len);
-=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (rc =3D=3D len) {
-=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 /= * all flushed */
+=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if ((rc < 0 &&= errno !=3D EAGAIN) || (rc =3D=3D len)) {
+=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 /= * all flushed or error */
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0 QDECREF(mon->outbuf);
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0 mon->outbuf =3D qstring_new();
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0 return;

Comments?

Thanks,
Stratos

--=20
Stratos Psomadakis
<ps=
omas@grnet.gr>
--------------000603050805080500040709-- --EDtDk9I24eEoh8dVLVmtX8ejTikJEmaWp Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.21 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBAgAGBQJS3+mWAAoJEO/NY8gC4r+p/W0QAMDoG9Ua2Ueg6PhSJO4xegCH wcYf3tfGC8cJx1BEUg541SCw8/sH6TSoOx26J2aWxWsUO3cfMqGLyqSrrEwFdqaR hr7kIPU6s5+BxMHYerTmsplNXn0ynWO44oOcJftXW2DVXwCvz4+od8k/GJlIXwIh D5XB7hevKnqbeqbHBRBfIAmymHBcFSYCJ45fpS+hlaOCwBJkNtrA/9a4u/224L7+ V8yha50cL0I+MhHz8+qOratEDVHBN3Iur20ixiMnOIrZfA15Bf/iiD2kbXpDjY1J 0BxImiBIS1a+rs7xQzO9COCoTYMOlzeWJiKw9qIRhiWN7WLxwfYSEl5UaziYF70N PnPop0041GtAykOccrNXlgQgyekkedyDutmI+LVjUSyxSPdM+qmYP9+rmSzAIK90 BnyYGBGnyy748sTvWXYomlgdnVhkSqbFBBdCLDBp+9nsGT7CvnQMffVdLPMF+eer 47J0V63DKl9JzTpOzPnRyz4I/7DSC9GeBJ7MQKsR2rOvr3QYnACZ6oCkRKXpRsM7 3ICbyT660JhvGXJSAMgPbz3CAdMNjJinH1XSTmqgtZpvPGkjE3gbjHxNJKXCJhPx wjVGoqdBKsAal1dflytmh7445JPlkArnoPIrGFk/ZxxmCnFFSehQ/XfF/phYfmdI gnEcIZES6lnJM4t7gi/H =DTGR -----END PGP SIGNATURE----- --EDtDk9I24eEoh8dVLVmtX8ejTikJEmaWp--