From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:43201) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UQaDP-0004wA-NJ for qemu-devel@nongnu.org; Fri, 12 Apr 2013 05:24:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UQaDO-0003zZ-FC for qemu-devel@nongnu.org; Fri, 12 Apr 2013 05:24:15 -0400 Received: from mx1.redhat.com ([209.132.183.28]:17663) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UQaDO-0003zT-87 for qemu-devel@nongnu.org; Fri, 12 Apr 2013 05:24:14 -0400 Date: Fri, 12 Apr 2013 14:54:10 +0530 From: Amit Shah Message-ID: <20130412092410.GC17454@amit.redhat.com> References: <1365600207-21685-1-git-send-email-pbonzini@redhat.com> <20130410175934.GA5213@amit.redhat.com> <51667B36.9020107@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <51667B36.9020107@redhat.com> 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: Hans de Goede , aliguori@us.ibm.com, qemu-devel@nongnu.org On (Thu) 11 Apr 2013 [10:58:30], Paolo Bonzini wrote: > Il 10/04/2013 19:59, Amit Shah ha scritto: > > On (Wed) 10 Apr 2013 [15:23:27], 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. > > > > This patch doesn't apply directly to master, applies with some fuzz. > > However, this patch causes qemu freeze. My testcase is: > > > > Open chardev on host > > Write something to a virtserialport in guest > > Close chardev on host > > Keep writing to virtserialport in guest > > > > When I apply the patch to the old qemu version with select, that > > starts working fine with the testcase above. > > I cannot replicate the freeze. The patch works on both old and new > versions of QEMU. My testcases are: > > 1) on host, nc -l -p 12345 > on host, start qemu > in guest, cat > /dev/vport0p1 > in guest, write something > on host, close nc > in guest, write something > in guest, ^D and poweroff > > 2) on host, nc -l -p 12345 > on host, start qemu > in guest, echo abc > /dev/vport0p1 > on host, close nc > in guest, echo abc > /dev/vport0p1 > in guest, poweroff Can you try multiple writes from the guest? At least 3-4? QEMU doesn't detect a backend getting closed right away (another bug), so the freeze doesn't trigger til qemu detects there's no chardev anymore. > > There's a slight difference in my old qemu tree, I have Hans's > > "virtio-console: Remove any pending watches on close" patch applied, > > which makes use of the tag obtained on adding the watch. That patch > > hasn't found its way to master yet, but it should go in soon. > > I don't have that patch in my (new) tree. It's vanilla upstream QEMU. Yep, I tested upstream QEMU from master as well. (It's just my 'old' qemu tree which has Hans's patches too.) Amit