Re: [Qemu-devel] [PATCH] virtio-blk: Do not segfault fault if failed to initialize dataplane

[KONRAD Frédéric]

On 19/03/2013 11:33, Kevin Wolf wrote:
> Am 19.03.2013 um 09:37 hat KONRAD Frédéric geschrieben:
> > On 19/03/2013 09:27, Dunrong Huang wrote:
> > > $ ~/usr/bin/qemu-system-x86_64 -enable-kvm -m 1024 -drive 
> > > if=none,id=drive0,cache=none,aio=native,format=raw,file=/root/Image/centos-6.4.raw
> > >  -device virtio-blk-pci,drive=drive0,scsi=off,x-data-plane=on,config-wce=on # 
> > > make dataplane fail to initialize
> > > qemu-system-x86_64: -device 
> > > virtio-blk-pci,drive=drive0,scsi=off,x-data-plane=on,config-wce=on: device is 
> > > incompatible with x-data-plane, use config-wce=off
> > > *** glibc detected *** /root/usr/bin/qemu-system-x86_64: free(): invalid 
> > > pointer: 0x00007f001fef12f8 ***
> > > ======= Backtrace: =========
> > > /lib64/libc.so.6(+0x7d776)[0x7f00153a5776]
> > > /root/usr/bin/qemu-system-x86_64(+0x2c34ec)[0x7f001cf5b4ec]
> > > /root/usr/bin/qemu-system-x86_64(+0x342f9a)[0x7f001cfdaf9a]
> > > /root/usr/bin/qemu-system-x86_64(+0x33694e)[0x7f001cfce94e]
> > > ....................
> > >
> > >   (gdb) bt
> > >   #0  0x00007f3bf3a12015 in raise () from /lib64/libc.so.6
> > >   #1  0x00007f3bf3a1348b in abort () from /lib64/libc.so.6
> > >   #2  0x00007f3bf3a51a4e in __libc_message () from /lib64/libc.so.6
> > >   #3  0x00007f3bf3a57776 in malloc_printerr () from /lib64/libc.so.6
> > >   #4  0x00007f3bfb60d4ec in free_and_trace (mem=0x7f3bfe0129f8) at vl.c:2786
> > >   #5  0x00007f3bfb68cf9a in virtio_cleanup (vdev=0x7f3bfe0129f8) at 
> > > /root/Develop/QEMU/qemu/hw/virtio.c:900
> > >   #6  0x00007f3bfb68094e in virtio_blk_device_init (vdev=0x7f3bfe0129f8) at 
> > > /root/Develop/QEMU/qemu/hw/virtio-blk.c:666
> > >   #7  0x00007f3bfb68dadf in virtio_device_init (qdev=0x7f3bfe0129f8) at 
> > > /root/Develop/QEMU/qemu/hw/virtio.c:1092
> > >   #8  0x00007f3bfb50da46 in device_realize (dev=0x7f3bfe0129f8, 
> > > err=0x7fff479c9258) at hw/qdev.c:176
> > > .............................
> > >
> > > In virtio_blk_device_init(), the memory which vdev point to is a static
> > > member of "struct VirtIOBlkPCI", not heap memory, and it does not
> > > get freed. So we shoule use virtio_common_cleanup() to clean this VirtIODevice
> > > rather than virtio_cleanup(), which attempts to free the vdev.
> > >
> > > This error was introduced by commit 05ff686536f408ba6e8426b1b54d25bd3379fda2
> > > recently.
> > >
> > > Signed-off-by: Dunrong Huang <address@hidden>
> > Reviewed-by: KONRAD Frederic <address@hidden>
> >
> > Oops sorry for that :/
> So virtio_init() has to be paired with virtio_common_cleanup(), and
> virtio_common_init() with virtio_cleanup()? Confusing...

True, I agree that's confusing...

Note that this will disappear when all device will be re-factored and 
was transparent with the big patch-set.

(I think that the original code had virtio_cleanup and virtio_common_init.)

Fred
> Anyway, the patch looks correct. Thanks, applied to the block branch.
>
> Kevin