On 01.12.2015 08:35, tu bo wrote:
Hi Max:
在 2015/12/1 3:38, Max Reitz 写道:
On 26.11.2015 10:53, Bo Tu wrote:
From: Bo Tu <address@hidden>
The tests for device type "ide_cd" should only be tested for the pc
platform.
The default device id of hard disk on the s390 platform differs to that
of the x86 platform. A new variable device_id is defined and "virtio0"
set for the s390 platform. A x86 platform specific output file is also
needed.
Warning message expected for s390x when drive without device.
Reviewed-by: Max Reitz <address@hidden>
I can't remember having given this for this patch. ;-)
It's my mistake, please forgive me for it.
No problem.
Reviewed-by: Sascha Silbe <address@hidden>
Signed-off-by: Bo Tu <address@hidden>
---
tests/qemu-iotests/051 | 99 ++++++----
tests/qemu-iotests/051.out | 422
----------------------------------------
tests/qemu-iotests/051.pc.out | 422
++++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/051.s390.out | 335 +++++++++++++++++++++++++++++++
tests/qemu-iotests/Makefile | 7 +-
5 files changed, 828 insertions(+), 457 deletions(-)
delete mode 100644 tests/qemu-iotests/051.out
create mode 100644 tests/qemu-iotests/051.pc.out
create mode 100644 tests/qemu-iotests/051.s390.out
I didn't even know we had a Makefile in tests/qemu-iotests. I don't
think it is invoked automatically, and if it was, it should not modify
the source tree. For instance, I have a separate build directory so my
source tree remains clean.
I don't think expecting the user to invoke the Makefile manually is a
good idea either. When I said "copy" I literally meant adding a copy of
051.s390.out with this patch (even though it is ugly, I'd still consider
it better).
Adding a copy of 051.s390.out is not perfect, but acceptable.
Anyway, we can circumvent the whole issue anyway. While 051 does test
some devices explicitly which are only available on the PC platform, the
thing in question which would require a s390-specific output, too, is
the final part of the test ("=== Snapshot mode ===").
You can just drop the "case" introduced by this patch and set device_id
to whatever you want (e.g. "drive0" or "none0"). Then, you replace every
"-drive file..." by "-drive if=none,id=$device_id,file=..." and you're
done (obviously, the reference output needs to be adjusted for the
changed drive ID).
Then, you don't need an 051.s390.out at all, and can just make that the
common output (051.out).
I got your idea. we can get the common output(051.out) for the final
part of the test ("=== Snapshot mode ===") with this solution.
However, other parts of this test has different outputs as below,
1. s390x has no output for some test commands for "=== No medium ===",
"=== Read-only ===" and "=== Cache modes ===". for example, "line 204 -
209" has output for pc, but no output for s390x.
202 case "$QEMU_DEFAULT_MACHINE" in
203 pc)
204 run_qemu -drive media=cdrom,cache=none
205 run_qemu -drive media=cdrom,cache=directsync
206 run_qemu -drive media=cdrom,cache=writeback
207 run_qemu -drive media=cdrom,cache=writethrough
208 run_qemu -drive media=cdrom,cache=unsafe
209 run_qemu -drive media=cdrom,cache=invalid_value
210 ;;
211 *)
212 ;;
213 esac
Yes, indeed. However, it has this output only for pc, but it will not
have any output for any other platform type. Therefore, we don't need an
s390-specific reference output, but only one reference output for pc
(051.pc.out) and one for all the other platforms (051.out).
2. s390x has "Warning: Orphaned drive without device:" for
run_qemu -drive file="$TEST_IMG",if=scsi,readonly=on
run_qemu -drive if=scsi
Please refer
http://lists.nongnu.org/archive/html/qemu-devel/2015-11/msg04836.html.
Yes, and that's exactly the reason why I advocated filtering out that
line because we don't know whether it is visible on any platform but s390.
(e.g. see the _filter_orphan added in
http://lists.nongnu.org/archive/html/qemu-devel/2015-05/msg05992.html)
Even if we decide against adding such a filter I think we should put the
s390 output into the common 051.out file anyway. If someone tries to run
the tests on a non-s390 and non-pc platform and does not get this
warning, thus failing the test, we can still talk about adding that filter.
I plan to change the final part of the test("=== Snapshot mode ===")
with your solution, and adding a copy of 051.s390.out. If this is fine
to you, I plan to send a new version of patch for review asap, since
Christmas is coming :-)
I'm fine with patches after christmas, too. ;-)
If you change the final part of the test and put the output into 051.out
instead of 051.s390.out (and don't create any copy for s390-virtio),
that should be fine.