Re: [Qemu-devel] [PATCH v2 2/4] qemu-iotests: s390x: fix test 051

[Sascha Silbe]

Dear Max,

Max Reitz <address@hidden> writes:

> OK, so it is expected for s390x; however, this is strictly speaking not
> the output file for s390x but for any platform but PC. That's why I'd
> rather not have it in this “generic” reference output.
>
> This patch already assumes that the iotests only support s390 and PC
> (hunk @@ -251,28 +273,37 @@ in 051 adds a case statement which knows
> only these two cases). Therefore, I would be fine with renaming this
> reference output file to "051.s390.out" with a copy
> "051.s390-ccw-virtio.out" and just removing the generic "051.out". Then
> you can keep the warnings in.
>
> (Or you filter the warnings out and keep it as "051.out".)

This PC/s390x-only hunk looks like an oversight to me. We should make
one of the options the default. I'd prefer defaulting to virtio (see
below), but since the test previously hard-coded IDE that would be fine,
too.

For the other cases, IIRC it's really PC that's the odd one out, that's
why I suggested adding a PC-specific output file and patching the
generic output file to look like the output on most other architectures.

But I'm fine with anything that gets the job done on both PC and s390x
today. I'll have a closer look again once things settle down a
bit. While the support for one output file per architecture is a very
useful generic solution, we should make use of it only sparingly. The
git log already shows that people forget to update test output
files. This will get worse with multiple output files.

One of the things I'm considering is splitting off the IDE tests to a
separate test case and skipping it entirely on machines that do not
support IDE. Another is using virtio-blk on all architectures whenever
the test case doesn't care about the device type. (I doubt the tests
currently work on architectures that don't support virtio, but will of
course check this).

Sascha
-- 
Softwareentwicklung Sascha Silbe, Niederhofenstraße 5/1, 71229 Leonberg
https://se-silbe.de/
USt-IdNr. DE281696641