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 <peng.h...@zte.com.cn> > >>> Reviewed-by: Wang Yechao <wang.yechao...@zte.com.cn> > >>> --- > >>> 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 :|