From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:40596) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UHwas-0000ae-Ft for qemu-devel@nongnu.org; Tue, 19 Mar 2013 09:28:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UHwan-0000c3-AO for qemu-devel@nongnu.org; Tue, 19 Mar 2013 09:28:46 -0400 Received: from greensocs.com ([87.106.252.221]:60040 helo=s15328186.onlinehome-server.info) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UHwam-0000bt-W0 for qemu-devel@nongnu.org; Tue, 19 Mar 2013 09:28:41 -0400 Message-ID: <51486803.2040908@greensocs.com> Date: Tue, 19 Mar 2013 14:28:35 +0100 From: =?ISO-8859-1?Q?KONRAD_Fr=E9d=E9ric?= MIME-Version: 1.0 References: <1363681649-23162-1-git-send-email-huangdr@cloud-times.com> <514823E3.3000102@greensocs.com> <20130319103355.GB2418@dhcp-200-207.str.redhat.com> In-Reply-To: <20130319103355.GB2418@dhcp-200-207.str.redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] virtio-blk: Do not segfault fault if failed to initialize dataplane List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: Anthony Liguori , Dunrong Huang , Stefan Hajnoczi , qemu-devel On 19/03/2013 11:33, Kevin Wolf wrote: > Am 19.03.2013 um 09:37 hat KONRAD Fr=E9d=E9ric geschrieben: >> On 19/03/2013 09:27, Dunrong Huang wrote: >>> $ ~/usr/bin/qemu-system-x86_64 -enable-kvm -m 1024 -drive if=3Dnone,i= d=3Ddrive0,cache=3Dnone,aio=3Dnative,format=3Draw,file=3D/root/Image/cent= os-6.4.raw -device virtio-blk-pci,drive=3Ddrive0,scsi=3Doff,x-data-plane=3D= on,config-wce=3Don # make dataplane fail to initialize >>> qemu-system-x86_64: -device virtio-blk-pci,drive=3Ddrive0,scsi=3Doff,= x-data-plane=3Don,config-wce=3Don: device is incompatible with x-data-pla= ne, use config-wce=3Doff >>> *** glibc detected *** /root/usr/bin/qemu-system-x86_64: free(): inva= lid pointer: 0x00007f001fef12f8 *** >>> =3D=3D=3D=3D=3D=3D=3D Backtrace: =3D=3D=3D=3D=3D=3D=3D=3D=3D >>> /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=3D0x7f3bfe0129f8) at = vl.c:2786 >>> #5 0x00007f3bfb68cf9a in virtio_cleanup (vdev=3D0x7f3bfe0129f8) at= /root/Develop/QEMU/qemu/hw/virtio.c:900 >>> #6 0x00007f3bfb68094e in virtio_blk_device_init (vdev=3D0x7f3bfe01= 29f8) at /root/Develop/QEMU/qemu/hw/virtio-blk.c:666 >>> #7 0x00007f3bfb68dadf in virtio_device_init (qdev=3D0x7f3bfe0129f8= ) at /root/Develop/QEMU/qemu/hw/virtio.c:1092 >>> #8 0x00007f3bfb50da46 in device_realize (dev=3D0x7f3bfe0129f8, err= =3D0x7fff479c9258) at hw/qdev.c:176 >>> ............................. >>> >>> In virtio_blk_device_init(), the memory which vdev point to is a stat= ic >>> member of "struct VirtIOBlkPCI", not heap memory, and it does not >>> get freed. So we shoule use virtio_common_cleanup() to clean this Vir= tIODevice >>> rather than virtio_cleanup(), which attempts to free the vdev. >>> >>> This error was introduced by commit 05ff686536f408ba6e8426b1b54d25bd3= 379fda2 >>> recently. >>> >>> Signed-off-by: Dunrong Huang >> Reviewed-by: KONRAD Frederic >> >> 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=20 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