From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35315) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XSRII-0008VE-Fq for qemu-devel@nongnu.org; Fri, 12 Sep 2014 09:53:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XSRID-0006wg-Dn for qemu-devel@nongnu.org; Fri, 12 Sep 2014 09:53:46 -0400 Sender: Stratos Psomadakis Message-ID: <5412FADC.30802@grnet.gr> Date: Fri, 12 Sep 2014 16:53:32 +0300 From: Stratos Psomadakis MIME-Version: 1.0 References: <1410448769-10495-1-git-send-email-psomas@grnet.gr> <1410448769-10495-2-git-send-email-psomas@grnet.gr> <87wq991g1c.fsf@blackfin.pond.sub.org> In-Reply-To: <87wq991g1c.fsf@blackfin.pond.sub.org> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="wDRsIjbWVHw6aSqFb6MQW0SBcPLSbplhl" Subject: Re: [Qemu-devel] [synnefo-devel] Re: [PATCH 1/2] monitor: Reset HMP mon->rs on CHR_EVENT_CLOSED List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: synnefo-devel@googlegroups.com, qemu-stable@nongnu.org, qemu-devel@nongnu.org, lcapitulino@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --wDRsIjbWVHw6aSqFb6MQW0SBcPLSbplhl Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 12/09/2014 09:58 =CF=80=CE=BC, Markus Armbruster wrote: > Stratos Psomadakis writes: > >> Commit cdaa86a54 ("Add G_IO_HUP handler for socket chardev") exposed a= >> bug in the way the HMP monitor handles its input. When a client close= s >> the connection to the monitor, tcp_chr_read() will catch the HUP >> 'signal' and call tcp_chr_disconnect() to close the server-side >> connection too. > Your wording suggests SIGUP, but that's misleading. Suggest > "tcp_chr_read() will detect the G_IO_HUP condition, and call". ack > >> Due to the fact that monitor reads 1 byte at a time (f= or >> each tcp_chr_read()), the monitor readline state / buffers can be left= >> in an inconsistent state (i.e. a half-finished command). > The state is not really inconsistent, there's just junk left in > rs->cmd_buf[]. ack >> Thus, without= >> calling readline_restart() on mon->rs upon CHR_EVENT_CLOSED, future HM= P >> commands will fail. > To make sure I understand you correctly: when you connect again, any > leftover junk is prepended to your input, which messes up your first > command. Correct? Yeap. >> Signed-off-by: Stratos Psomadakis >> Signed-off-by: Dimitris Aragiorgis >> --- >> monitor.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/monitor.c b/monitor.c >> index 34cee74..7857300 100644 >> --- a/monitor.c >> +++ b/monitor.c >> @@ -5252,6 +5252,7 @@ static void monitor_event(void *opaque, int even= t) >> break; >> =20 >> case CHR_EVENT_CLOSED: >> + readline_restart(mon->rs); >> mon_refcount--; >> monitor_fdsets_cleanup(); >> break; > Patch looks good to me. ok, I'll edit the commit msg and resend the patch. Thanks, Stratos > --=20 Stratos Psomadakis --wDRsIjbWVHw6aSqFb6MQW0SBcPLSbplhl Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJUEvrcAAoJEO/NY8gC4r+ptqgQANIo3CTuhHtgraMlb6yY+pCw 1+ne9GCl4qBVRMrNB8yFHpz4f813Nctn+Wals8fvPrXWiL+fr42eTZdyiEw1h53k kVaZAK48V4Oha7q7w40Q8411PjwuUdloTo4zzkkkrwbePjgBZo3WHO9qM+/zSR+p WNZjaseA4soVPCwLl6ND+/o7MzxvYUqCNBXELIl7qPIcdzuDwo9xGa+MDTrs7erF hPhXRSIevD2THZgVeKlDSzwMTn4oFDNvjpTnK3cFmnUyziJ20FnWzUIkmEcKqay/ X//uxp4jq4Q6SJj/DoibSr3i+NfHsS8y7ZGFfTRRtlgfijiZdgzg7mti5mu05WBh KSO4zX6HWMUxVSSzn5ISX5EeHeMFadnxC3tvksz+x9nEfGqCXjdiqUoEXuHOi1Jv ePj3y/IWIajBeHeh2ER5EAJsakqdldwYRE67U668OKohUzvJdLzAXn0dnDDZ6smA GfQPUORg7GU7yPYYxi1lEij5B7qihoFoHV3CgOyEPyYb82aVWwesDFXrj58VHgdb 2ukDuw72/sYJDbMCNdqT+HR7zGUOTL7N+xQPJIuKMIsfq/2ELuTJpnXeatVB8Ue/ ipJ0tW4BKGtydU21aH0KARrCKIQBYl06ALT63hri/8ytqghYALdGS1aoZSsedfN/ 6nFyC48HCUkWyPlu2Qqm =HdW1 -----END PGP SIGNATURE----- --wDRsIjbWVHw6aSqFb6MQW0SBcPLSbplhl--