[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] virtio-serial: Fix segfault on guest boot
From: |
Luiz Capitulino |
Subject: |
Re: [Qemu-devel] [PATCH] virtio-serial: Fix segfault on guest boot |
Date: |
Fri, 17 Jun 2011 10:16:44 -0300 |
On Fri, 17 Jun 2011 12:17:36 +0530
Amit Shah <address@hidden> wrote:
> On (Thu) 16 Jun 2011 [13:38:49], Luiz Capitulino wrote:
> > If I start qemu with:
> >
> > # qemu -hda disks/test.img -enable-kvm -m 1G -snapshot \
> > -device virtio-serial \
> > -chardev socket,host=localhost,port=1234,server,nowait,id=foo \
> > -device virtserialport,chardev=foo,name=org.qemu.guest_agent
> >
> > I get a segfault when booting a Fedora 14 guest. The backtrace says:
> >
> > Program terminated with signal 11, Segmentation fault.
> > #0 0x0000000000420850 in handle_control_message (vser=0x3732bd0,
> > buf=0x2c173e0, len=8) at
> > /home/lcapitulino/src/qmp-unstable/hw/virtio-serial-bus.c:335
> > 335 info = DO_UPCAST(VirtIOSerialPortInfo, qdev, port->dev.info);
>
> Strange, I've not seen it so far in my testing (neither in the daily
> test runs of the virtio-serial testsuite).
>
> > I've also bisected this and git points out to commit:
> >
> > commit a15bb0d6a981de749452a5180fc8084d625671da
> > Author: Markus Armbruster <address@hidden>
> > Date: Wed May 25 14:21:13 2011 +0200
> >
> > virtio-serial: Drop redundant VirtIOSerialPort member info
> >
> > I think what's happening is that the device is not initialized on a
> > VIRTIO_CONSOLE_DEVICE_READY event. Moving the DO_UPCAST() call to
> > the other events fixes the problem to me.
> >
> > Signed-off-by: Luiz Capitulino <address@hidden>
> > ---
> > hw/virtio-serial-bus.c | 4 ++--
> > 1 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
> > index 9a12104..579f676 100644
> > --- a/hw/virtio-serial-bus.c
> > +++ b/hw/virtio-serial-bus.c
> > @@ -332,8 +332,6 @@ static void handle_control_message(VirtIOSerial *vser,
> > void *buf, size_t len)
> > if (!port && cpkt.event != VIRTIO_CONSOLE_DEVICE_READY)
> > return;
> >
> > - info = DO_UPCAST(VirtIOSerialPortInfo, qdev, port->dev.info);
> > -
>
> Ah - this missed the !port check. It should be possible to do this in
> a 'if (port)' block instead of replicating in the individual case
> statements.
>
> Thanks for the debugging and patch; please update with the above and
> I'll apply it to the virtio-serial tree.
What about moving the VIRTIO_CONSOLE_DEVICE_READY handling out of the
switch, like the patch below? This way the function is divided in a way
that related events are handled together.
I'll implement your first suggestion if you don't like this...
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 579f676..5f96245 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -325,19 +325,12 @@ static void handle_control_message(VirtIOSerial *vser,
void *buf, size_t len)
return;
}
- cpkt.event = lduw_p(&gcpkt->event);
cpkt.value = lduw_p(&gcpkt->value);
-
- port = find_port_by_id(vser, ldl_p(&gcpkt->id));
- if (!port && cpkt.event != VIRTIO_CONSOLE_DEVICE_READY)
- return;
-
- switch(cpkt.event) {
- case VIRTIO_CONSOLE_DEVICE_READY:
+ cpkt.event = lduw_p(&gcpkt->event);
+ if (cpkt.event == VIRTIO_CONSOLE_DEVICE_READY) {
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 device is up, we can now tell the device about all the
@@ -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;
+ }
+ port = find_port_by_id(vser, ldl_p(&gcpkt->id));
+ assert(port != NULL);
+
+ switch(cpkt.event) {
case VIRTIO_CONSOLE_PORT_READY:
if (!cpkt.value) {
error_report("virtio-serial-bus: Guest failure in adding port %u
for device %s\n",
- [Qemu-devel] [PATCH] virtio-serial: Fix segfault on guest boot, Luiz Capitulino, 2011/06/16
- Re: [Qemu-devel] [PATCH] virtio-serial: Fix segfault on guest boot, Amit Shah, 2011/06/17
- Re: [Qemu-devel] [PATCH] virtio-serial: Fix segfault on guest boot, Markus Armbruster, 2011/06/17
- Re: [Qemu-devel] [PATCH] virtio-serial: Fix segfault on guest boot,
Luiz Capitulino <=
- Re: [Qemu-devel] [PATCH] virtio-serial: Fix segfault on guest boot, Luiz Capitulino, 2011/06/17
- Re: [Qemu-devel] [PATCH] virtio-serial: Fix segfault on guest boot, Amit Shah, 2011/06/17
- Re: [Qemu-devel] [PATCH] virtio-serial: Fix segfault on guest boot, Luiz Capitulino, 2011/06/17
- Re: [Qemu-devel] [PATCH] virtio-serial: Fix segfault on guest boot, Amit Shah, 2011/06/17
- Re: [Qemu-devel] [PATCH] virtio-serial: Fix segfault on guest boot, Blue Swirl, 2011/06/18
- Re: [Qemu-devel] [PATCH] virtio-serial: Fix segfault on guest boot, Amit Shah, 2011/06/22
- Re: [Qemu-devel] [PATCH] virtio-serial: Fix segfault on guest boot, Blue Swirl, 2011/06/26
- Re: [Qemu-devel] 80-column rule and breaking output statements, Amit Shah, 2011/06/27