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.
>
> Signed-off-by: Stefan Hajnoczi <stefa...@redhat.com>
> ---
> tests/Makefile | 2 ++
> tests/qdev-monitor-test.c | 81
> +++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 83 insertions(+)
> create mode 100644 tests/qdev-monitor-test.c
>
> 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 += tests/rtc-test$(EXESUF)
> check-qtest-i386-y += tests/i440fx-test$(EXESUF)
> check-qtest-i386-y += tests/fw_cfg-test$(EXESUF)
> check-qtest-i386-y += tests/blockdev-test$(EXESUF)
> +check-qtest-i386-y += tests/qdev-monitor-test$(EXESUF)
> check-qtest-x86_64-y = $(check-qtest-i386-y)
> gcov-files-i386-y += i386-softmmu/hw/mc146818rtc.c
Same comment here about adding to the list.
> gcov-files-x86_64-y = $(subst
> i386-softmmu/,x86_64-softmmu/,$(gcov-files-i386-y))
> @@ -176,6 +177,7 @@ tests/tmp105-test$(EXESUF): tests/tmp105-test.o
> $(libqos-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-pc-obj-y)
> tests/qemu-iotests/socket_scm_helper$(EXESUF):
> tests/qemu-iotests/socket_scm_helper.o
>
> # 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 <stefa...@redhat.com>
> + *
> + * 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 <string.h>
> +#include <glib.h>
> +#include "libqtest.h"
> +#include "qapi/qmp/qjson.h"
> +
> +static void test_device_add(void)
> +{
> + QDict *response;
> + QDict *error;
> +
> + qtest_start("-drive if=none,id=drive0");
> +
> + /* 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 = qmp("{\"execute\": \"device_add\","
> + " \"arguments\": {"
> + " \"driver\": \"virtio-blk-pci\","
> + " \"drive\": \"drive0\""
> + "}}");
> + g_assert(response);
> + error = 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 = 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 leaked
> + * virtio-blk-pci exists that holds a reference to the old drive0.
> + */
> + response = qmp("{\"execute\": \"human-monitor-command\","
> + " \"arguments\": {"
> + " \"command-line\": \"drive_add pci-addr=auto
> if=none,id=drive0\""
> + "}}");
> + 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 = 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();
> +}
>
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
