From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:39247) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1USUM9-0008PI-CV for qemu-devel@nongnu.org; Wed, 17 Apr 2013 11:33:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1USUM8-0004Ey-02 for qemu-devel@nongnu.org; Wed, 17 Apr 2013 11:33:09 -0400 Received: from mx1.redhat.com ([209.132.183.28]:49974) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1USUM7-0004Ed-PE for qemu-devel@nongnu.org; Wed, 17 Apr 2013 11:33:07 -0400 Message-ID: <516EC18E.7060702@redhat.com> Date: Wed, 17 Apr 2013 17:36:46 +0200 From: Hans de Goede MIME-Version: 1.0 References: <1365600207-21685-1-git-send-email-pbonzini@redhat.com> <516D16A4.2030105@redhat.com> <516D1B1B.6020603@redhat.com> In-Reply-To: <516D1B1B.6020603@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] qemu-char: another io_add_watch_poll fix List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: amit.shah@redhat.com, aliguori@us.ibm.com, Gerd Hoffmann , qemu-devel@nongnu.org Ji, On 04/16/2013 11:34 AM, Paolo Bonzini wrote: > Il 16/04/2013 11:15, Gerd Hoffmann ha scritto: >> On 04/10/13 15:23, Paolo Bonzini wrote: >>> After attaching the source, we have to remove the reference we hold >>> to it, because we do not hold anymore a pointer to the source. >>> >>> If we do not do this, removing the source will not finalize it and >>> will not drop the "real" I/O watch source. >>> >>> This showed up when backporting the new flow control patches to older >>> versions of QEMU that still used select. The whole select then failed >>> with EBADF (poll instead will reporting POLLNVAL on a single pollfd) >>> and QEMU froze. >> >> I get freezes now in master, bisecting points to this patch. >> >> Reproducer: "qemu -serial pty". >> >> qemu is pretty much unusable with libvirt now as libvirt uses pty >> chardevs by default for serial & monitor ... > > I'm not sure why all users of qemu_chr_fe_add_watch believe that the > watch will be one-shot. This is definitely not what g_io_create_watch > does... It is supposed to depend on the return value of the callback, if you return False, then the source should be removed, in essence making the watch one-shot, see: http://www.gtk.org/api/2.6/glib/glib-The-Main-Event-Loop.html#GSourceFunc Which specifically says: Returns: it should return FALSE if the source should be removed. And using sources in a one-shot mode by making the callback return false is quite normal in the glib world. Regards, Hans