* [Qemu-devel] [PATCH V2] chardev: fix parallel device can't be reconnect
@ 2017-07-11 11:47 Peng Hao
2017-07-11 8:30 ` Paolo Bonzini
0 siblings, 1 reply; 7+ messages in thread
From: Peng Hao @ 2017-07-11 11:47 UTC (permalink / raw)
To: pbonzini, marcandre.lureau; +Cc: qemu-devel, wang.yechao255, Peng Hao
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.hao2@zte.com.cn>
Reviewed-by: Wang Yechao <wang.yechao255@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);
+ if (size == 0 || size == -1) {
+ tcp_chr_disconnect(chr);
+ }
+ return TRUE;
+ }
+
if (!s->connected || s->max_size <= 0) {
return TRUE;
}
diff --git a/chardev/char.c b/chardev/char.c
index 2b679a2..7f7f486 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -148,6 +148,16 @@ int qemu_chr_write(Chardev *s, const uint8_t *buf, int len, bool write_all)
return offset;
}
+int qemu_chr_null_be_can_read(Chardev *s)
+{
+ CharBackend *be = s->be;
+
+ if (!be || !be->chr_can_read) {
+ return 1;
+ }
+ return 0;
+}
+
int qemu_chr_be_can_write(Chardev *s)
{
CharBackend *be = s->be;
diff --git a/include/chardev/char.h b/include/chardev/char.h
index 8a9ade4..8dd28a8 100644
--- a/include/chardev/char.h
+++ b/include/chardev/char.h
@@ -114,6 +114,15 @@ void qemu_chr_cleanup(void);
Chardev *qemu_chr_new_noreplay(const char *label, const char *filename);
/**
+ * @qemu_chr_null_be_can_read:
+ *
+ * Check if Chardev's chr_can_read is registered.
+ *
+ * Returns: 1 if Chardev's chr_can_read is null.
+ */
+int qemu_chr_null_be_can_read(Chardev *s);
+
+/**
* @qemu_chr_be_can_write:
*
* Determine how much data the front end can currently accept. This function
--
1.8.3.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [Qemu-devel] [PATCH V2] chardev: fix parallel device can't be reconnect
2017-07-11 11:47 [Qemu-devel] [PATCH V2] chardev: fix parallel device can't be reconnect Peng Hao
@ 2017-07-11 8:30 ` Paolo Bonzini
2017-07-11 9:15 ` Daniel P. Berrange
0 siblings, 1 reply; 7+ messages in thread
From: Paolo Bonzini @ 2017-07-11 8:30 UTC (permalink / raw)
To: Peng Hao, marcandre.lureau; +Cc: qemu-devel, wang.yechao255, Daniel Berrange
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.hao2@zte.com.cn>
> Reviewed-by: Wang Yechao <wang.yechao255@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.
> + if (size == 0 || size == -1) {
> + tcp_chr_disconnect(chr);
> + }
> + return TRUE;
> + }
> +
> if (!s->connected || s->max_size <= 0) {
> return TRUE;
> }
> diff --git a/chardev/char.c b/chardev/char.c
> index 2b679a2..7f7f486 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -148,6 +148,16 @@ int qemu_chr_write(Chardev *s, const uint8_t *buf, int len, bool write_all)
> return offset;
> }
>
> +int qemu_chr_null_be_can_read(Chardev *s)
> +{
> + CharBackend *be = s->be;
> +
> + if (!be || !be->chr_can_read) {
I think it's really chr_read that you want to consider here.
Thanks,
Paolo
> + return 1;
> + }
> + return 0;
> +}
> +
> int qemu_chr_be_can_write(Chardev *s)
> {
> CharBackend *be = s->be;
> diff --git a/include/chardev/char.h b/include/chardev/char.h
> index 8a9ade4..8dd28a8 100644
> --- a/include/chardev/char.h
> +++ b/include/chardev/char.h
> @@ -114,6 +114,15 @@ void qemu_chr_cleanup(void);
> Chardev *qemu_chr_new_noreplay(const char *label, const char *filename);
>
> /**
> + * @qemu_chr_null_be_can_read:
> + *
> + * Check if Chardev's chr_can_read is registered.
> + *
> + * Returns: 1 if Chardev's chr_can_read is null.
> + */
> +int qemu_chr_null_be_can_read(Chardev *s);
> +
> +/**
> * @qemu_chr_be_can_write:
> *
> * Determine how much data the front end can currently accept. This function
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Qemu-devel] [PATCH V2] chardev: fix parallel device can't be reconnect
2017-07-11 8:30 ` Paolo Bonzini
@ 2017-07-11 9:15 ` Daniel P. Berrange
2017-07-11 12:21 ` Paolo Bonzini
0 siblings, 1 reply; 7+ messages in thread
From: Daniel P. Berrange @ 2017-07-11 9:15 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Peng Hao, marcandre.lureau, qemu-devel, wang.yechao255
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.hao2@zte.com.cn>
> > Reviewed-by: Wang Yechao <wang.yechao255@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 needed to properly
handle non-blocking I/O, to replace the workaround done in
commit 6ab3fc32ea640026726bc5f9f4db622d0954fb8a
Author: Daniel P. Berrange <berrange@redhat.com>
Date: Tue Sep 6 14:56:04 2016 +0100
hw: replace most use of qemu_chr_fe_write with qemu_chr_fe_write_all
Some places in the chardev code in fact alreadying use watches for a few
actions, but then use sync i/o for other stuff. So using events will give
a consistent model. Once you're doing proper event driven I/O, then
detecting hangup is also trivial.
IOW, I don't think we need add anything to QIOChannel - just continue to
cleanup the chardev backends to better use the functionality that already
exists.
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 :|
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Qemu-devel] [PATCH V2] chardev: fix parallel device can't be reconnect
2017-07-11 9:15 ` Daniel P. Berrange
@ 2017-07-11 12:21 ` Paolo Bonzini
2017-07-11 12:36 ` Marc-André Lureau
2017-07-11 12:37 ` Daniel P. Berrange
0 siblings, 2 replies; 7+ messages in thread
From: Paolo Bonzini @ 2017-07-11 12:21 UTC (permalink / raw)
To: Daniel P. Berrange; +Cc: Peng Hao, marcandre.lureau, qemu-devel, wang.yechao255
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.hao2@zte.com.cn>
>>> Reviewed-by: Wang Yechao <wang.yechao255@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.
Paolo
> commit 6ab3fc32ea640026726bc5f9f4db622d0954fb8a
> Author: Daniel P. Berrange <berrange@redhat.com>
> Date: Tue Sep 6 14:56:04 2016 +0100
>
> hw: replace most use of qemu_chr_fe_write with qemu_chr_fe_write_all
>
> Some places in the chardev code in fact alreadying use watches for a few
> actions, but then use sync i/o for other stuff. So using events will give
> a consistent model. Once you're doing proper event driven I/O, then
> detecting hangup is also trivial.
>
> IOW, I don't think we need add anything to QIOChannel - just continue to
> cleanup the chardev backends to better use the functionality that already
> exists.
>
> Regards,
> Daniel
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Qemu-devel] [PATCH V2] chardev: fix parallel device can't be reconnect
2017-07-11 12:21 ` Paolo Bonzini
@ 2017-07-11 12:36 ` Marc-André Lureau
2017-07-11 12:37 ` Daniel P. Berrange
1 sibling, 0 replies; 7+ messages in thread
From: Marc-André Lureau @ 2017-07-11 12:36 UTC (permalink / raw)
To: Paolo Bonzini, Daniel P. Berrange; +Cc: Peng Hao, wang.yechao255, qemu-devel
Hi
On Tue, Jul 11, 2017 at 2:21 PM Paolo Bonzini <pbonzini@redhat.com> 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.hao2@zte.com.cn>
> >>> Reviewed-by: Wang Yechao <wang.yechao255@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.
>
fwiw, we have some code handling HUP in interesting ways, see
a6553598be42e3be899acdb153fd615cd6c3eab8
--
Marc-André Lureau
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Qemu-devel] [PATCH V2] chardev: fix parallel device can't be reconnect
2017-07-11 12:21 ` Paolo Bonzini
2017-07-11 12:36 ` Marc-André Lureau
@ 2017-07-11 12:37 ` Daniel P. Berrange
2017-07-11 12:40 ` Paolo Bonzini
1 sibling, 1 reply; 7+ messages in thread
From: Daniel P. Berrange @ 2017-07-11 12:37 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Peng Hao, marcandre.lureau, qemu-devel, wang.yechao255
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.hao2@zte.com.cn>
> >>> Reviewed-by: Wang Yechao <wang.yechao255@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 :|
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Qemu-devel] [PATCH V2] chardev: fix parallel device can't be reconnect
2017-07-11 12:37 ` Daniel P. Berrange
@ 2017-07-11 12:40 ` Paolo Bonzini
0 siblings, 0 replies; 7+ messages in thread
From: Paolo Bonzini @ 2017-07-11 12:40 UTC (permalink / raw)
To: Daniel P. Berrange; +Cc: Peng Hao, marcandre.lureau, qemu-devel, wang.yechao255
On 11/07/2017 14:37, Daniel P. Berrange wrote:
> 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.hao2@zte.com.cn>
>>>>> Reviewed-by: Wang Yechao <wang.yechao255@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.
Right, but in this case we don't want to monitor G_IO_IN, because there
is no listener. The parallel port is a special case but actually the
same would happen for any other device whose can_read callback can
return zero.
Some Unix variants require you to specify G_IO_HUP in the events
(FreeBSD), others don't (Linux).
Paolo
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-07-11 12:40 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-07-11 11:47 [Qemu-devel] [PATCH V2] chardev: fix parallel device can't be reconnect Peng Hao
2017-07-11 8:30 ` Paolo Bonzini
2017-07-11 9:15 ` Daniel P. Berrange
2017-07-11 12:21 ` Paolo Bonzini
2017-07-11 12:36 ` Marc-André Lureau
2017-07-11 12:37 ` Daniel P. Berrange
2017-07-11 12:40 ` Paolo Bonzini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).