Re: [qemu-s390x] [PATCH v3 05/16] s390-bios: Factor finding boot device out of virtio code path

[Cornelia Huck]

On Mon, 4 Mar 2019 20:26:17 +0100
Thomas Huth <address@hidden> wrote:

> On 04/03/2019 18.07, Cornelia Huck wrote:
> > On Fri,  1 Mar 2019 13:59:25 -0500
> > "Jason J. Herne" <address@hidden> wrote:
> >   
> >> Make a new routine find_boot_device to locate the boot device for all
> >> cases, not just virtio.
> >>
> >> The error message for the case where no boot device has been specified
> >> and a suitable boot device cannot be auto detected was specific to
> >> virtio devices. We update this message to remove virtio specific wording.  
> > 
> > ...and, consequently, need to tweak the serial output boot test.
> >   
> >>
> >> Signed-off-by: Jason J. Herne <address@hidden>
> >> Reviewed-by: Farhan Ali <address@hidden>
> >> ---
> >>  pc-bios/s390-ccw/main.c  | 85 
> >> ++++++++++++++++++++++++++----------------------
> >>  tests/boot-serial-test.c |  2 +-
> >>  2 files changed, 47 insertions(+), 40 deletions(-)
> >>  
> > (...)  
> >> -    IPL_assert(found, "No virtio device found");
> >> +    IPL_assert(found, "Boot device not found\n");  
> > 
> > You change the output in the bios here...
> > 
> > (...)  
> >> diff --git a/tests/boot-serial-test.c b/tests/boot-serial-test.c
> >> index 58a48f3..9daf2cb 100644
> >> --- a/tests/boot-serial-test.c
> >> +++ b/tests/boot-serial-test.c
> >> @@ -112,7 +112,7 @@ static testdef_t tests[] = {
> >>      { "sparc", "SS-4", "", "MB86904" },
> >>      { "sparc", "SS-600MP", "", "TMS390Z55" },
> >>      { "sparc64", "sun4u", "", "UltraSPARC" },
> >> -    { "s390x", "s390-ccw-virtio", "", "virtio device" },
> >> +    { "s390x", "s390-ccw-virtio", "", "device" },  
> > 
> > ...and therefore need to change the expected string here. Question: is
> > that ok ("device" looks a bit generic), or would "Boot device" be a
> > better match?  
> 
> Just "device" should be OK. The exact string in the boot-serial-test
> does not matter too much, as long as it comes from the same print statement.

Ok, makes sense.

Reviewed-by: Cornelia Huck <address@hidden>