From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53336) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UoxSG-0005PM-UB for qemu-devel@nongnu.org; Tue, 18 Jun 2013 11:04:22 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UoxSE-00044q-Ly for qemu-devel@nongnu.org; Tue, 18 Jun 2013 11:04:20 -0400 Received: from e9.ny.us.ibm.com ([32.97.182.139]:36791) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UoxSE-00043p-Hm for qemu-devel@nongnu.org; Tue, 18 Jun 2013 11:04:18 -0400 Received: from /spool/local by e9.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 18 Jun 2013 11:04:17 -0400 Received: from d01relay06.pok.ibm.com (d01relay06.pok.ibm.com [9.56.227.116]) by d01dlp03.pok.ibm.com (Postfix) with ESMTP id 5E542C90044 for ; Tue, 18 Jun 2013 11:04:15 -0400 (EDT) Received: from d03av04.boulder.ibm.com (d03av04.boulder.ibm.com [9.17.195.170]) by d01relay06.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r5IF36tS44957874 for ; Tue, 18 Jun 2013 11:03:07 -0400 Received: from d03av04.boulder.ibm.com (loopback [127.0.0.1]) by d03av04.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id r5IF2xnO011137 for ; Tue, 18 Jun 2013 09:03:00 -0600 From: Anthony Liguori In-Reply-To: <87a9mnlf17.fsf@blackfin.pond.sub.org> References: <1371208516-7857-1-git-send-email-armbru@redhat.com> <1371208516-7857-9-git-send-email-armbru@redhat.com> <87hah0g5vg.fsf@codemonkey.ws> <87a9mnlf17.fsf@blackfin.pond.sub.org> Date: Tue, 18 Jun 2013 10:02:54 -0500 Message-ID: <87wqprihr5.fsf@codemonkey.ws> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Subject: Re: [Qemu-devel] [PATCH v3 08/16] boot-order-test: New; covering just PC for now List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: jan.kiszka@siemens.com, qemu-devel@nongnu.org, Luiz Capitulino , alex.williamson@redhat.com, aviksil@linux.vnet.ibm.com, afaerber@suse.de Markus Armbruster writes: > Anthony Liguori writes: > >> Markus Armbruster writes: >> >>> Signed-off-by: Markus Armbruster >>> --- >>> tests/Makefile | 2 ++ >>> tests/boot-order-test.c | 68 >>> +++++++++++++++++++++++++++++++++++++++++++++++++ >>> 2 files changed, 70 insertions(+) >>> create mode 100644 tests/boot-order-test.c >>> >>> diff --git a/tests/Makefile b/tests/Makefile >>> index c107489..394e029 100644 >>> --- a/tests/Makefile >>> +++ b/tests/Makefile >>> @@ -54,6 +54,7 @@ gcov-files-i386-y = hw/fdc.c >>> check-qtest-i386-y += tests/ide-test$(EXESUF) >>> check-qtest-i386-y += tests/hd-geo-test$(EXESUF) >>> gcov-files-i386-y += hw/hd-geometry.c >>> +check-qtest-i386-y += tests/boot-order-test$(EXESUF) >>> 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) >>> @@ -130,6 +131,7 @@ tests/m48t59-test$(EXESUF): tests/m48t59-test.o >>> tests/fdc-test$(EXESUF): tests/fdc-test.o >>> tests/ide-test$(EXESUF): tests/ide-test.o $(libqos-pc-obj-y) >>> tests/hd-geo-test$(EXESUF): tests/hd-geo-test.o >>> +tests/boot-order-test$(EXESUF): tests/boot-order-test.o >>> 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) >>> diff --git a/tests/boot-order-test.c b/tests/boot-order-test.c >>> new file mode 100644 >>> index 0000000..2215710 >>> --- /dev/null >>> +++ b/tests/boot-order-test.c >>> @@ -0,0 +1,68 @@ >>> +/* >>> + * Boot order test cases. >>> + * >>> + * Copyright (c) 2013 Red Hat Inc. >>> + * >>> + * Authors: >>> + * Markus Armbruster , >>> + * >>> + * This work is licensed under the terms of the GNU GPL, version 2 or later. >>> + * See the COPYING file in the top-level directory. >>> + */ >>> + >>> +#include >>> +#include "libqtest.h" >>> + >>> +static void test_pc_cmos_byte(int reg, int expected) >>> +{ >>> + int actual; >>> + >>> + outb(0x70, reg); >>> + actual = inb(0x71); >>> + g_assert_cmphex(actual, ==, expected); >>> +} >>> + >>> +static void test_pc_cmos(uint8_t boot1, uint8_t boot2) >>> +{ >>> + test_pc_cmos_byte(0x38, boot1); >>> + test_pc_cmos_byte(0x3d, boot2); >>> +} >>> + >>> +static void test_pc_with_args(const char *test_args, >>> + uint8_t boot1, uint8_t boot2, >>> + uint8_t reboot1, uint8_t reboot2) >>> +{ >>> + char *args = g_strdup_printf("-nodefaults -display none %s", test_args); >>> + >>> + qtest_start(args); >>> + test_pc_cmos(boot1, boot2); >>> + qmp("{ 'execute': 'system_reset' }"); > test_pc_cmos(reboot1, reboot2); >> >> I think this races. I'd suggest doing a tight loop of this test and >> running it a few thousand times to see if you can catch it. >> >> qmp_system_reset() calls qemu_system_reset_requested() which stops all >> CPUs but let's control fall back to the main loop which actually does >> the device reset. >> >> I think there's a tiny window where this command could return while the >> reset routines have not been actually called yet. >> >> Technically speaking, I think it's necessary to wait for a reset event >> to know that the device model has been reset. > > Hmm. > > First attempt to "win" this race: tight loop around test_a_boot_order(), > i.e. the complete test. Failed because libqtest leaks two file > descriptors and some memory per iteration. With that fixed (patch > coming), I still couldn't make the test fail in >75,000 runs on two > otherwise pretty much unloaded cores. > > Second attempt: tight loop around just > > qmp("{ 'execute': 'system_reset' }"); > actual = read_boot_order(); > g_assert_cmphex(actual, ==, expected_reboot); > > Still no luck with x86, but "success" with ppc. > > Waiting for the event RESET is safe. But doing that right involves > quite some infrastructure work. All we have now is qtest_qmpv(), which > sends the command, then reads QMP output character by character until it > got a complete object. Normally, that's the QMP command response. But > it could be an event. Racy all by itself, even without my "help" :) > > Oh, and it doesn't know about strings, it just counts curlies. If the > output has unmatched curlies in strings... I wonder how this code ever > made it past review ;-P > > The proper solution is real QMP support in libqtest. Unfortunately, > that's not something I can do right now. > > fdc-test.c uses qmp("") to ignore an expected event. If I put a similar > hack into boot-order-test.c, ppc survives >500,000 iterations. Good > enough to get this test in? With a very big comment stating what's happening. Regards, Anthony Liguori > > [...]