Re: [Qemu-devel] [PATCH V2] chardev: fix parallel device can't be reconnect

[Daniel P. Berrange]

On Tue, Jul 11, 2017 at 02:21:08PM +0200, Paolo Bonzini wrote:
> On 11/07/2017 11:15, Daniel P. Berrange wrote:
> > On Tue, Jul 11, 2017 at 10:30:14AM +0200, Paolo Bonzini wrote:
> >> On 11/07/2017 13:47, Peng Hao wrote:
> >>> Parallel device don't register be->chr_can_read function, but remote
> >>> disconnect event is handled in chr_read.So connected parallel device
> >>> can not detect remote disconnect event. The chardevs with 
> >>> chr_can_read=NULL
> >>> has the same problem.
> >>>
> >>> Signed-off-by: Peng Hao <address@hidden>
> >>> Reviewed-by: Wang Yechao <address@hidden>
> >>> ---
> >>>  chardev/char-socket.c  | 11 +++++++++++
> >>>  chardev/char.c         | 10 ++++++++++
> >>>  include/chardev/char.h |  9 +++++++++
> >>>  3 files changed, 30 insertions(+)
> >>>
> >>> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> >>> index ccc499c..aa44f8f 100644
> >>> --- a/chardev/char-socket.c
> >>> +++ b/chardev/char-socket.c
> >>> @@ -139,6 +139,9 @@ static int tcp_chr_read_poll(void *opaque)
> >>>          return 0;
> >>>      }
> >>>      s->max_size = qemu_chr_be_can_write(chr);
> >>> +    if (qemu_chr_null_be_can_read(chr)) {
> >>> +        return 1;
> >>> +    }
> >>>      return s->max_size;
> >>>  }
> >>>  
> >>> @@ -422,6 +425,14 @@ static gboolean tcp_chr_read(QIOChannel *chan, 
> >>> GIOCondition cond, void *opaque)
> >>>      uint8_t buf[CHR_READ_BUF_LEN];
> >>>      int len, size;
> >>>  
> >>> +    if (qemu_chr_null_be_can_read(chr)) {
> >>> +        size = tcp_chr_recv(chr, (void *)buf, CHR_READ_BUF_LEN);
> >>
> >> It would be better not to destroy data in the channel, because the
> >> device could set handlers later.
> >>
> >> Daniel, maybe QIOChannel could have a function to check for hung-up
> >> channels and return a bool?  For file descriptors it would poll the file
> >> descriptor and check for POLLHUP, while other channels would have a more
> >> or less obvious implementation.
> > 
> > IMHO the chardev code all needs to switch over to using even driven I/O
> > using the  qio_channel_add_watch() function.
> 
> This is a slightly different case; qio_channel_add_watch() is for the
> write side, while in this case we're concerned with the read side.
> 
> The read-side watch is managed automatically by common chardev code.
> I'll see if we can change that code to set up a G_IO_HUP watch in
> addition to G_IO_IN.

NB You don't ever need to use G_IO_HUP as an explicit input flag - poll()
will always report G_IO_HUP if you asked it to monitor for G_IO_IN.

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