Re: [Qemu-devel] [PATCH RFC v7 6/7] qemu-iotests: s390x: fix test 051

Mon, 27 Apr 2015 04:20:03 -0700 

Hi Tu Bo :-)

On 27.04.2015 05:57, tu bo wrote:

Hi Max:

On 04/24/2015 01:00 AM, Max Reitz wrote:

On 23.04.2015 04:42, Xiao Guang Chen wrote:
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.
A new filter was added to filter orphan warnings.

Reviewed-by: Max Reitz <mre...@redhat.com>
Reviewed-by: Michael Mueller <m...@linux.vnet.ibm.com>
Signed-off-by: Xiao Guang Chen <che...@linux.vnet.ibm.com>
---
  tests/qemu-iotests/051           |  79 ++++---
  tests/qemu-iotests/051.out       | 156 +++++---------
tests/qemu-iotests/051.pc.out | 433 +++++++++++++++++++++++++++++++++++++++ 
  tests/qemu-iotests/common.filter |   7 +
  4 files changed, 545 insertions(+), 130 deletions(-)
  create mode 100644 tests/qemu-iotests/051.pc.out

diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051
index 0360f37..8de16a5 100755
--- a/tests/qemu-iotests/051
+++ b/tests/qemu-iotests/051
@@ -147,13 +147,19 @@ run_qemu -drive if=ide
  run_qemu -drive if=virtio
  run_qemu -drive if=scsi
  -run_qemu -drive if=none,id=disk -device ide-cd,drive=disk
-run_qemu -drive if=none,id=disk -device lsi53c895a -device scsi-cd,drive=disk 
-
-run_qemu -drive if=none,id=disk -device ide-drive,drive=disk
-run_qemu -drive if=none,id=disk -device ide-hd,drive=disk
-run_qemu -drive if=none,id=disk -device lsi53c895a -device scsi-disk,drive=disk -run_qemu -drive if=none,id=disk -device lsi53c895a -device scsi-hd,drive=disk 
+case "$QEMU_DEFAULT_MACHINE" in
+    pc)
+        run_qemu -drive if=none,id=disk -device ide-cd,drive=disk
+ run_qemu -drive if=none,id=disk -device lsi53c895a -device scsi-cd,drive=disk 
+
+        run_qemu -drive if=none,id=disk -device ide-drive,drive=disk
+        run_qemu -drive if=none,id=disk -device ide-hd,drive=disk
+ run_qemu -drive if=none,id=disk -device lsi53c895a -device scsi-disk,drive=disk + run_qemu -drive if=none,id=disk -device lsi53c895a -device scsi-hd,drive=disk 
+        ;;
+    *)
+        ;;
+esac
    echo
  echo === Read-only ===
@@ -167,13 +173,19 @@ run_qemu -drive file="$TEST_IMG",if=ide,readonly=on 
  run_qemu -drive file="$TEST_IMG",if=virtio,readonly=on
  run_qemu -drive file="$TEST_IMG",if=scsi,readonly=on
-run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on -device ide-cd,drive=disk -run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on -device lsi53c895a -device scsi-cd,drive=disk 
+case "$QEMU_DEFAULT_MACHINE" in
+    pc)
+ run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on -device ide-cd,drive=disk + run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on -device lsi53c895a -device scsi-cd,drive=disk -run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on -device ide-drive,drive=disk -run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on -device ide-hd,drive=disk -run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on -device lsi53c895a -device scsi-disk,drive=disk -run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on -device lsi53c895a -device scsi-hd,drive=disk + run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on -device ide-drive,drive=disk + run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on -device ide-hd,drive=disk + run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on -device lsi53c895a -device scsi-disk,drive=disk + run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on -device lsi53c895a -device scsi-hd,drive=disk 
+        ;;
+    *)
+        ;;
+esac
    echo
  echo === Cache modes ===
@@ -182,12 +194,12 @@ echo
# Cannot use the test image because cache=none might not work on the host FS 
  # Use cdrom so that we won't get errors about missing media
  -run_qemu -drive media=cdrom,cache=none
