Hi Kevin:
On 04/27/2015 07:34 PM, Kevin Wolf wrote:
Am 27.04.2015 um 13:24 hat Max Reitz geschrieben:
On 27.04.2015 09:15, tu bo wrote:
Hi Max:
On 04/24/2015 01:07 AM, Max Reitz wrote:
Well, that's a peculiar commit title. :-)
I guess it's supposed to be "qemu-iotests: s390x: fix test 130"?
You're right. I will change it in the next version :-)
On 23.04.2015 04:42, Xiao Guang Chen wrote:
From: Bo Tu <t...@linux.vnet.ibm.com>
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.
Signed-off-by: Bo Tu <t...@linux.vnet.ibm.com>
---
tests/qemu-iotests/130 | 13 +++++++++++--
tests/qemu-iotests/130.out | 4 ++--
tests/qemu-iotests/130.pc.out | 43
+++++++++++++++++++++++++++++++++++++++++++
3 files changed, 56 insertions(+), 4 deletions(-)
create mode 100644 tests/qemu-iotests/130.pc.out
diff --git a/tests/qemu-iotests/130 b/tests/qemu-iotests/130
index bc26247..de40c7b 100755
--- a/tests/qemu-iotests/130
+++ b/tests/qemu-iotests/130
@@ -58,9 +58,18 @@ echo "=== HMP commit ==="
echo
# bdrv_make_empty() involves a header update for qcow2
+case "$QEMU_DEFAULT_MACHINE" in
+ pc)
+ device_id="ide0-hd0"
+ ;;
+ s390)
+ device_id="virtio0"
+ ;;
I think I mentioned before that I don't really like not taking the fact
into account that there are other machine types, too. I'm still
accepting it based on the fact that I think those machine types won't
pass the tests right now anyway, so not caring for them in these case
blocks won't break any tests, but it still feels like something we can
avoid (like defaulting to virtio0 for any non-pc platform).
Anyway, because I seem to remember I accepted it before:
With the commit title fixed:
Reviewed-by: Max Reitz <mre...@redhat.com>
I guess you discussed with Xiao Guang Chen and accepted it in "[PATCH RFC
v5 6/7] qemu-iotests s390x fix test-051", because test 130 and 051 are
using the same fix solution, and test 051 was fixed in v5. Seeing section
of v5 in cover letter as below:
Indeed we discussed it. Just for clarification, I disliked having only cases
for "pc" and "s390" -- there are other platforms, too, which will simply break
by not including them in these case statements. We could try to avoid this by
defaulting to virtio0 for every non-pc platform, and it will probably work for
most without having to do further work here.
However, I did accept it because all those non-PC (and non-s390) platforms
won't pass the tests before this patch set either (because these test cases try
to use IDE devices which will not be available there). So the series will not
break them because they didn't work before either.
Bottom line: I'm fine with this solution as it is.
Maybe I'm missing the obvious, but why don't you just specify an
explicit ID instead of guessing the default ID that qemu will use
depending on the platform?
Please forgive me that I'm very sure about the meaning of "default ID"
you mentioned. Maybe you mean "default device ID"? If I'm wrong, please
correct me :-)
The default device id of hard disk on the s390 platform differs to the
device id on the x86 platform, so we need to use different device id
for different platform. For instance, using "virtio0" for s390x, and
using "ide0-hd0" for x86 as below:
/+_send_qemu_cmd $QEMU_HANDLE "commit " *"virtio0"* "(qemu)"//
//+_send_qemu_cmd $QEMU_HANDLE "commit " *"ide0-hd0"* "(qemu)"/
Kevin
