[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] qemu-char: another io_add_watch_poll fix
From: |
Amit Shah |
Subject: |
Re: [Qemu-devel] [PATCH] qemu-char: another io_add_watch_poll fix |
Date: |
Fri, 12 Apr 2013 14:54:10 +0530 |
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