Re: [Qemu-devel] [PATCH v4 2/3] qemu-iotests: s390x: fix test 051

[Bo Tu <tubo@linux.vnet.ibm.com>]

Hi Max:

在 2015/12/5 5:21, Max Reitz 写道:
> On 03.12.2015 11:01, Bo Tu wrote:
>> From: Bo Tu <tubo@linux.vnet.ibm.com>
>>
>> The tests for ide device should only be tested for the pc
>> platform.
>> Set device_id to "drive0", and replace every "-drive file..."
>> by "-drive file=...,if=none,id=$device_id", then x86 and s390x
>> can get the common output in the test of "Snapshot mode".
>> Warning message expected for s390x when drive without device.
>> A x86 platform specific output file is also needed.
>>
>> Reviewed-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
>> Signed-off-by: Bo Tu <tubo@linux.vnet.ibm.com>
>> ---
>>   tests/qemu-iotests/051        |  95 ++++++----
>>   tests/qemu-iotests/051.out    | 163 ++++------------
>>   tests/qemu-iotests/051.pc.out | 422 ++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 521 insertions(+), 159 deletions(-)
>>   create mode 100644 tests/qemu-iotests/051.pc.out
>
> I like the overall design now, but I think we can do more still.
>
> As evidenced by changes in the reference output from "ide0-hd0" to
> "virtio0", there are still more places which are missing an explicit if
> (like if=none) and id in 051 and are thus currently subject to whatever
> the target platform uses as the default (which happens to be
> virtio/virtio0 for s390). I think all of the places in 051 which do not
> specify an interface and ID should do so (i.e. if=none,id=drive0).
>
> Next, there are the warnings regarding orphaned drives. I'm only just
> now asking myself the question where they really come from, and I guess
> it's mostly the fact that we are trying to use if=ide, if=floppy and
> if=scsi which do not seem to be supported by s390. I guess we should
> simply not do any of the tests which use any of these interfaces (or,
> phrased differently, only do tests which use if=none or if=virtio).
>
> I'd be willing to merge this patch now anyway, though, as long as there
> will be a follow-up patch addressing these issues, if you agree.

I will create a follow-up patch to address these issues you mentioned. 
thanks a lot for your review!! Have a good weekend.

>
> Max
>