[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH V2] chardev: fix parallel device can't be reconn
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH V2] chardev: fix parallel device can't be reconnect |
Date: |
Tue, 11 Jul 2017 10:30:14 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.0 |
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.
> + 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
>