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

[KONRAD Frédéric]

On 15/03/2013 13:44, Cornelia Huck wrote:
> 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.
So the issue is not this two patches, I mean the patch-set alone is not 
working
right?

I'll take a look.
> > 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(-)