| Message ID | 1499705121-71730-1-git-send-email-peng.hao2@zte.com.cn (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi On Mon, Jul 10, 2017 at 10:36 AM Peng Hao <peng.hao2@zte.com.cn> 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. > > What is it that you call a parallel device? you are modifying the socket code here. Could you describe a test setup? even better would be to write a test in test-char.c. > Signed-off-by: Peng Hao <peng.hao2@zte.com.cn> > Reviewed-by: Wang Yechao <wang.yechao255@zte.com.cn> > --- > chardev/char-socket.c | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/chardev/char-socket.c b/chardev/char-socket.c > index ccc499c..59509d4 100644 > --- a/chardev/char-socket.c > +++ b/chardev/char-socket.c > @@ -131,6 +131,14 @@ static int tcp_chr_write(Chardev *chr, const uint8_t > *buf, int len) > } > } > > +static gboolean is_parallel_device(Chardev *chr) > +{ > + if (chr && chr->label && strstr(chr->label, "charparallel")) { > + return TRUE; > What guarantees that a parallel device will have "charparallel" in its label? > + } > + return FALSE; > +} > + > static int tcp_chr_read_poll(void *opaque) > { > Chardev *chr = CHARDEV(opaque); > @@ -138,6 +146,8 @@ static int tcp_chr_read_poll(void *opaque) > if (!s->connected) { > return 0; > } > + if (is_parallel_device(chr)) { > + return 1; > That means 1 byte can be read, but s->max_size isn't updated, this will confuse tcp_chr_read(). > + } > s->max_size = qemu_chr_be_can_write(chr); > return s->max_size; > } > @@ -422,6 +432,15 @@ static gboolean tcp_chr_read(QIOChannel *chan, > GIOCondition cond, void *opaque) > uint8_t buf[CHR_READ_BUF_LEN]; > int len, size; > > + /* for parallel-device handle the socket close event here*/ > + if (!s->max_size && is_parallel_device(chr)) { > + size = tcp_chr_recv(chr, (void *)buf, CHR_READ_BUF_LEN); > + if (size == 0 || size == -1) { > + tcp_chr_disconnect(chr); > I don't understand why that condition wouldn't be reached by the following tcp_chr_recv() handling. Furthermore, you may read more that s->max_size, which will likely break qemu_chr_be_write() in various cases. > + } > + return TRUE; > + } > + > if (!s->connected || s->max_size <= 0) { > return TRUE; > } > -- > 1.8.3.1 > > > > -- Marc-André Lureau
On 10/07/2017 11:17, Marc-André Lureau 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. > > What is it that you call a parallel device? you are modifying the socket > code here. Could you describe a test setup? even better would be to > write a test in test-char.c. If I understand the patch correctly, the parallel device is the parallel port emulation. It's still not clear to me what scenario the patch is fixing, and the patch is wrong anyway: 1) because the backend is not supposed to know the details of the frontend that is connected to it; 2) because the character device's label is not a reliable indicator for the frontend type (there is no such indicator, because of point 1). Paolo
diff --git a/chardev/char-socket.c b/chardev/char-socket.c index ccc499c..59509d4 100644 --- a/chardev/char-socket.c +++ b/chardev/char-socket.c @@ -131,6 +131,14 @@ static int tcp_chr_write(Chardev *chr, const uint8_t *buf, int len) } } +static gboolean is_parallel_device(Chardev *chr) +{ + if (chr && chr->label && strstr(chr->label, "charparallel")) { + return TRUE; + } + return FALSE; +} + static int tcp_chr_read_poll(void *opaque) { Chardev *chr = CHARDEV(opaque); @@ -138,6 +146,8 @@ static int tcp_chr_read_poll(void *opaque) if (!s->connected) { return 0; } + if (is_parallel_device(chr)) { + return 1; + } s->max_size = qemu_chr_be_can_write(chr); return s->max_size; } @@ -422,6 +432,15 @@ static gboolean tcp_chr_read(QIOChannel *chan, GIOCondition cond, void *opaque) uint8_t buf[CHR_READ_BUF_LEN]; int len, size; + /* for parallel-device handle the socket close event here*/ + if (!s->max_size && is_parallel_device(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; }