[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: tu bo
Subject: Re: [Qemu-devel] [PATCH RFC v7 7/7] qemu-iotests-s390x-fix-test-130
Date: Mon, 04 May 2015 14:30:51 +0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0

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 

             You're right. I will change it in the next version :-)

                 On 23.04.2015 04:42, Xiao Guang Chen wrote:

                     From: Bo Tu <address@hidden>

                     The tests for device type "ide_cd" should only be tested 
for the pc
                     The default device id of hard disk on the s390 platform 
differs to
                     of the x86 platform. A new variable device_id is defined 
                     set for the s390 platform. A x86 platform specific output 
file is

                     Signed-off-by: Bo Tu <address@hidden>
                       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 
                     index bc26247..de40c7b 100755
                     --- a/tests/qemu-iotests/130
                     +++ b/tests/qemu-iotests/130
                     @@ -58,9 +58,18 @@ echo "=== HMP commit ==="
                       # 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 <address@hidden>

             I guess you discussed with Xiao Guang Chen and accepted it in 
             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 
             of v5 in cover letter as below:

         Indeed we discussed it. Just for clarification, I disliked having only 
         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) 
         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.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]