Re: [Qemu-devel] [PATCH v3 0/7] qdev and blockdev refcount leak fixes

[Stefan Hajnoczi <stefanha@gmail.com>]

On Wed, Oct 30, 2013 at 02:54:29PM +0100, Stefan Hajnoczi wrote:
> v3:
>  * I lost track of this patch, now I'm pushing it again
>  * Rebase onto qemu.git/master
>  * Add Patch 7 to do s/qdev_free()/object_unparent()/ [afaerber]
> 
> It started with a bug report along these lines:
> 
>   (qemu) device_add virtio-blk-pci,drive=drive0,x-data-plane=on
>   device is incompatible with x-data-plane, use scsi=off
>   Device initialization failed.
>   Device initialization failed.
>   Device 'virtio-blk-pci' could not be initialized
>   (qemu) drive_del drive0
>   (qemu) drive_add 0 if=none,id=drive0
>   Duplicate ID 'drive0' for drive
> 
> The drive_add command should succeed since the old "drive0" was deleted.
> 
> With the help of Andreas and Paolo we figured out that the problem is not
> virtio-blk or dataplane.  There are actually two problems:
> 
> 1. qdev_device_add() must release its DeviceState reference if
>    qdev_init() failed.
> 
> 2. blockdev_init() must release its QemuOpts on failure or early return when no
>    file= option was specified.
> 
> This series fixes these problems and then qtest test cases for both bugs.  In
> order to do this we need to add QMP response objects to the libqtest API, which
> currently discards QMP responses.
> 
> Patches 1 & 2 fix the leaks.
> Patches 2 & 3 add QMP response objects to libqtest.
> Patches 5 & 6 add qtest test cases for the bugs.
> Patch 7 replaces the confusing qdev_free() function with object_unparent()
> 
> Stefan Hajnoczi (7):
>   blockdev: fix drive_init() opts and bs_opts leaks
>   qdev: unref qdev when device_add fails
>   libqtest: rename qmp() to qmp_discard_response()
>   libqtest: add qmp(fmt, ...) -> QDict* function
>   blockdev-test: add test case for drive_add duplicate IDs
>   qdev-monitor-test: add device_add leak test cases
>   qdev: drop misleading qdev_free() function
> 
>  blockdev.c                | 27 +++++++++-------
>  hw/acpi/piix4.c           |  2 +-
>  hw/core/qdev.c            | 12 ++-----
>  hw/pci/pci-hotplug-old.c  |  2 +-
>  hw/pci/pci_bridge.c       |  2 +-
>  hw/pci/pcie.c             |  2 +-
>  hw/pci/shpc.c             |  2 +-
>  hw/s390x/virtio-ccw.c     |  2 +-
>  hw/scsi/scsi-bus.c        |  6 ++--
>  hw/usb/bus.c              |  7 ++--
>  hw/usb/dev-storage.c      |  2 +-
>  hw/usb/host-legacy.c      |  2 +-
>  hw/virtio/virtio-bus.c    |  4 +--
>  hw/xen/xen_platform.c     |  2 +-
>  include/hw/qdev-core.h    |  1 -
>  qdev-monitor.c            |  6 ++--
>  tests/Makefile            |  4 +++
>  tests/blockdev-test.c     | 59 ++++++++++++++++++++++++++++++++++
>  tests/boot-order-test.c   |  4 +--
>  tests/fdc-test.c          | 15 +++++----
>  tests/ide-test.c          | 10 +++---
>  tests/libqtest.c          | 72 +++++++++++++++++++++++++++++++----------
>  tests/libqtest.h          | 51 +++++++++++++++++++++++++----
>  tests/qdev-monitor-test.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++
>  24 files changed, 299 insertions(+), 78 deletions(-)
>  create mode 100644 tests/blockdev-test.c
>  create mode 100644 tests/qdev-monitor-test.c
> 
> -- 
> 1.8.3.1

Applied the remaining patches to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan