From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([140.186.70.92]:59565)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <amit.shah@redhat.com>) id 1QXmRB-0004GI-0F
	for qemu-devel@nongnu.org; Fri, 17 Jun 2011 23:43:10 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <amit.shah@redhat.com>) id 1QXmR9-000586-L5
	for qemu-devel@nongnu.org; Fri, 17 Jun 2011 23:43:08 -0400
Received: from mx1.redhat.com ([209.132.183.28]:53037)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <amit.shah@redhat.com>) id 1QXmR9-000582-Al
	for qemu-devel@nongnu.org; Fri, 17 Jun 2011 23:43:07 -0400
Date: Sat, 18 Jun 2011 09:12:45 +0530
From: Amit Shah <amit.shah@redhat.com>
Message-ID: <20110618034245.GA26613@amit-x200.redhat.com>
References: <20110616133849.02fb4bea@doriath>
	<20110617064736.GA18513@amit-x200.redhat.com>
	<20110617101644.5e275e53@doriath>
	<20110617160926.GA25648@amit-x200.redhat.com>
	<20110617150811.156acbae@doriath>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20110617150811.156acbae@doriath>
Subject: Re: [Qemu-devel] [PATCH] virtio-serial: Fix segfault on guest boot
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Luiz Capitulino <lcapitulino@redhat.com>
Cc: qemu-devel <qemu-devel@nongnu.org>, Markus Armbruster <armbru@redhat.com>, mdroth@linux.vnet.ibm.com

On (Fri) 17 Jun 2011 [15:08:11], Luiz Capitulino wrote:

> > >          if (!cpkt.value) {
> > > -            error_report("virtio-serial-bus: Guest failure in adding device %s\n",
> > > -                         vser->bus.qbus.name);
> > > -            break;
> > > +            error_report("virtio-serial-bus: Guest failure in adding device %s\n", vser->bus.qbus.name);
> > > +            return;
> > 
> > The line split should remain -- else it goes beyond 80 chars.
> 
> It's already beyond 80 chars to me.

I prefer to not break strings that get printed out -- makes it easier
for greppers to find out the source of the string.

> > > @@ -346,8 +339,13 @@ static void handle_control_message(VirtIOSerial *vser, void *buf, size_t len)
> > >          QTAILQ_FOREACH(port, &vser->ports, next) {
> > >              send_control_event(port, VIRTIO_CONSOLE_PORT_ADD, 1);
> > >          }
> > > -        break;
> > > +        return;
> > > +    }
> > 
> > Makes me think of one case (totally unrelated to what you found)where
> > the guest can fool us: by sending multiple VIRTIO_CONSOLE_DEVICE_READY
> > messages.
> 
> It will be handled just fine, no?

We'll send out the VIRTIO_CONSOLE_PORT_ADD events for each port
(again).  That's the case now.  No idea how the code might change in
the future and we could end up doing something else in addition which
might be bad.  Anyway, all this is for a buggy or a bad guest.

		Amit