From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39044) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W6HMJ-0000pb-TB for qemu-devel@nongnu.org; Thu, 23 Jan 2014 05:18:09 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1W6HMD-0007HJ-Sj for qemu-devel@nongnu.org; Thu, 23 Jan 2014 05:18:03 -0500 Received: from averel.grnet-hq.admin.grnet.gr ([195.251.29.3]:39843) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W6HMD-0007GK-GD for qemu-devel@nongnu.org; Thu, 23 Jan 2014 05:17:57 -0500 Message-ID: <52E0EC4B.7010603@grnet.gr> Date: Thu, 23 Jan 2014 12:17:47 +0200 From: Stratos Psomadakis MIME-Version: 1.0 References: <52DFE990.9000403@grnet.gr> <20140123030714.GA2591@T430.redhat.com> In-Reply-To: <20140123030714.GA2591@T430.redhat.com> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="6dMp2MLEEqc4o15jn927QMvSGwgooSaRg" Subject: Re: [Qemu-devel] Possible bug in monitor code List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng Cc: Synnefo Development , Ganeti Development , qemu-devel@nongnu.org, Dimitris Aragiorgis This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --6dMp2MLEEqc4o15jn927QMvSGwgooSaRg Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable On 01/23/2014 05:07 AM, Fam Zheng wrote: > On Wed, 01/22 17:53, Stratos Psomadakis wrote: >> Hi, >> >> we've encountered a weird issue regarding monitor (qmp and hmp) behavi= or >> with qemu-1.7 (and qemu-1.5). The following steps will reproduce the i= ssue: >> >> 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!!! > Hi Stratos, thank you for debugging and reporting this. > > I tested this sequence but can't fully reproduce this. What I see is 5)= but no > 10). Client C acts normally. And your patch below doesn't solve it for = me. Hm, which qemu version (or repo branch / tag) did you use? We did a quick scan of the master branch code / commits, but we didn't find anything that might fix the issue. > To submit a patch, please follow instructions as described in > http://wiki.qemu.org/Contribute/SubmitAPatch > so it could be picked up by maintainers. Specifically, you need to form= at your > patch email with "git format-patch" and add a "Signed-off-by:" line in = your > patch email. Ok. If any dev can confirm that this is a bug (and that the patch below is the correct way to fix it) I'll resubmit it properly. Thanks, Stratos > Thanks, > > Fam > >> 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 no= w >> 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 operatio= n. >> Thus, mon->outbuf is not cleaned up properly, which results in duplica= te >> 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 >> >> > --=20 Stratos Psomadakis --6dMp2MLEEqc4o15jn927QMvSGwgooSaRg 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/ iQIcBAEBAgAGBQJS4OxTAAoJEO/NY8gC4r+p0M0QAMupI1TyYNbZgabhCuzUaF9n zg0YVSmC1HpRYBU8Quw2XU3ZV96eu34txAjHajxBp/hqnpBOvNm/93cJT9peTMsu jKOPDD6iI6mXziaDUXCiBigeAaFb8gi+aTeISmVulRNjFeeBqVXnvw4VYF/S0N0P zQPSr4th2iI5cto2HG6Xh7Y1nZoAmS//uj9482T1WwJNu08qtK9K/CGpFkNcJhZH 9CQ+KDcoG0670mQA+Glpf/gLl5hBRxVtyJTomvgivrOC01efOW2AXgAnHdts4vwU DARUisx5+p4LTFIpCLuIS2DiyXvXecH8j7JFi+zVvrgF5raPOSL+7IJXyugt1eQW v3x+qdqXTPpKWG9r+LfSz+KuzRffO0iwCg1wZk6CcndQhbrqzFcws3rxF8oKOZjs O7fXt4JaymxTlDJlO6sYinnVe+ZhxYYNVog9D3ugVgsvyqw2Oqso2iy6OfovRcLN tIOxh41buhDhshYNXSjKLTxWI9hENDHfAfTD2U18IH9LAGv+Oq3SIymrl6j0uKzj O5dMHvzvzaTOh8p64iViSwFgZtZGviHMfKwJtKEF3Shw2ovfRBtFutsi07yZD11g 2o+bgXOmycCjPhFq/zMP12FRXxQaaqSRk2kTAsfsotm7LtCyI0++lP1xWistbPhZ va0HYLzYhvGkY8ArZnU+ =vYLj -----END PGP SIGNATURE----- --6dMp2MLEEqc4o15jn927QMvSGwgooSaRg--