From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33417) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vqh9w-0006Pn-RH for qemu-devel@nongnu.org; Wed, 11 Dec 2013 05:36:58 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Vqh9p-0005yW-Mr for qemu-devel@nongnu.org; Wed, 11 Dec 2013 05:36:52 -0500 Sender: Paolo Bonzini Message-ID: <52A84038.8000608@redhat.com> Date: Wed, 11 Dec 2013 11:36:40 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <1386755260-13781-1-git-send-email-ghammer@redhat.com> In-Reply-To: <1386755260-13781-1-git-send-email-ghammer@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2] char: restore read callback on a reattached (hotplug) chardev List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gal Hammer Cc: Amit Shah , qemu-devel@nongnu.org, Anthony Liguori , qemu-stable@nongnu.org Il 11/12/2013 10:47, Gal Hammer ha scritto: > Fix a bug that was introduced in commit 386a5a1e. A removal of a device > set the chr handlers to NULL. However when the device is plugged back, > its read callback is not restored so data can't be transfter from the > host to the guest via the virtio-serial port. > > https://bugzilla.redhat.com/show_bug.cgi?id=1027181 > > V2: - do not call chr_update_read_handler on device removal. > - add asserts to verify chr_update_read_handler is not called > with an assigned fd_in_tag to prevent fd leaks. > - update fd and udp backends' chr_update_read_handler function > so it won't remove fd_in to prevent a double release. > > Signed-off-by: Gal Hammer > --- > qemu-char.c | 17 +++++++++++++++-- > 1 file changed, 15 insertions(+), 2 deletions(-) > > diff --git a/qemu-char.c b/qemu-char.c > index e00f84c..69649f0 100644 > --- a/qemu-char.c > +++ b/qemu-char.c > @@ -213,7 +213,7 @@ void qemu_chr_add_handlers(CharDriverState *s, > s->chr_read = fd_read; > s->chr_event = fd_event; > s->handler_opaque = opaque; > - if (s->chr_update_read_handler) > + if (fe_open && s->chr_update_read_handler) > s->chr_update_read_handler(s); > > if (!s->explicit_fe_open) { > @@ -870,6 +870,7 @@ static void fd_chr_update_read_handler(CharDriverState *chr) > { > FDCharDriver *s = chr->opaque; > > + assert(!chr->fd_in_tag); > remove_fd_in_watch(chr); > if (s->fd_in) { > chr->fd_in_tag = io_add_watch_poll(s->fd_in, fd_chr_read_poll, > @@ -2228,7 +2229,7 @@ static void udp_chr_update_read_handler(CharDriverState *chr) > { > NetCharDriver *s = chr->opaque; > > - remove_fd_in_watch(chr); > + assert(!chr->fd_in_tag); > if (s->chan) { > chr->fd_in_tag = io_add_watch_poll(s->chan, udp_chr_read_poll, > udp_chr_read, chr); > @@ -2510,6 +2511,17 @@ static void tcp_chr_connect(void *opaque) > qemu_chr_be_generic_open(chr); > } > > +static void tcp_chr_update_read_handler(CharDriverState *chr) > +{ > + TCPCharDriver *s = chr->opaque; > + > + assert(!chr->fd_in_tag); > + if (s->chan && !chr->fd_in_tag) { > + chr->fd_in_tag = io_add_watch_poll(s->chan, tcp_chr_read_poll, > + tcp_chr_read, chr); > + } > +} > + > #define IACSET(x,a,b,c) x[0] = a; x[1] = b; x[2] = c; > static void tcp_chr_telnet_init(int fd) > { > @@ -2665,6 +2677,7 @@ static CharDriverState *qemu_chr_open_socket_fd(int fd, bool do_nodelay, > chr->get_msgfd = tcp_get_msgfd; > chr->chr_add_client = tcp_chr_add_client; > chr->chr_add_watch = tcp_chr_add_watch; > + chr->chr_update_read_handler = tcp_chr_update_read_handler; > /* be isn't opened until we get a connection */ > chr->explicit_be_open = true; > > Cc: qemu-stable@nongnu.org