Re: [Qemu-devel] [PATCH] chardev/char-socket: add POLLHUP handler

[Daniel P. Berrange]

On Thu, Jan 18, 2018 at 12:41:08PM +0300, klim wrote:
> On 01/16/2018 08:25 PM, Paolo Bonzini wrote:
> > On 10/01/2018 14:18, Klim Kireev wrote:
> > > The following behavior was observed for QEMU configured by libvirt
> > > to use guest agent as usual for the guests without virtio-serial
> > > driver (Windows or the guest remaining in BIOS stage).
> > > 
> > > In QEMU on first connect to listen character device socket
> > > the listen socket is removed from poll just after the accept().
> > > virtio_serial_guest_ready() returns 0 and the descriptor
> > > of the connected Unix socket is removed from poll and it will
> > > not be present in poll() until the guest will initialize the driver
> > > and change the state of the serial to "guest connected".
> > > 
> > > In libvirt connect() to guest agent is performed on restart and
> > > is run under VM state lock. Connect() is blocking and can
> > > wait forever.
> > > In this case libvirt can not perform ANY operation on that VM.
> > > 
> > > The bug can be easily reproduced this way:
> > > 
> > > Terminal 1:
> > > qemu-system-x86_64 -m 512 -device pci-serial,chardev=serial1 -chardev 
> > > socket,id=serial1,path=/tmp/console.sock,server,nowait
> > > (virtio-serial and isa-serial also fit)
> > > 
> > > Terminal 2:
> > > minicom -D unix\#/tmp/console.sock
> > > (type something and pres enter)
> > > C-a x (to exit)
> > > 
> > > Do 3 times:
> > > minicom -D unix\#/tmp/console.sock
> > > C-a x
> > > 
> > > It needs 4 connections, because the first one is accepted by QEMU, then 
> > > two are queued by
> > > the kernel, and the 4th blocks.
> > > 
> > > The problem is that QEMU doesn't add a read watcher after succesful read
> > > until the guest device wants to acquire recieved data, so
> > > I propose to install a separate pullhup watcher regardless of
> > > whether the device waits for data or not. After closing the connection,
> > > the guest has a capability to read the data within timeout.
> > I don't understand the timeout part.
> The timeout is important, because of following situation:
> client sends to the guest big bunch of data and closes his end of socket.
> if we just destroy the connection on the qemu's side, the guest can't read
> remaining data in channel.

Why is that a problem that needs solving ?  IMHO if the clients wants any
kind of assurance that the guest has read all the data, it should keep the
socket open and wait for a reply from the guest agent. It is totally
reasonable for the request to be dropped/partially sent if the client closes
the socket without waiting. The guest should be robust to seeing such
partial messages.

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 :|