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

[tu bo]

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 <address@hidden>
> > > > Signed-off-by: Bo Tu <address@hidden>
> > > > ---
> > > >    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) qququiquit
> > > > +(qemu) Warning: Orphaned drive without device:
> > > > id=floppy0,file=,if=floppy,bus=0,unit=0
> > > > +qququiquit
> > >
> > > 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