From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35349) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yp9uc-0000xv-MJ for qemu-devel@nongnu.org; Mon, 04 May 2015 02:31:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Yp9uX-0000Dj-Jt for qemu-devel@nongnu.org; Mon, 04 May 2015 02:31:30 -0400 Received: from e28smtp03.in.ibm.com ([122.248.162.3]:38018) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yp9uX-0000DM-0T for qemu-devel@nongnu.org; Mon, 04 May 2015 02:31:25 -0400 Received: from /spool/local by e28smtp03.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 4 May 2015 12:01:17 +0530 Received: from d28relay02.in.ibm.com (d28relay02.in.ibm.com [9.184.220.59]) by d28dlp02.in.ibm.com (Postfix) with ESMTP id 982673940063 for ; Mon, 4 May 2015 12:01:14 +0530 (IST) Received: from d28av02.in.ibm.com (d28av02.in.ibm.com [9.184.220.64]) by d28relay02.in.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id t446Uten3473646 for ; Mon, 4 May 2015 12:00:55 +0530 Received: from d28av02.in.ibm.com (localhost [127.0.0.1]) by d28av02.in.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id t445wcbH032209 for ; Mon, 4 May 2015 11:28:38 +0530 Message-ID: <5547121B.5070709@linux.vnet.ibm.com> Date: Mon, 04 May 2015 14:30:51 +0800 From: tu bo 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> <553926D3.9010904@redhat.com> <553DE1FC.6040000@linux.vnet.ibm.com> <553E1C56.8020901@redhat.com> <20150427113459.GE4046@noname.str.redhat.com> <553EF7A7.7000102@linux.vnet.ibm.com> <20150428082322.GA4378@noname.redhat.com> In-Reply-To: <20150428082322.GA4378@noname.redhat.com> Content-Type: text/plain; charset=windows-1252; 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: Kevin Wolf Cc: qemu-devel@nongnu.org, Max Reitz , armbru@redhat.com, mimu@linux.vnet.ibm.com Hi Kevin: On 04/28/2015 04:23 PM, Kevin Wolf wrote: > Am 28.04.2015 um 04:59 hat tu bo geschrieben: >> 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 >> >> 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 >> >> 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)" > Any why not use this? > > qemu -drive id=testdisk,... > (qemu) commit testdisk I tried and it works fine, which is a more easier solution for test 130. I'll use your solution in patch v8. thanks a lot > > That would be the same on all platforms. > > Kevin >