-run_qemu -drive media=cdrom,cache=directsync
-run_qemu -drive media=cdrom,cache=writeback
-run_qemu -drive media=cdrom,cache=writethrough
-run_qemu -drive media=cdrom,cache=unsafe
-run_qemu -drive media=cdrom,cache=invalid_value
+run_qemu -drive if=scsi,media=cdrom,cache=none
+run_qemu -drive if=scsi,media=cdrom,cache=directsync
+run_qemu -drive if=scsi,media=cdrom,cache=writeback
+run_qemu -drive if=scsi,media=cdrom,cache=writethrough
+run_qemu -drive if=scsi,media=cdrom,cache=unsafe
+run_qemu -drive if=scsi,media=cdrom,cache=invalid_value
    echo
  echo === Specifying the protocol layer ===
@@ -251,28 +263,37 @@ echo
  echo === Snapshot mode ===
  echo
  +case "$QEMU_DEFAULT_MACHINE" in
+    pc)
+        device_id="ide0-hd0"
+        ;;
+    s390)
+        device_id="virtio0"
+        ;;
+esac
+
  $QEMU_IO -c "write -P 0x11 0 4k" "$TEST_IMG" | _filter_qemu_io
-echo 'qemu-io ide0-hd0 "write -P 0x22 0 4k"' | run_qemu -drive file="$TEST_IMG" -snapshot | _filter_qemu_io -echo 'qemu-io ide0-hd0 "write -P 0x22 0 4k"' | run_qemu -drive file="$TEST_IMG",snapshot=on | _filter_qemu_io -echo 'qemu-io ide0-hd0 "write -P 0x22 0 4k"' | run_qemu -drive file.filename="$TEST_IMG",driver=qcow2,snapshot=on | _filter_qemu_io -echo 'qemu-io ide0-hd0 "write -P 0x22 0 4k"' | run_qemu -drive file.filename="$TEST_IMG",driver=qcow2 -snapshot | _filter_qemu_io -echo 'qemu-io ide0-hd0 "write -P 0x22 0 4k"' | run_qemu -drive file="file:$TEST_IMG" -snapshot | _filter_qemu_io -echo 'qemu-io ide0-hd0 "write -P 0x22 0 4k"' | run_qemu -drive file="file:$TEST_IMG",snapshot=on | _filter_qemu_io +echo "qemu-io $device_id \"write -P 0x22 0 4k\"" | run_qemu -drive file="$TEST_IMG" -snapshot | _filter_qemu_io +echo "qemu-io $device_id \"write -P 0x22 0 4k\"" | run_qemu -drive file="$TEST_IMG",snapshot=on | _filter_qemu_io +echo "qemu-io $device_id \"write -P 0x22 0 4k\"" | run_qemu -drive file.filename="$TEST_IMG",driver=qcow2,snapshot=on | _filter_qemu_io +echo "qemu-io $device_id \"write -P 0x22 0 4k\"" | run_qemu -drive file.filename="$TEST_IMG",driver=qcow2 -snapshot | _filter_qemu_io +echo "qemu-io $device_id \"write -P 0x22 0 4k\"" | run_qemu -drive file="file:$TEST_IMG" -snapshot | _filter_qemu_io +echo "qemu-io $device_id \"write -P 0x22 0 4k\"" | run_qemu -drive file="file:$TEST_IMG",snapshot=on | _filter_qemu_io 
    # Opening a read-only file r/w with snapshot=on
  chmod u-w "$TEST_IMG"
-echo 'qemu-io ide0-hd0 "write -P 0x22 0 4k"' | run_qemu -drive file="$TEST_IMG" -snapshot | _filter_qemu_io -echo 'qemu-io ide0-hd0 "write -P 0x22 0 4k"' | run_qemu -drive file="$TEST_IMG",snapshot=on | _filter_qemu_io +echo "qemu-io $device_id \"write -P 0x22 0 4k\"" | run_qemu -drive file="$TEST_IMG" -snapshot | _filter_qemu_io +echo "qemu-io $device_id \"write -P 0x22 0 4k\"" | run_qemu -drive file="$TEST_IMG",snapshot=on | _filter_qemu_io 
  chmod u+w "$TEST_IMG"
    $QEMU_IO -c "read -P 0x11 0 4k" "$TEST_IMG" | _filter_qemu_io
