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

["KONRAD Frédéric" <fred.konrad@greensocs.com>]

On 15/03/2013 13:44, Cornelia Huck wrote:
> On Fri, 15 Mar 2013 10:19:50 +0100
> KONRAD Frédéric <fred.konrad@greensocs.com> wrote:
>
>> On 15/03/2013 10:16, fred.konrad@greensocs.com wrote:
>>> From: KONRAD Frederic <fred.konrad@greensocs.com>
>>>
>>> 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(-)
>>>