From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=60274 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Q2OI7-0005eq-UA for qemu-devel@nongnu.org; Wed, 23 Mar 2011 09:40:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Q2OI0-0005Um-SM for qemu-devel@nongnu.org; Wed, 23 Mar 2011 09:39:58 -0400 Received: from mx1.redhat.com ([209.132.183.28]:62657) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Q2OI0-0005UU-Gq for qemu-devel@nongnu.org; Wed, 23 Mar 2011 09:39:56 -0400 Date: Wed, 23 Mar 2011 19:09:51 +0530 From: Amit Shah Message-ID: <20110323133951.GA25904@amit-x200.redhat.com> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: [Qemu-devel] Re: [PATCH 5/7] virtio-serial: Don't clear ->have_data() pointer after unplug List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Juan Quintela Cc: qemu list On (Wed) 23 Mar 2011 [14:33:25], Juan Quintela wrote: > Amit Shah wrote: > > After a port unplug operation, the port->info->have_data() pointer was > > set to NULL. The problem is, the ->info struct is shared by all ports, > > effectively disabling writes to other ports. > > > > Reported-by: juzhang > > Signed-off-by: Amit Shah > > --- > > hw/virtio-console.c | 1 - > > 1 files changed, 0 insertions(+), 1 deletions(-) > > > > diff --git a/hw/virtio-console.c b/hw/virtio-console.c > > index 4440784..be59558 100644 > > --- a/hw/virtio-console.c > > +++ b/hw/virtio-console.c > > @@ -82,7 +82,6 @@ static int virtconsole_exitfn(VirtIOSerialPort *port) > > VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port); > > > > if (vcon->chr) { > > - port->info->have_data = NULL; > > qemu_chr_close(vcon->chr); > > } > > Discussed with Amit over irc, I think that we are missing setup of > have_data for non console devices, but that is a different bug that the > one being fixed here. Actually other virtio_serial devices will provide their own have_data, like spice did earlier (now it's a chardev, so it uses this code path). I think the bug is that we should set have_data regardless of a chardev backend and call qemu_chr_write() in have_data only if a chardev exists. Amit