Re: socket chardevs: data loss when other end closes connection?

[Daniel P . Berrangé]

On Wed, Oct 19, 2022 at 05:26:28PM +0100, Peter Maydell wrote:
> On Tue, 18 Oct 2022 at 20:21, Daniel P. Berrangé <berrange@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 :|