Re: [Qemu-devel] [PATCH] qemu-char: another io_add_watch_poll fix

[Amit Shah]

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