-echo 'qemu-io ide0-hd0 "write -P 0x22 0 4k"' | run_qemu -drive file="$TEST_IMG",snapshot=off | _filter_qemu_io +echo "qemu-io $device_id \"write -P 0x22 0 4k\"" | run_qemu -drive file="$TEST_IMG",snapshot=off | _filter_qemu_io 
    $QEMU_IO -c "read -P 0x22 0 4k" "$TEST_IMG" | _filter_qemu_io
-echo -e 'qemu-io ide0-hd0 "write -P 0x33 0 4k"\ncommit ide0-hd0' | run_qemu -drive file="$TEST_IMG",snapshot=on | _filter_qemu_io +echo -e "qemu-io $device_id \"write -P 0x33 0 4k\"\ncommit $device_id" | run_qemu -drive file="$TEST_IMG",snapshot=on | _filter_qemu_io 
    $QEMU_IO -c "read -P 0x33 0 4k" "$TEST_IMG" | _filter_qemu_io
  diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out
index 2890eac..63e0e81 100644
--- a/tests/qemu-iotests/051.out
+++ b/tests/qemu-iotests/051.out
@@ -5,16 +5,16 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file='TEST_DIR 
  === Unknown option ===
    Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=: Block format 'qcow2' used by device 'ide0-hd0' doesn't support the option 'unknown_opt' +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=: Block format 'qcow2' used by device 'virtio0' doesn't support the option 'unknown_opt' 
    Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=on
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=on: Block format 'qcow2' used by device 'ide0-hd0' doesn't support the option 'unknown_opt' +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=on: Block format 'qcow2' used by device 'virtio0' doesn't support the option 'unknown_opt' 
    Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=1234
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=1234: Block format 'qcow2' used by device 'ide0-hd0' doesn't support the option 'unknown_opt' +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=1234: Block format 'qcow2' used by device 'virtio0' doesn't support the option 'unknown_opt' 
    Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=foo
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=foo: Block format 'qcow2' used by device 'ide0-hd0' doesn't support the option 'unknown_opt' +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=foo: Block format 'qcow2' used by device 'virtio0' doesn't support the option 'unknown_opt' 
      === Unknown protocol option ===
@@ -51,8 +51,7 @@ QEMU_PROG: -drive file=TEST_DIR/t.qcow2,driver=qcow2,format=qcow2: Cannot specif 
    Testing: -device virtio-scsi-pci -device scsi-hd
  QEMU X.Y.Z monitor - type 'help' for more information
-(qemu) QEMU_PROG: -device scsi-hd: drive property not set
-QEMU_PROG: -device scsi-hd: Device 'scsi-hd' could not be initialized
+(qemu) QEMU_PROG: -device virtio-scsi-pci: No 'PCI' bus found for device 'virtio-scsi-pci'
This change looks like you'd rather just disable this test for non-PCI systems. Also, it's a change from v6 which forces me to remind you to please remove Reviewed-by statements from the commit message if there are functional changes in a patch from the previously reviewed version.
(Because just like in this case, the reviewer (i.e., me) may find some of these changes questionable, or might even outright object them.) 

Max
1. Sure. will remove Reviewed-by statements if any functional changes in a new patch version. thanks for the reminder 

Thank you :-)
2. Test 051 will be executed for PCI systems like x86, and test 051 will be executed for non-PCI systems like s390x as well. However, its golden test data is different(051.out is for s390x, 051.pc.out is for x86) Test steps are different between PCI systems and non-PCI systems. For instance, the tests for device type "ide_cd" should only be tested for the pc platform, and be disabled for non-PCI systems.
Right. So I think the same is the case here: A PCI device (virtio-scsi-pci) is created by the test, so it should be disabled for non-PCI systems. While testing against the reference output "No 'PCI' bus found" will probably not break on non-PCI systems, it doesn't seem like it's something we want to test here, though. 

Max

Reply via email to