From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55006) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ve0Nv-0003tv-3B for qemu-devel@nongnu.org; Wed, 06 Nov 2013 05:30:56 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Ve0Np-0007FR-NZ for qemu-devel@nongnu.org; Wed, 06 Nov 2013 05:30:51 -0500 Received: from mail-ee0-x22f.google.com ([2a00:1450:4013:c00::22f]:43784) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ve0Np-0007FD-Fx for qemu-devel@nongnu.org; Wed, 06 Nov 2013 05:30:45 -0500 Received: by mail-ee0-f47.google.com with SMTP id c13so2402239eek.20 for ; Wed, 06 Nov 2013 02:30:44 -0800 (PST) Date: Wed, 6 Nov 2013 11:30:41 +0100 From: Stefan Hajnoczi Message-ID: <20131106103041.GA24507@stefanha-thinkpad.redhat.com> References: <1383141276-19230-1-git-send-email-stefanha@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1383141276-19230-1-git-send-email-stefanha@redhat.com> Subject: Re: [Qemu-devel] [PATCH v3 0/7] qdev and blockdev refcount leak fixes List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Paolo Bonzini , qemu-devel@nongnu.org, Markus Armbruster , Andreas Faerber 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