qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH for-2.4 v5 0/7] Target-specific unit test suppor


From: Eduardo Habkost
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
Date: Wed, 8 Apr 2015 10:42:04 -0300
User-agent: Mutt/1.5.23 (2014-03-12)

(CCing Jiri Denemark in case he has anything to add about libvirt
requirements)

On Wed, Apr 08, 2015 at 03:06:41PM +0200, Andreas Färber 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 command-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 need
> for stubs and without risking to break the test suite by forgetting to
> run make check after device changes or forcing work duplication onto others.

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: <address@hidden>
  http://article.gmane.org/gmane.comp.emulators.qemu/299411

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()).

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 be
unfortunate.

I don't understand the "forcing work duplication onto others" part.
What kind of work duplication is being forced onto others?

> 
> Specifically, you added properties to allow inspection of CPUID feature
> bits that - as I understood - today no one (including libvirt) is using.

Not sure why this is related to the test code, but:

libvirt didn't use the feature-words property because it requires
re-running QEMU for every CPU model. I don't know why libvirt doesn't
use the filtered-features property yet (as it is a more reliable way to
check if a given CPU configuration is runnable in a host, than the
current libvirt approach of running CPUID directly).

We also found out recently that libvirt have serious issues with the
fact that CPU models may change depending on the machine-type, and their
approach will probably change from "ask QEMU for CPU model information
and see if it matches what libvirt expects" (requiring more information
to flow from QEMU to libvirt through a probing interface) to "ensure
that QEMU will make the CPU match exactly what libvirt expects"
(requiring more information to flow from libvirt to QEMU through a more
flexible configuration interface). That makes the feature flag QOM
properties even more relevant for them.

-- 
Eduardo



reply via email to

[Prev in Thread] Current Thread [Next in Thread]