Re: [Qemu-devel] [PATCH] virtio-serial: Fix segfault on guest boot

[Blue Swirl]

On Sat, Jun 18, 2011 at 6:42 AM, Amit Shah <address@hidden> wrote:
> 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.

Please read CODING_STYLE and use scripts/checkpatch.pl before
submitting patches.

>> > > @@ -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
>
>