From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47908) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a4QVq-0004Y8-8y for qemu-devel@nongnu.org; Thu, 03 Dec 2015 04:49:20 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a4QVm-0002yr-Vy for qemu-devel@nongnu.org; Thu, 03 Dec 2015 04:49:18 -0500 Received: from e18.ny.us.ibm.com ([129.33.205.208]:47859) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a4QVm-0002yg-Sx for qemu-devel@nongnu.org; Thu, 03 Dec 2015 04:49:14 -0500 Received: from localhost by e18.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 3 Dec 2015 04:49:13 -0500 Received: from b01cxnp23032.gho.pok.ibm.com (b01cxnp23032.gho.pok.ibm.com [9.57.198.27]) by d01dlp02.pok.ibm.com (Postfix) with ESMTP id 40CD36E8040 for ; Thu, 3 Dec 2015 04:37:21 -0500 (EST) Received: from d01av02.pok.ibm.com (d01av02.pok.ibm.com [9.56.224.216]) by b01cxnp23032.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id tB39nB3W25821364 for ; Thu, 3 Dec 2015 09:49:11 GMT Received: from d01av02.pok.ibm.com (localhost [127.0.0.1]) by d01av02.pok.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id tB39nBKg021720 for ; Thu, 3 Dec 2015 04:49:11 -0500 References: <1448531594-27192-1-git-send-email-tubo@linux.vnet.ibm.com> <1448531594-27192-3-git-send-email-tubo@linux.vnet.ibm.com> <565CA5C0.1000800@redhat.com> <565D4DC0.7070402@linux.vnet.ibm.com> <565F12E6.2090803@redhat.com> From: tu bo Message-ID: <56601014.30800@linux.vnet.ibm.com> Date: Thu, 3 Dec 2015 17:49:08 +0800 MIME-Version: 1.0 In-Reply-To: <565F12E6.2090803@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH v3 2/3] qemu-iotests: s390x: fix test 051 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz , qemu-devel@nongnu.org Cc: kwolf@redhat.com, silbe@linux.vnet.ibm.com, armbru@redhat.com, mimu@linux.vnet.ibm.com Hi Max: On 12/02/2015 11:48 PM, Max Reitz wrote: > 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 >>>> >>>> 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 >>> >>> 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 >>>> Signed-off-by: Bo Tu >>>> --- >>>> 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 prefer to keep the warning in 051.out. > >> 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. Sure, I'll put the output into 051.out for any non-pc platform, and remove 051.s390.out(and rollback the changes in Makefile). thanks > > Max >