[Qemu-devel] [PATCH] char: restore read callback on a reattached (hotplug) chardev

[Qemu-devel] [PATCH] char: restore read callback on a reattached (hotplug) chardev

[Gal Hammer]

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

Signed-off-by: Gal Hammer <ghammer@redhat.com>
---
 qemu-char.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/qemu-char.c b/qemu-char.c
index e00f84c..44499e4 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2510,6 +2510,16 @@ 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;
+
+    if (s->chan && !chr->avail_connections) {
+        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 +2675,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;
 
-- 
1.8.1.4

[Qemu-devel] [PATCH] char: restore read callback on a reattached (hotplug) chardev

[Gal Hammer]

Hi,

Trying again.

Thanks,

     Gal.

-------- Original Message --------
Subject: [Qemu-devel] [PATCH] char: restore read callback on a 
reattached (hotplug) chardev
Date: Mon, 25 Nov 2013 10:07:40 +0200
From: Gal Hammer <ghammer@redhat.com>
To: qemu-devel@nongnu.org
CC: Gal Hammer <ghammer@redhat.com>

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

Signed-off-by: Gal Hammer <ghammer@redhat.com>
---
  qemu-char.c | 11 +++++++++++
  1 file changed, 11 insertions(+)

diff --git a/qemu-char.c b/qemu-char.c
index e00f84c..44499e4 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2510,6 +2510,16 @@ 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;
+
+    if (s->chan && !chr->avail_connections) {
+        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 +2675,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;

-- 
1.8.1.4

Re: [Qemu-devel] [PATCH] char: restore read callback on a reattached (hotplug) chardev

[Amit Shah]

Hi,

On (Mon) 25 Nov 2013 [10:07:40], Gal Hammer wrote:
> 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.

Yes, good catch.

That commit removes the callback for all backend types, but this
commit only takes care of tcp.  Can you check the others too?  There
are a couple of backends which even remove their callback in the
update_read_handler() function, which may result in a double-free.

Can you look at that too?

Thanks,

		Amit