From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40905) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YfqcJ-0005Sd-1e for qemu-devel@nongnu.org; Wed, 08 Apr 2015 10:06:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YfqcC-0005TQ-PX for qemu-devel@nongnu.org; Wed, 08 Apr 2015 10:06:06 -0400 Received: from cantor2.suse.de ([195.135.220.15]:53878 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YfqcC-0005TD-FF for qemu-devel@nongnu.org; Wed, 08 Apr 2015 10:06:00 -0400 Message-ID: <552535C7.6060400@suse.de> Date: Wed, 08 Apr 2015 16:05:59 +0200 From: =?windows-1252?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1427139432-27530-1-git-send-email-ehabkost@redhat.com> <552527E1.3010604@suse.de> <20150408134204.GT7031@thinpad.lan.raisama.net> In-Reply-To: <20150408134204.GT7031@thinpad.lan.raisama.net> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH for-2.4 v5 0/7] Target-specific unit test support, add unit tests for target-i386/cpu.c code List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: Paolo Bonzini , Jiri Denemark , qemu-devel@nongnu.org, Igor Mammedov Am 08.04.2015 um 15:42 schrieb Eduardo Habkost: > (CCing Jiri Denemark in case he has anything to add about libvirt > requirements) >=20 > On Wed, Apr 08, 2015 at 03:06:41PM +0200, Andreas F=E4rber wrote: >> Am 23.03.2015 um 20:37 schrieb Eduardo Habkost: >>> Changes v4 -> v5: >>> * Rewrite fake kvm_arch_get_supported_cpuid() code >>> * Add new test to ensure KVM defaults won't override explicit comman= d-line >>> options >>> >>> Changes v3 -> v4: >>> * Move target_words_bigendian() prototype back inside tests/x86-stub= .c. >>> >>> Changes v2 -> v3: >>> * Extra KVM "host" CPU model test cases >>> * Move target_words_bigendian() prototype to exec-all.h >>> >>> Changes v1 -> v2: >>> * Make dependency list of test binary much simpler, now that cpus.o >>> was removed. >> >> I don't recall seeing the previous four versions of this... I think >> adding unit tests for whole devices is the wrong approach. We had that >> discussion for tmp105. Instead, using the QTest framework - be it >> extending my pc-cpu-test or adding a new one - allows to reuse the >> device infrastructure in-place in the actual executable without the ne= ed >> for stubs and without risking to break the test suite by forgetting to >> run make check after device changes or forcing work duplication onto o= thers. >=20 > The problem with using the actual executable is that it requires > defining the external interfaces before writing tests, and the whole > point of the unit test is to be allow us to check if we are not doing > anything wrong _while implementing_ those external interfaces. This was > mentioned before, see: > Message-ID: <20140926163410.GA3113@thinpad.lan.raisama.net> > http://article.gmane.org/gmane.comp.emulators.qemu/299411 >=20 > Also, I don't even expect external interfaces to be available for some > hooks that this test code require (e.g. changing the return value of > kvm_arch_get_supported_cpuid()). >=20 > Personally, I want (and will) run this test after every change made on > target-i386, even if the series is not accepted, but adding the test > code to qemu.git would make life easier for everyone. Having to create = a > qemu-x86-unit-tests.git repository for it (even if temporarily) would b= e > unfortunate. Having code in qemu.git and having it in make check are two separate things though, and my problem is with the latter. > I don't understand the "forcing work duplication onto others" part. > What kind of work duplication is being forced onto others? Changes to random device etc. infrastructure can break your x86-specific test case. If people don't expect such cross-subsystem effects then in the worst case maintainers (me, mst, bonzini, armbru, ...) end up sending a pull request to Peter, who then needs to reject it based on a breaking x86 test case. When I work on PReP, I often just run make check-qtest-ppc because the full suite takes much too long, and I expect unit tests to be fine-granular like testing only an APIC ID API but not the whole device infrastructure. So please don't just think about your own requirements, think of the consequences this design has for others. If tests don't get usage because they are not run, that doesn't really help. For QTest we have gcov for evaluating which APIs are covered, if you start from zero with your own private framework now, you don't get that and we'll end up having less ideal coverage in two separate places rather than increasing coverage in the QTest framework we already have. >> Specifically, you added properties to allow inspection of CPUID featur= e >> bits that - as I understood - today no one (including libvirt) is usin= g. >=20 > Not sure why this is related to the test code, [...] Because that is *your* existing external interface that you could use to implement your tests and thereby even give that interface some actual testing and usage beyond my generic qom-test. I don't care about libvirt requirements here, this is all about QEMU. Regards, Andreas --=20 SUSE Linux GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Felix Imend=F6rffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu, Graham Norton; HRB 21284 (AG N=FCrnberg)