From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57906) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ymh3f-00028a-Lm for qemu-devel@nongnu.org; Mon, 27 Apr 2015 07:18:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Ymh3b-0000Xa-Ji for qemu-devel@nongnu.org; Mon, 27 Apr 2015 07:18:39 -0400 Received: from mx1.redhat.com ([209.132.183.28]:44246) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ymh3b-0000XT-9I for qemu-devel@nongnu.org; Mon, 27 Apr 2015 07:18:35 -0400 Message-ID: <553E1B03.7030803@redhat.com> Date: Mon, 27 Apr 2015 13:18:27 +0200 From: Max Reitz MIME-Version: 1.0 References: <1429756938-17186-1-git-send-email-chenxg@linux.vnet.ibm.com> <1429756938-17186-7-git-send-email-chenxg@linux.vnet.ibm.com> <55392528.1080804@redhat.com> <553DB395.5080308@linux.vnet.ibm.com> In-Reply-To: <553DB395.5080308@linux.vnet.ibm.com> Content-Type: text/plain; charset=iso-8859-15; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH RFC v7 6/7] qemu-iotests: s390x: fix test 051 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: tu bo , Xiao Guang Chen , qemu-devel@nongnu.org Cc: kwolf@redhat.com, armbru@redhat.com, mimu@linux.vnet.ibm.com 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 >>> Reviewed-by: Michael Mueller >>> Signed-off-by: Xiao Guang Chen >>> --- >>> 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