[Qemu-devel] Re: virtio-serial semantics for binary data and guest agents

[Anthony Liguori <aliguori@linux.vnet.ibm.com>]

On 02/24/2011 06:48 AM, Amit Shah wrote:
> On (Wed) 23 Feb 2011 [08:31:52], Michael Roth wrote:
>    
>> On 02/22/2011 10:59 PM, Amit Shah wrote:
>>      
>>> On (Tue) 22 Feb 2011 [16:40:55], Michael Roth wrote:
>>>        
>>>> If something in the guest is attempting to read/write from the
>>>> virtio-serial device, and nothing is connected to virtio-serial's
>>>> host character device (say, a socket)
>>>>
>>>> 1. writes will block until something connect()s, at which point the
>>>> write will succeed
>>>>
>>>> 2. reads will always return 0 until something connect()s, at which
>>>> point the reads will block until there's data
>>>>
>>>> This makes it difficult (impossible?) to implement the notion of
>>>> connect/disconnect or open/close over virtio-serial without layering
>>>> another protocol on top using hackish things like length-encoded
>>>> payloads or sentinel values to determine the end of one
>>>> RPC/request/response/session and the start of the next.
>>>>
>>>> For instance, if the host side disconnects, then reconnects before
>>>> we read(), we may never get the read()=0, and our FD remains valid.
>>>> Whereas with a tcp/unix socket our FD is no longer valid, and the
>>>> read()=0 is an event we can check for at any point after the other
>>>> end does a close/disconnect.
>>>>          
>>> There's SIGIO support, so host connect-disconnect notifications can be
>>> caught via the signal.
>>>        
>> I recall looking into this at some point....but don't we get a SIGIO
>> for read/write-ability in general?
>>      
> I don't get you -- the virtio_console driver emits the SIGIO signal
> only when the host side connects or disconnects.  See
>    

Um, that's not the expected semantics of SIGIO.  SIGIO can be delivered 
for any number of reasons (including on a normal file descriptor) so if 
there's no way to poll for the specific event then the mechanism is 
inherently racy.

Regards,

Anthony Liguori

> http://www.linux-kvm.org/page/Virtio-serial_API
>
> So whenever you receive a SIGIO, poll() in the signal handler for all
> fds of interest and whichever has POLLIN set is writable.  Whichever
> has POLLHUP set is not.  If you maintain previous state of the fd
> (before signal), you can figure out if something happened on the host
> side.
>
>    
>> So you still need some way
>> differentiate, say, readability from a disconnect/EOF, and the
>> read()=0 that could determine this is still racing with host-side
>> reconnects.
>>      
>    
>>> Also, nonblocking reads/writes will return -EPIPE if the host-side
>>> connection is not up.
>>>        
>> But we still essentially need to poll() for a host-side disconnected
>> state, which is still racy since they may reconnect before we've
>> done a read/write that would've generated the -EPIPE. It seems like
>> what we really need is for the FD to be invalid from that point
>> forward.
>>      
> This would go against (or abuse) a chardev interface.  It would
> effectively treat a host-side port close as a hot-unplug event.
>
>    
>> Also, I focused more on the guest-side connect/disconnect detection,
>> but as Anthony mentioned I think the host side shares similar
>> limitations as well. AFAIK once we connect to the chardev that FD
>> remains valid until the connected process closes it, and so races
>> with the guest side on detecting connect/disconnect events in a
>> similar manner. For the host side it looks like virtio-console has
>> guest_close/guest_open callbacks already that we could potentially
>> use...seems like it's just a matter of tying them to the chardev...
>> basically having virtio-serial's guest_close() result in a close()
>> on the corresponding chardev connection's FD.
>>      
> Yes, this could be used.
>
> However, the problem with that will be that the chardev can't be
> opened again (AFAIR) and a new chardev will have to be used.
>
>
> So if this is done on both the sides, the race will be eliminated but
> the expectation that a chardev port is just a serial port will be
> broken and we'll try to bake in some connection layer on top of it.
> That wasn't the original idea.  We could extend this, but a better way
> to achieve this could be a library on either side to abstract these
> details off.
>
>                  Amit
>