On Wed, Oct 19, 2022 at 05:26:28PM +0100, Peter Maydell wrote: > On Tue, 18 Oct 2022 at 20:21, Daniel P. Berrangé <berra...@redhat.com> wrote: > > > > On Tue, Oct 18, 2022 at 06:55:08PM +0100, Peter Maydell wrote: > > > How is this intended to work? I guess the socket ought to go > > > into some kind of "disconnecting" state, but not actually do > > > a tcp_chr_disconnect() until all the data has been read via > > > tcp_chr_read() and it's finally got an EOF indication back from > > > tcp_chr_recv() ? > > > > Right, this is basically broken by (lack of) design right now. > > > > The main problem here is that we're watching the socket twice. > > One set of callbacks added with io_add_watch_poll, and then > > a second callback added with qio_chanel_create_watch just for > > G_IO_HUP. > > > > We need there to be only 1 callback, and when that callback > > gets G_IO_IN, it should *ignore* G_IO_HUP until tcp_chr_recv > > returns 0 to indicate EOF. This would cause tcp_chr_read to > > be invoked repeatedly with G_IO_IN | G_IO_HUP, as we read > > "halt\r" one byte at a time. > > Makes sense. > > I've filed https://gitlab.com/qemu-project/qemu/-/issues/1264 to > track this socket chardev bug. > > It did occur to me that there's a potential complication with > the 'server' mode of this chardev: does it need to cope with > a new connection coming into the server socket while the old > fd is still hanging around in this "waiting for the guest to > read it" state? Currently tcp_chr_disconnect_locked() is where > we restart listening for new connections, so QEMU wouldn't > accept any new connection until the guest had got round to > completely draining the data from the old one.
That's fine IMHO. We never actually stop listening at a socket level, we just stop trying to accept(). So any new client will get queued until we've drained data, then accept()d and its new data handled With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|