From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56696) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XSUDg-0006x2-6q for qemu-devel@nongnu.org; Fri, 12 Sep 2014 13:01:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XSUDa-0007yL-2k for qemu-devel@nongnu.org; Fri, 12 Sep 2014 13:01:12 -0400 Sender: Stratos Psomadakis Message-ID: <541326D0.4040102@grnet.gr> Date: Fri, 12 Sep 2014 20:01:04 +0300 From: Stratos Psomadakis MIME-Version: 1.0 References: <87wq991g1c.fsf@blackfin.pond.sub.org> <1410530852-13631-1-git-send-email-psomas@grnet.gr> <20140912112110.3b61c84b@redhat.com> In-Reply-To: <20140912112110.3b61c84b@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="5Lgog36hl8o0t1nhHrfVVt62qG8wgG4Wi" Subject: Re: [Qemu-devel] [synnefo-devel] Re: [PATCH resend 1/2] monitor: Reset HMP mon->rs on CHR_EVENT_CLOSED List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: synnefo-devel@googlegroups.com, qemu-devel@nongnu.org, armbru@redhat.com, qemu-stable@nongnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --5Lgog36hl8o0t1nhHrfVVt62qG8wgG4Wi Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 12/09/2014 06:21 =CE=BC=CE=BC, Luiz Capitulino wrote: > On Fri, 12 Sep 2014 17:07:32 +0300 > Stratos Psomadakis wrote: > >> Commit cdaa86a54 ("Add G_IO_HUP handler for socket chardev") exposed a= bug in >> the way the HMP monitor handles its command buffer. When a client clos= es the >> connection to the monitor, tcp_chr_read() will detect the G_IO_HUP con= dition >> and call tcp_chr_disconnect() to close the server-side connection too.= Due to >> the fact that monitor reads 1 byte at a time (for each tcp_chr_read())= , the >> monitor readline state / buffers might contain junk (i.e. a half-finis= hed >> command). Thus, without calling readline_restart() on mon->rs upon >> CHR_EVENT_CLOSED, future HMP commands will fail. > What's your reproducer? We have a script that opens a connection to the HMP socket and starts sending 'info version' commands to the monitor in a loop. If we kill the script (in the middle of the loop) and re-run it, we get "unknown command" errors from the HMP ("unknown command: 'infinfo'" for example). > Are you using the mux feature? Nope (on the cli we use '-monitor unix:.mon,server,nowait' for the HMP). > We also reset it > in CHR_EVENT_OPENED if the mux feature is not used, why isn't that > good enough? I checked the code and on CHR_EVENT_OPENED the monitor calls readline_show_prompt (when not using mux). This resets the last_cmd_index/size readline variables, but the cmd_buf_index/size remains intact. I think that readline_restart() is necessary in order to cleanup the readline cmd buf (either in CHR_EVENT_OPENED or in CHR_EVENT_CLOSED). Thanks, Stratos > >> 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; --=20 Stratos Psomadakis --5Lgog36hl8o0t1nhHrfVVt62qG8wgG4Wi 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 iQIcBAEBAgAGBQJUEybQAAoJEO/NY8gC4r+phhIP/i1EPx3FmbT4DURtoVhSqqZs SCMkrK3HpwSaAMfqwAMWufgNWXM+sVj3rCPQjnuadZ/kDoSgmNrKA0Y2tq2X4J38 ls5JFAve8CseYkTF3Em06Hdfyy7DjEaCoiz2WG5x0Mrxcfd1y6IPKBuyHUTJqsH9 uEftS4+03/x+Ai5wm3JKOj5ENZjSi/VVkAX8PfGNM6JrBCd/kfCNOuz9pwiB/IZj BrRmLhcGJSJGDxF3ORZsOL0Pbj4Y99IfhEPbmamov8tJ26gO6kjmIkkQZbz/tuJd a16b/raKzJrkw3F6p99T5dd06R4enGKz7g4T+0gpHA9SsYZg8EQYhu0YPPU/XQG9 pRSIrpUXIy7ZICV91G/BeEsWe1la5uiuTeCPeyqL/9QKxLea/3IvAjFhCu9zY42V UAlbrjDceJKc4n28TbECM2Zjwi8hOI5evsSb7E2scA32CzwVZ3lsC4TrvQmUN5Jk jaSsvM0liYqzsgJqe1TJr/fZsiGK44tGzYiUtiW+qcU1U8/UY9dpLIoBYlY713qr VsjYF4N7rGxpDhHAVAFONkv8fZ0FBR5WfFJdy51ttIAaLTvNvo6N+ga6uFp2NGPN Pl/XeSLZxA2YrT9yquDPFSoKcA9oGeyW3u3CcET63S54cMsAtcTMOmZNOR3MJFAD 6bIHHz2QXcq8UHxguETx =mstZ -----END PGP SIGNATURE----- --5Lgog36hl8o0t1nhHrfVVt62qG8wgG4Wi--