On Wed, Oct 17, 2018 at 04:43:15PM -0300, Murilo Opsfelder Araujo wrote: > On Wed, Oct 17, 2018 at 07:40:51PM +0100, Peter Maydell wrote: > > On 17 October 2018 at 18:38, Cleber Rosa <[email protected]> wrote: > > > > > > > > > On 10/17/18 12:29 PM, Eduardo Habkost wrote: > > >> On Wed, Oct 17, 2018 at 01:34:41PM +0100, Peter Maydell wrote: > > >>> So, why does the test code need to care? It's not clear > > >>> from the patch... My expectation would be that you'd > > >>> just test all the testable target architectures, > > >>> regardless of what the host architecture is. > > >> > > >> I tend to agree. Maybe the right solution is to get rid of the > > >> os.uname(). I think the default should be testing all QEMU > > >> binaries that were built, and the host architecture shouldn't > > >> matter. > > > > Yes, looking at os.uname() also seems like an odd thing > > for the tests to be doing here. The test framework > > should be as far as possible host-architecture agnostic. > > (For some of the KVM cases there probably is a need to > > care, but those are exceptions, not the rule.) > > > > > I'm in favor of exercising all built targets, but that seems to me to be > > > on another layer, above the test themselves. This change is about the > > > behavior of a test when not told about the target arch (and thus binary) > > > it should use. > > > > At that level, I think the right answer is "tell the user > > they need to specify the qemu executable they are trying to test". > > In particular, there is no guarantee that the user has actually > > built the executable for the target that corresponds to the > > host, so it doesn't work to try to default to that anyway. > > > > thanks > > -- PMM > > > > I agree with Peter. We can make qemu_bin parameter mandatory. If it is not > given, error out. Trying to guess it based on host architecture turns out to > be > troublesome. > > If we decide to follow this approach of not guessing QEMU binary, we should > update docs/devel/testing.rst to make it crystal clear qemu_bin parameter is > mandatory.
That's not a perfect solution to me, but it sounds better than using uname() and silently making a decision for the user. -- Eduardo
