From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54379) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZFhD9-0002MW-EY for qemu-devel@nongnu.org; Thu, 16 Jul 2015 07:20:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZFhD3-0004Bz-Lb for qemu-devel@nongnu.org; Thu, 16 Jul 2015 07:20:19 -0400 Received: from mx1.redhat.com ([209.132.183.28]:44352) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZFhD3-0004Bk-G5 for qemu-devel@nongnu.org; Thu, 16 Jul 2015 07:20:13 -0400 Date: Thu, 16 Jul 2015 16:49:54 +0530 From: Amit Shah Message-ID: <20150716111954.GY10280@grmbl.mre> References: <1436775182-67309-1-git-send-email-pyssling@ludd.ltu.se> <55A38FBA.50505@redhat.com> <20150713104149.GB11699@grmbl.mre> <20150716062448.GX10280@grmbl.mre> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH] qemu-char: Fix missed data on unix socket List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Nils Carlson Cc: Paolo Bonzini , liangshunbin@jovaunn.com, qemu-devel@nongnu.org On (Thu) 16 Jul 2015 [10:11:04], Nils Carlson wrote: > On Thu, 16 Jul 2015, Amit Shah wrote: > > >On (Wed) 15 Jul 2015 [23:44:52], Nils Carlson wrote: > >>On Mon, 13 Jul 2015, Nils Carlson wrote: > >> > >>>On Mon, 13 Jul 2015, Amit Shah wrote: > >> > >> > >> > >>>>Also, returning TRUE there isn't right - if the connection ends, we > >>>>should return FALSE. > >>> > >>>I agree that this seems reasonable. I will change it and re-test. > >>> > >> > >>I had a closer look, and it seems always returning true is intentional here, > >>the called function, tcp_chr_disconnect(chr), handles the deregistration > >>from handlers. If we were to return FALSE we would be duplicating work and > >>possibly breaking things. > > > >Not sure how. > > > >Anyway, can you please start a new thread, with the author and > >reviewers of the patch CC'ed, so they can chime in as well? > > The authors and reviewers of which patch exactly? > > The fix for windows which broke cloudstack was done in 812c1057. Yea, the author of this patch. Also, I meant the return TRUE in this patch, not the one Paolo added. > The returning TRUE everywhere was done in commit cdbf6e16 by Paolo Bonzini, > CC:d above. > > I don't think there was anything wrong with either patch, the former one > simply broken a strange behaviour CloudStack was relying on which I would > say is dubious (fire and forget messaging.) There need not be anything wrong with a previous patch; but just because someone touched the area recently, they might have more context, and even help in spotting the bug. Amit