From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57730) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WLIeG-0003gi-4u for qemu-devel@nongnu.org; Wed, 05 Mar 2014 15:42:46 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WLIe5-0000Vm-Mg for qemu-devel@nongnu.org; Wed, 05 Mar 2014 15:42:40 -0500 Message-ID: <53178C32.3080103@suse.de> Date: Wed, 05 Mar 2014 21:42:26 +0100 From: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= MIME-Version: 1.0 References: <1391947053-26753-1-git-send-email-afaerber@suse.de> <1392004376-17016-1-git-send-email-aik@ozlabs.ru> <1392004376-17016-3-git-send-email-aik@ozlabs.ru> <52F8F4BE.8000108@suse.de> <52FD95F0.6010101@ozlabs.ru> In-Reply-To: <52FD95F0.6010101@ozlabs.ru> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 2/2] tests-ppc64: test for -device spapr-pci-host-bridge List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexey Kardashevskiy , qemu-devel@nongnu.org Cc: qemu-ppc@nongnu.org, Alexander Graf Am 14.02.2014 05:05, schrieb Alexey Kardashevskiy: > On 02/11/2014 02:48 AM, Andreas F=C3=A4rber wrote: >> Am 10.02.2014 04:52, schrieb Alexey Kardashevskiy: >>> This adds a test if SPAPR PHB can be added via the command line. >>> >>> Signed-off-by: Alexey Kardashevskiy >>> --- >>> tests/Makefile | 2 ++ >>> tests/spapr-phb-test.c | 28 ++++++++++++++++++++++++++++ >>> 2 files changed, 30 insertions(+) >>> create mode 100644 tests/spapr-phb-test.c >>> >>> diff --git a/tests/Makefile b/tests/Makefile >>> index 3a00ea7..303225e 100644 >>> --- a/tests/Makefile >>> +++ b/tests/Makefile >>> @@ -117,6 +117,7 @@ check-qtest-s390x-y +=3D tests/qom-test$(EXESUF) >>> check-qtest-unicore32-y +=3D tests/qom-test$(EXESUF) >>> check-qtest-xtensa-y +=3D tests/qom-test$(EXESUF) >>> check-qtest-xtensaeb-y =3D $(check-qtest-xtensa-y) >>> +check-qtest-ppc64-y +=3D tests/spapr-phb-test$(EXESUF) >>> =20 >>> check-qapi-schema-y :=3D $(addprefix tests/qapi-schema/, \ >>> comments.json empty.json funny-char.json indented-expr.json = \ >>> @@ -203,6 +204,7 @@ libqos-omap-obj-y =3D $(libqos-obj-y) tests/libqo= s/i2c-omap.o >>> tests/rtc-test$(EXESUF): tests/rtc-test.o >>> tests/m48t59-test$(EXESUF): tests/m48t59-test.o >>> tests/endianness-test$(EXESUF): tests/endianness-test.o >>> +tests/spapr-phb-test$(EXESUF): tests/spapr-phb-test.o $(libqos-obj-y= ) >>> 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 >>> diff --git a/tests/spapr-phb-test.c b/tests/spapr-phb-test.c >>> new file mode 100644 >>> index 0000000..8cad354 >>> --- /dev/null >>> +++ b/tests/spapr-phb-test.c >>> @@ -0,0 +1,28 @@ >>> +/* >>> + * QTest testcase for SPAPR PHB >>> + * >> >> Missing some IBM or aik copyright? >> >>> + * 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" >>> + >>> +#define TYPE_SPAPR_PCI_HOST_BRIDGE "spapr-pci-host-bridge" >>> + >>> +static void test_phb_device(void) >>> +{ >>> + qtest_start("-device " TYPE_SPAPR_PCI_HOST_BRIDGE ",index=3D100"= ); >>> + >>> + qtest_end(); >>> +} >>> + >>> +int main(int argc, char **argv) >>> +{ >>> + g_test_init(&argc, &argv, NULL); >>> + >>> + qtest_add_func("/qmp/phb_device", test_phb_device); >>> + >>> + return g_test_run(); >>> +} >>> + >> >> Trailing white line FWIW. >> >> This is looking good, thanks for your work. Apart from the device >> there's nothing ppc-specific in here, so I might take it through >> qom-next. One nit here is the test name, which I think should rather b= e >> /spapr-phb/device (nothing QMP-specific in there) - I could tweak that >> for you. >> >> Another question is, do you plan to test variations of the device? >> Otherwise I would suggest to place the qtest_start() and qtest_end() >> into main(), so that individual future tests can test certain >> functionalities on the instantiated device rather than repeatedly >> launching QEMU processes. >=20 > Is this better? It does the job. Hmm... ;) > I am not quite sure what qtest_add_func() is for. Is there any good exa= mple > of calling such a function via json or whatever it is? Thanks. There are lots of other tests/*-test.c files, including the series you replied to. I've tweaked your original patch so that it outputs: $ make check-qtest-ppc64 V=3D1 [...] TEST: tests/spapr-phb-test... (pid=3D3080) /ppc64/spapr-phb/device: OK PASS: tests/spapr-phb-test "/spapr-phb/device" is the name of the unit test supplied to qtest_add_func(), which then prepends the architecture for uniqueness. Without calling it, there is no indication that the test was successful. Individual tests reading/writing registers etc. will need the qemu-system-ppc64 process running, so g_test_run() needs to go before qtest_end(). Or if you want to launch it multiple times (e.g. with different parameters) place it into the test function like originally. Your test did not pass on qemu.git or qom-next - I picked up the corresponding fix from ppc-next to reallow -device spapr-pci-host-bridge. Further the patch was missing a line for gcov to pick up the .o file: Gcov report for ppc64-softmmu/hw/ppc/spapr_pci.c: File '/home/andreas/QEMU/qemu/hw/ppc/spapr_pci.c' Lines executed:42.98% of 342 File '../trace/generated-tracers.h' Lines executed:0.00% of 12 File '/usr/include/libfdt.h' Lines executed:100.00% of 5 File '/usr/include/libfdt_env.h' Lines executed:100.00% of 2 File '/home/andreas/QEMU/qemu/include/hw/pci-host/spapr.h' Lines executed:0.00% of 2 File '/home/andreas/QEMU/qemu/include/hw/ppc/spapr.h' Lines executed:22.22% of 9 File '/home/andreas/QEMU/qemu/include/hw/pci/pci.h' Lines executed:0.00% of 4 File '/home/andreas/QEMU/qemu/include/hw/irq.h' Lines executed:0.00% of 4 File '/home/andreas/QEMU/qemu/include/qemu/bitops.h' Lines executed:100.00% of 5 File '/home/andreas/QEMU/qemu/include/qemu/bswap.h' Lines executed:100.00% of 6 File '/usr/include/bits/byteswap.h' Lines executed:100.00% of 4 Regards, Andreas > diff --git a/tests/spapr-phb-test.c b/tests/spapr-phb-test.c > new file mode 100644 > index 0000000..11f186c > --- /dev/null > +++ b/tests/spapr-phb-test.c > @@ -0,0 +1,24 @@ > +/* > + * QTest testcase for SPAPR PHB > + * > + * Authors: > + * Alexey Kardashevskiy > + * > + * 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" > + > +#define TYPE_SPAPR_PCI_HOST_BRIDGE "spapr-pci-host-bridge" > + > +int main(int argc, char **argv) > +{ > + g_test_init(&argc, &argv, NULL); > + > + qtest_start("-device " TYPE_SPAPR_PCI_HOST_BRIDGE ",index=3D100"); > + qtest_end(); > + > + return g_test_run(); > +} --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=C3=BCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=C3=B6rffer; HRB 16746 AG N=C3=BC= rnberg