From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34937) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Uowev-0002hE-NT for qemu-devel@nongnu.org; Tue, 18 Jun 2013 10:13:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Uoweq-00005M-QG for qemu-devel@nongnu.org; Tue, 18 Jun 2013 10:13:21 -0400 Received: from cantor2.suse.de ([195.135.220.15]:44290 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Uoweq-00005F-Gl for qemu-devel@nongnu.org; Tue, 18 Jun 2013 10:13:16 -0400 Message-ID: <51C06AF5.3040503@suse.de> Date: Tue, 18 Jun 2013 16:13:09 +0200 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= MIME-Version: 1.0 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> In-Reply-To: <87a9mnlf17.fsf@blackfin.pond.sub.org> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable 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: Anthony Liguori , jan.kiszka@siemens.com, qemu-devel@nongnu.org, Luiz Capitulino , alex.williamson@redhat.com, aviksil@linux.vnet.ibm.com Am 18.06.2013 15:33, schrieb Markus Armbruster: > Anthony Liguori writes: >=20 >> 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 =3D hw/fdc.c >>> check-qtest-i386-y +=3D tests/ide-test$(EXESUF) >>> check-qtest-i386-y +=3D tests/hd-geo-test$(EXESUF) >>> gcov-files-i386-y +=3D hw/hd-geometry.c >>> +check-qtest-i386-y +=3D tests/boot-order-test$(EXESUF) >>> 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) >>> @@ -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 o= r 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 =3D inb(0x71); >>> + g_assert_cmphex(actual, =3D=3D, 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 =3D g_strdup_printf("-nodefaults -display none %s", t= est_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 th= e >> 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. >=20 > Hmm. >=20 > 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. >=20 > Second attempt: tight loop around just >=20 > qmp("{ 'execute': 'system_reset' }"); > actual =3D read_boot_order(); > g_assert_cmphex(actual, =3D=3D, expected_reboot); >=20 > Still no luck with x86, but "success" with ppc. >=20 > 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 i= t > 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" :) >=20 > 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 >=20 > The proper solution is real QMP support in libqtest. Unfortunately, > that's not something I can do right now. Didn't Michael or Jason have patches improving the qtest QMP support for migration some months ago? At least they allowed to access the QMP response in parsed form IIRC. Andreas >=20 > fdc-test.c uses qmp("") to ignore an expected event. If I put a simila= r > hack into boot-order-test.c, ppc survives >500,000 iterations. Good > enough to get this test in? >=20 > [...] >=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