From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60850) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WNYfZ-0001io-R5 for qemu-devel@nongnu.org; Tue, 11 Mar 2014 22:13:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WNYfU-0007FF-05 for qemu-devel@nongnu.org; Tue, 11 Mar 2014 22:13:21 -0400 Received: from mail-pb0-f49.google.com ([209.85.160.49]:38775) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WNYfT-0007F3-Nt for qemu-devel@nongnu.org; Tue, 11 Mar 2014 22:13:15 -0400 Received: by mail-pb0-f49.google.com with SMTP id jt11so395799pbb.8 for ; Tue, 11 Mar 2014 19:13:14 -0700 (PDT) Message-ID: <531FC2B4.9070005@ozlabs.ru> Date: Wed, 12 Mar 2014 13:13:08 +1100 From: Alexey Kardashevskiy 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> <53178C32.3080103@suse.de> <5317D683.3060905@ozlabs.ru> In-Reply-To: <5317D683.3060905@ozlabs.ru> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= , qemu-devel@nongnu.org Cc: qemu-ppc@nongnu.org, Alexander Graf On 03/06/2014 12:59 PM, Alexey Kardashevskiy wrote: > On 03/06/2014 07:42 AM, Andreas Färber wrote: >> Am 14.02.2014 05:05, schrieb Alexey Kardashevskiy: >>> On 02/11/2014 02:48 AM, Andreas Färber 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 += tests/qom-test$(EXESUF) >>>>> check-qtest-unicore32-y += tests/qom-test$(EXESUF) >>>>> check-qtest-xtensa-y += tests/qom-test$(EXESUF) >>>>> check-qtest-xtensaeb-y = $(check-qtest-xtensa-y) >>>>> +check-qtest-ppc64-y += tests/spapr-phb-test$(EXESUF) >>>>> >>>>> check-qapi-schema-y := $(addprefix tests/qapi-schema/, \ >>>>> comments.json empty.json funny-char.json indented-expr.json \ >>>>> @@ -203,6 +204,7 @@ libqos-omap-obj-y = $(libqos-obj-y) tests/libqos/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 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" >>>>> + >>>>> +static void test_phb_device(void) >>>>> +{ >>>>> + qtest_start("-device " TYPE_SPAPR_PCI_HOST_BRIDGE ",index=100"); >>>>> + >>>>> + 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 be >>>> /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. >>> >>> Is this better? It does the job. >> >> Hmm... ;) >> >>> I am not quite sure what qtest_add_func() is for. Is there any good example >>> 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=1 >> [...] >> TEST: tests/spapr-phb-test... (pid=3080) >> /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. > > Sure it did not pass, this is why I made the test ;) > > But I wonder why this fix not gone to upstream immediately - it was breakage. > > >> 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 > > > > What did you do to get this output? None from "make check-help" seems to do > this. > > btw it would nice if people updated "make check-help" when they add new > tests as without documentation this is all we have to know what tests are > there. For example, it was not very obvious how > check-tests/qemu-iotests-quick.sh is called and how it works. > > Or there is some howto and I am just disturbingly blind? Thanks. > > > Also thanks for comments and tweaks but I still do not understand if I > still have to do anything regarding these 2 patches. Andreas? >> 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=100"); >>> + qtest_end(); >>> + >>> + return g_test_run(); >>> +} >> > > -- Alexey