Re: [Qemu-devel] [PATCH v8 00/10] virtio-blk refactoring.

[Cornelia Huck]

On Fri, 15 Mar 2013 10:19:50 +0100
KONRAD Frédéric <address@hidden> wrote:

> On 15/03/2013 10:16, address@hidden wrote:
> > From: KONRAD Frederic <address@hidden>
> >
> > This is the next part of virtio-refactoring.
> >
> > Basically it creates virtio-blk device which extends virtio-device.
> > Then a virtio-blk can be connected on a virtio-bus.
> > virtio-blk-pci, virtio-blk-s390x, virtio-blk-ccw are created too, they 
> > extend
> > respectively virtio-pci, virtio-s390-device, virtio-ccw-device and have a
> > virtio-blk.
> >
> > You can checkout my branch here:
> >
> > git://project.greensocs.com/qemu-virtio.git virtio-blk-v8
> >
> > I made basic tests (with linux guests) on:
> >   * qemu-system-i386
> >   * qemu-system-s390x
> >
> > Cornelia made virtio-ccw test, and Stefan tried dataplane.
> >
> > Changes v7 -> v8:
> >      * Fix the allow_hotplug assertion spotted by Anthony.
> Cornelia, if you have time can you test virtio-blk hotplug with that change?

Sadly, no joy :(

(qemu) device_add virtio-blk-ccw,drive=drive-virtio-disk1,id=xxx
<device is added and looks sane>
(qemu) device_del xxx
*** glibc detected *** 
/data/git/yyy/qemu/build/s390x-softmmu/qemu-system-s390x: double free or 
corruption (fasttop): 0x0000000080cb8e30 ***

gdb says:

<snip>
#15 0x000003fffdb9cbee in __libc_message () from /lib64/libc.so.6
#16 0x000003fffdba5146 in _int_free () from /lib64/libc.so.6
#17 0x000000008019fa3e in free_and_trace (mem=0x80cb8e30)
    at /data/git/yyy/qemu/vl.c:2786
#18 0x000003fffded16de in g_free () from /lib64/libglib-2.0.so.0
#19 0x000000008019a406 in qemu_del_vm_change_state_handler (e=0x80cb8e30)
    at /data/git/yyy/qemu/vl.c:1691
#20 0x000000008020fd38 in virtio_common_cleanup (vdev=0x80cbb818)
    at /data/git/yyy/qemu/hw/virtio.c:892
#21 0x00000000801fe888 in virtio_blk_device_exit (dev=0x80cbb818)
    at /data/git/yyy/qemu/hw/virtio-blk.c:694
#22 0x00000000800bb50a in device_unparent (obj=0x80cbb818)
    at /data/git/yyy/qemu/hw/qdev.c:774
#23 0x0000000080146956 in object_unparent (obj=0x80cbb818)
    at /data/git/yyy/qemu/qom/object.c:370
#24 0x00000000800b92aa in qdev_free (dev=0x80cbb818)
    at /data/git/yyy/qemu/hw/qdev.c:271
#25 0x00000000800e751c in virtio_bus_destroy_device (bus=0x80cbb740)
    at /data/git/yyy/qemu/hw/virtio-bus.c:97
#26 0x00000000801f34f6 in virtio_ccw_blk_exit (ccw_dev=0x80cb9660)
    at /data/git/yyy/qemu/hw/s390x/virtio-ccw.c:588
#27 0x00000000801f41c8 in virtio_ccw_busdev_exit (dev=0x80cb9660)
    at /data/git/yyy/qemu/hw/s390x/virtio-ccw.c:858
#28 0x00000000800bb50a in device_unparent (obj=0x80cb9660)
    at /data/git/yyy/qemu/hw/qdev.c:774
#29 0x0000000080146956 in object_unparent (obj=0x80cb9660)
    at /data/git/yyy/qemu/qom/object.c:370
#30 0x00000000800b92aa in qdev_free (dev=0x80cb9660)
    at /data/git/yyy/qemu/hw/qdev.c:271
#31 0x00000000801f42ec in virtio_ccw_busdev_unplug (dev=0x80cb9660)
    at /data/git/yyy/qemu/hw/s390x/virtio-ccw.c:877
#32 0x00000000800b8e8e in qdev_unplug (dev=0x80cb9660, errp=0x3ffffffd388)
    at /data/git/yyy/qemu/hw/qdev.c:204
#33 0x0000000080126610 in qmp_device_del (id=0x80cb8c80 "xxx", errp=
    0x3ffffffd388) at /data/git/yyy/qemu/qdev-monitor.c:626
#34 0x000000008008f8c2 in hmp_device_del (mon=0x80c5ff60, qdict=0x80cb6550)
    at /data/git/yyy/qemu/hmp.c:1175
#35 0x000000008022c73c in handle_user_command (mon=0x80c5ff60, cmdline=
    0x80c603d0 "device_del xxx") at /data/git/yyy/qemu/monitor.c:3966
#36 0x000000008022e7b4 in monitor_command_cb (mon=0x80c5ff60, cmdline=
    0x80c603d0 "device_del xxx", opaque=0x0)
    at /data/git/yyy/qemu/monitor.c:4582
<snip>

That's with your tree +
http://marc.info/?l=qemu-devel&m=136334147200729&w=2 +
http://marc.info/?l=qemu-devel&m=136309339015104&w=2

master + the same two patches works fine.


> 
> Thanks,
> Fred
> >      * Attached the make virtio device's structures public (v4).
> > Changes v6 -> v7:
> >      * Fix the DEFINE_VIRTIO_BLK_PROPERTIES macro issue spotted by Peter.
> >
> > Thanks.
> >
> > Fred
> >
> > KONRAD Frederic (10):
> >    virtio: make virtio device's structures public.
> >    virtio-x-bus: fix allow_hotplug assertion.
> >    virtio-blk: don't use pointer for configuration.
> >    virtio-blk: add the virtio-blk device.
> >    virtio-blk-pci: switch to new API.
> >    virtio-blk-s390: switch to the new API.
> >    virtio-blk-ccw switch to new API.
> >    virtio-blk: cleanup: init and exit functions.
> >    virtio-blk: cleanup: QOM cast
> >    virtio-blk: cleanup: remove qdev field.
> >
> >   hw/s390x/s390-virtio-bus.c |  32 ++++++----
> >   hw/s390x/s390-virtio-bus.h |  13 +++-
> >   hw/s390x/virtio-ccw.c      |  39 +++++++-----
> >   hw/s390x/virtio-ccw.h      |  14 ++++-
> >   hw/virtio-balloon.c        |  15 -----
> >   hw/virtio-balloon.h        |  14 +++++
> >   hw/virtio-blk.c            | 151 
> > +++++++++++++++++++++++++--------------------
> >   hw/virtio-blk.h            |  39 ++++++++++++
> >   hw/virtio-net.c            |  50 ---------------
> >   hw/virtio-net.h            |  50 +++++++++++++++
> >   hw/virtio-pci.c            | 129 +++++++++++++++++---------------------
> >   hw/virtio-pci.h            |  15 ++++-
> >   hw/virtio-rng.c            |  19 ------
> >   hw/virtio-rng.h            |  19 ++++++
> >   hw/virtio-scsi.c           |  15 -----
> >   hw/virtio-scsi.h           |  16 +++++
> >   hw/virtio-serial-bus.c     |  41 ------------
> >   hw/virtio-serial.h         |  41 ++++++++++++
> >   hw/virtio.h                |   2 -
> >   19 files changed, 405 insertions(+), 309 deletions(-)
> >
>