From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42338) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YlKbD-0001pg-GA for qemu-devel@nongnu.org; Thu, 23 Apr 2015 13:07:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YlKbA-00035e-4K for qemu-devel@nongnu.org; Thu, 23 Apr 2015 13:07:39 -0400 Received: from mx1.redhat.com ([209.132.183.28]:56019) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YlKb9-00035U-SC for qemu-devel@nongnu.org; Thu, 23 Apr 2015 13:07:36 -0400 Message-ID: <553926D3.9010904@redhat.com> Date: Thu, 23 Apr 2015 19:07:31 +0200 From: Max Reitz MIME-Version: 1.0 References: <1429756938-17186-1-git-send-email-chenxg@linux.vnet.ibm.com> <1429756938-17186-8-git-send-email-chenxg@linux.vnet.ibm.com> In-Reply-To: <1429756938-17186-8-git-send-email-chenxg@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 7/7] qemu-iotests-s390x-fix-test-130 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Xiao Guang Chen , qemu-devel@nongnu.org Cc: kwolf@redhat.com, armbru@redhat.com, mimu@linux.vnet.ibm.com Well, that's a peculiar commit title. :-) I guess it's supposed to be "qemu-iotests: s390x: fix test 130"? On 23.04.2015 04:42, Xiao Guang Chen wrote: > From: Bo Tu > > 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 > --- > 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 > +esac > + > # Test that a backing file isn't written > _launch_qemu -drive file="$TEST_IMG",backing.file.filename="$TEST_IMG.base" > -_send_qemu_cmd $QEMU_HANDLE "commit ide0-hd0" "(qemu)" > +_send_qemu_cmd $QEMU_HANDLE "commit " $device_id "(qemu)" ("commit $device_id" "(qemu)" would've worked, too, but it's fine the way _send_qemu_cmd works) > _send_qemu_cmd $QEMU_HANDLE '' '(qemu)' > _cleanup_qemu > _img_info | _filter_img_info > @@ -69,7 +78,7 @@ _img_info | _filter_img_info > # command line, that backing file is retained, with the right format > _make_test_img -F raw -b "$TEST_IMG.orig" 64M > _launch_qemu -drive file="$TEST_IMG",backing.file.filename="$TEST_IMG.base",backing.driver=$IMGFMT > -_send_qemu_cmd $QEMU_HANDLE "commit ide0-hd0" "(qemu)" > +_send_qemu_cmd $QEMU_HANDLE "commit " $device_id "(qemu)" > _send_qemu_cmd $QEMU_HANDLE '' '(qemu)' > _cleanup_qemu > _img_info | _filter_img_info > diff --git a/tests/qemu-iotests/130.out b/tests/qemu-iotests/130.out > index ea68b5d..cc4a15a 100644 > --- a/tests/qemu-iotests/130.out > +++ b/tests/qemu-iotests/130.out > @@ -9,14 +9,14 @@ virtual size: 64M (67108864 bytes) > === HMP commit === > > QEMU X.Y.Z monitor - type 'help' for more information > -(qemu) ccocomcommcommicommitcommit commit icommit idcommit idecommit ide0commit ide0-commit ide0-hcommit ide0-hdcommit ide0-hd0 > +(qemu) ccocomcommcommicommitcommit commit vcommit vicommit vircommit virtcommit virticommit virtiocommit virtio0 > (qemu) > image: TEST_DIR/t.IMGFMT > file format: IMGFMT > virtual size: 64M (67108864 bytes) > Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 backing_file='TEST_DIR/t.IMGFMT.orig' backing_fmt='raw' > QEMU X.Y.Z monitor - type 'help' for more information > -(qemu) ccocomcommcommicommitcommit commit icommit idcommit idecommit ide0commit ide0-commit ide0-hcommit ide0-hdcommit ide0-hd0 > +(qemu) ccocomcommcommicommitcommit commit vcommit vicommit vircommit virtcommit virticommit virtiocommit virtio0 > (qemu) > image: TEST_DIR/t.IMGFMT > file format: IMGFMT > diff --git a/tests/qemu-iotests/130.pc.out b/tests/qemu-iotests/130.pc.out > new file mode 100644 > index 0000000..ea68b5d > --- /dev/null > +++ b/tests/qemu-iotests/130.pc.out > @@ -0,0 +1,43 @@ > +QA output created by 130 > +Formatting 'TEST_DIR/t.IMGFMT.orig', fmt=IMGFMT size=67108864 > +Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=67108864 > +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 > +image: TEST_DIR/t.IMGFMT > +file format: IMGFMT > +virtual size: 64M (67108864 bytes) > + > +=== HMP commit === > + > +QEMU X.Y.Z monitor - type 'help' for more information > +(qemu) ccocomcommcommicommitcommit commit icommit idcommit idecommit ide0commit ide0-commit ide0-hcommit ide0-hdcommit ide0-hd0 > +(qemu) > +image: TEST_DIR/t.IMGFMT > +file format: IMGFMT > +virtual size: 64M (67108864 bytes) > +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 backing_file='TEST_DIR/t.IMGFMT.orig' backing_fmt='raw' > +QEMU X.Y.Z monitor - type 'help' for more information > +(qemu) ccocomcommcommicommitcommit commit icommit idcommit idecommit ide0commit ide0-commit ide0-hcommit ide0-hdcommit ide0-hd0 > +(qemu) > +image: TEST_DIR/t.IMGFMT > +file format: IMGFMT > +virtual size: 64M (67108864 bytes) > +backing file: TEST_DIR/t.IMGFMT.orig > +backing file format: raw > + > +=== Marking image dirty (lazy refcounts) === > + > +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 > +wrote 4096/4096 bytes at offset 0 > +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) > +image: TEST_DIR/t.IMGFMT > +file format: IMGFMT > +virtual size: 64M (67108864 bytes) > +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 backing_file='TEST_DIR/t.IMGFMT.orig' backing_fmt='raw' > +wrote 4096/4096 bytes at offset 0 > +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) > +image: TEST_DIR/t.IMGFMT > +file format: IMGFMT > +virtual size: 64M (67108864 bytes) > +backing file: TEST_DIR/t.IMGFMT.orig > +backing file format: raw > +*** done