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

[tu bo <tubo@linux.vnet.ibm.com>]

Hi Max, Sascha:

On 11/21/2015 12:24 AM, Max Reitz wrote:
> On 19.11.2015 08:28, tu bo wrote:
>> Hi Max:
>>
>> On 11/19/2015 12:52 AM, Max Reitz wrote:
>>> On 04.11.2015 03:26, Bo Tu wrote:
>>>> 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: Sascha Silbe <silbe@linux.vnet.ibm.com>
>>>> Signed-off-by: Bo Tu <tubo@linux.vnet.ibm.com>
>>>> ---
>>>>    tests/qemu-iotests/051        |  99 ++++++----
>>>>    tests/qemu-iotests/051.out    | 143 +++-----------
>>>>    tests/qemu-iotests/051.pc.out | 422
>>>> ++++++++++++++++++++++++++++++++++++++++++
>>>>    3 files changed, 515 insertions(+), 149 deletions(-)
>>>>    create mode 100644 tests/qemu-iotests/051.pc.out
>>>>
>>>
>>> [...]
>>>
>>>> diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out
>>>> index 7765aa0..d17c969 100644
>>>> --- a/tests/qemu-iotests/051.out
>>>> +++ b/tests/qemu-iotests/051.out
>>>
>>> [...]
>>>
>>>> @@ -109,20 +109,23 @@ QEMU X.Y.Z monitor - type 'help' for more
>>>> information
>>>>
>>>>    Testing: -drive if=floppy
>>>>    QEMU X.Y.Z monitor - type 'help' for more information
>>>> -(qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
>>>> +(qemu) Warning: Orphaned drive without device:
>>>> id=floppy0,file=,if=floppy,bus=0,unit=0
>>>> +q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
>>>
>>> I'd still like these warnings to be filtered out (as I wrote in my reply
>>> to the original RFC's v4, and as was done in later versions of that RFC
>>> (the _filter_orphan function e.g. in
>>> http://lists.nongnu.org/archive/html/qemu-devel/2015-05/msg04816.html).
>>>
>>
>> I did more investigation about this.
>> "Warning: Orphaned drive without device:.*"  is expected for s390x, when
>> we define a drive without device, also this drive is not default
>> and interface is not "none".  Please refer line 228,229 in blockdev.c:
>>      218 bool drive_check_orphaned(void)
>>      219 {
>>      220     BlockBackend *blk;
>>      221     DriveInfo *dinfo;
>>      222     bool rs = false;
>>      223
>>      224     for (blk = blk_next(NULL); blk; blk = blk_next(blk)) {
>>      225         dinfo = blk_legacy_dinfo(blk);
>>      226         /* If dinfo->bdrv->dev is NULL, it has no device
>> attached. */
>>      227         /* Unless this is a default drive, this may be an
>> oversight. */
>>      228         if (!blk_get_attached_dev(blk) && !dinfo->is_default &&
>>      229             dinfo->type != IF_NONE) {
>>      230             fprintf(stderr, "Warning: Orphaned drive without
>> device: "
>>      231                     "id=%s,file=%s,if=%s,bus=%d,unit=%d\n",
>>      232                     blk_name(blk), blk_bs(blk)->filename,
>> if_name[dinfo->type],
>>      233                     dinfo->bus, dinfo->unit);
>>      234             rs = true;
>>      235         }
>>      236     }
>>
>> For example,  "run_qemu -drive if=scsi"  will generate orphan warning
>> message for s390x,  but "run_qemu -drive if=virtio"  will not generate
>> orphan warning message,  since virtio is a default drive.
>>
>> so I removed  _filter_orphan() in common.filter,  and added orphan
>> warning message(ie: for the output of "run_qemu -drive if=scsi") in
>> output file for s390x.   thanks
>
> OK, so it is expected for s390x; however, this is strictly speaking not
> the output file for s390x but for any platform but PC. That's why I'd
> rather not have it in this “generic” reference output.
>
> This patch already assumes that the iotests only support s390 and PC
> (hunk @@ -251,28 +273,37 @@ in 051 adds a case statement which knows
> only these two cases). Therefore, I would be fine with renaming this
> reference output file to "051.s390.out" with a copy
> "051.s390-ccw-virtio.out" and just removing the generic "051.out". Then
> you can keep the warnings in.
>

It looks good to me. thanks

Hi Sascha:
This change is to rename 051.out to "051.s390.out" with a copy
"051.s390-ccw-virtio.out" and just to remove the generic "051.out". Is 
this fine to you?  thanks

> (Or you filter the warnings out and keep it as "051.out".)
>
> Max
>