From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52616) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ve6WK-0008FJ-1W for qemu-devel@nongnu.org; Wed, 06 Nov 2013 12:04:03 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Ve6WE-0005uT-ML for qemu-devel@nongnu.org; Wed, 06 Nov 2013 12:03:55 -0500 Received: from cantor2.suse.de ([195.135.220.15]:39691 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ve6WE-0005uN-DU for qemu-devel@nongnu.org; Wed, 06 Nov 2013 12:03:50 -0500 Message-ID: <527A6278.9030308@suse.de> Date: Wed, 06 Nov 2013 16:38:32 +0100 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1383141276-19230-1-git-send-email-stefanha@redhat.com> <1383141276-19230-7-git-send-email-stefanha@redhat.com> In-Reply-To: <1383141276-19230-7-git-send-email-stefanha@redhat.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v3 6/7] qdev-monitor-test: add device_add leak test cases List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi , qemu-devel@nongnu.org Cc: Paolo Bonzini , Markus Armbruster Am 30.10.2013 14:54, schrieb Stefan Hajnoczi: > Ensure that the device_add error code path deletes device objects. > Failure to do so not only leaks the objects but can also keep other > objects (like drive or netdev) alive due to qdev properties holding > references. >=20 > Signed-off-by: Stefan Hajnoczi > --- > tests/Makefile | 2 ++ > tests/qdev-monitor-test.c | 81 +++++++++++++++++++++++++++++++++++++++= ++++++++ > 2 files changed, 83 insertions(+) > create mode 100644 tests/qdev-monitor-test.c >=20 > diff --git a/tests/Makefile b/tests/Makefile > index 7863123..2771f92 100644 > --- a/tests/Makefile > +++ b/tests/Makefile > @@ -68,6 +68,7 @@ check-qtest-i386-y +=3D tests/rtc-test$(EXESUF) > check-qtest-i386-y +=3D tests/i440fx-test$(EXESUF) > check-qtest-i386-y +=3D tests/fw_cfg-test$(EXESUF) > check-qtest-i386-y +=3D tests/blockdev-test$(EXESUF) > +check-qtest-i386-y +=3D tests/qdev-monitor-test$(EXESUF) > check-qtest-x86_64-y =3D $(check-qtest-i386-y) > gcov-files-i386-y +=3D i386-softmmu/hw/mc146818rtc.c Same comment here about adding to the list. > gcov-files-x86_64-y =3D $(subst i386-softmmu/,x86_64-softmmu/,$(gcov-f= iles-i386-y)) > @@ -176,6 +177,7 @@ tests/tmp105-test$(EXESUF): tests/tmp105-test.o $(l= ibqos-omap-obj-y) > tests/i440fx-test$(EXESUF): tests/i440fx-test.o $(libqos-pc-obj-y) > tests/fw_cfg-test$(EXESUF): tests/fw_cfg-test.o $(libqos-pc-obj-y) > tests/blockdev-test$(EXESUF): tests/blockdev-test.o $(libqos-pc-obj-y) > +tests/qdev-monitor-test$(EXESUF): tests/qdev-monitor-test.o $(libqos-p= c-obj-y) > tests/qemu-iotests/socket_scm_helper$(EXESUF): tests/qemu-iotests/sock= et_scm_helper.o > =20 > # QTest rules > diff --git a/tests/qdev-monitor-test.c b/tests/qdev-monitor-test.c > new file mode 100644 > index 0000000..d3d6d39 > --- /dev/null > +++ b/tests/qdev-monitor-test.c > @@ -0,0 +1,81 @@ > +/* > + * qdev-monitor.c test cases > + * > + * Copyright (C) 2013 Red Hat Inc. > + * > + * Authors: > + * Stefan Hajnoczi > + * > + * This work is licensed under the terms of the GNU LGPL, version 2.1 = or later. > + * See the COPYING.LIB file in the top-level directory. > + */ > + > +#include > +#include > +#include "libqtest.h" > +#include "qapi/qmp/qjson.h" > + > +static void test_device_add(void) > +{ > + QDict *response; > + QDict *error; > + > + qtest_start("-drive if=3Dnone,id=3Ddrive0"); > + > + /* Make device_add fail. If this leaks the virtio-blk-pci device = then a > + * reference to drive0 will also be held (via qdev properties). > + */ > + response =3D qmp("{\"execute\": \"device_add\"," > + " \"arguments\": {" > + " \"driver\": \"virtio-blk-pci\"," > + " \"drive\": \"drive0\"" > + "}}"); > + g_assert(response); > + error =3D qdict_get_qdict(response, "error"); > + g_assert(!strcmp(qdict_get_try_str(error, "class") ?: "", > + "GenericError")); > + g_assert(!strcmp(qdict_get_try_str(error, "desc") ?: "", > + "Device initialization failed.")); > + QDECREF(response); > + > + /* Delete the drive */ > + response =3D qmp("{\"execute\": \"human-monitor-command\"," > + " \"arguments\": {" > + " \"command-line\": \"drive_del drive0\"" > + "}}"); > + g_assert(response); > + g_assert(!strcmp(qdict_get_try_str(response, "return") ?: "(null)"= , "")); > + QDECREF(response); > + > + /* Try to re-add the drive. This fails with duplicate IDs if a le= aked > + * virtio-blk-pci exists that holds a reference to the old drive0. > + */ > + response =3D qmp("{\"execute\": \"human-monitor-command\"," > + " \"arguments\": {" > + " \"command-line\": \"drive_add pci-addr=3Dauto i= f=3Dnone,id=3Ddrive0\"" > + "}}"); > + g_assert(response); > + g_assert(!strcmp(qdict_get_try_str(response, "return") ?: "", > + "OK\r\n")); > + QDECREF(response); > + > + qtest_end(); > +} > + > +int main(int argc, char **argv) > +{ > + const char *arch =3D qtest_get_arch(); > + > + /* Check architecture */ > + if (strcmp(arch, "i386") && strcmp(arch, "x86_64")) { > + g_test_message("Skipping test for non-x86\n"); > + return 0; > + } > + > + /* Run the tests */ > + g_test_init(&argc, &argv, NULL); > + > + qtest_add_func("/qdev/device_add", test_device_add); What about naming these tests /qmp/...? Again, test itself looks great on a brief look. Andreas > + > + return g_test_run(); > +} >=20 --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg