Re: [PATCH for-5.2 3/6] pc-bios/s390-ccw: Move the inner logic of find_subch() to a separate function

[Claudio Imbrenda]

On Tue, 4 Aug 2020 15:24:09 +0200
Thomas Huth <thuth@redhat.com> wrote:

> On 03/08/2020 10.46, Claudio Imbrenda wrote:
> > On Tue, 28 Jul 2020 20:37:31 +0200
> > Thomas Huth <thuth@redhat.com> wrote:
> >   
> >> Move the code to a separate function to be able to re-use it from a
> >> different spot later.
> >>
> >> Signed-off-by: Thomas Huth <thuth@redhat.com>
> >> ---
> >>  pc-bios/s390-ccw/main.c | 99
> >> ++++++++++++++++++++++++----------------- 1 file changed, 57
> >> insertions(+), 42 deletions(-)
> >>
> >> diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
> >> index 9b64eb0c24..9477313188 100644
> >> --- a/pc-bios/s390-ccw/main.c
> >> +++ b/pc-bios/s390-ccw/main.c
> >> @@ -51,6 +51,60 @@ unsigned int get_loadparm_index(void)
> >>      return atoui(loadparm_str);
> >>  }
> >>  
> >> +static int check_sch_no(int dev_no, int sch_no)
> >> +{
> >> +    bool is_virtio;
> >> +    Schib schib;
> >> +    int r;
> >> +
> >> +    blk_schid.sch_no = sch_no;
> >> +    r = stsch_err(blk_schid, &schib);
> >> +    if (r == 3 || r == -EIO) {
> >> +        return -EIO;
> >> +    }
> >> +    if (!schib.pmcw.dnv) {
> >> +        return false;
> >> +    }
> >> +
> >> +    enable_subchannel(blk_schid);
> >> +    cutype = cu_type(blk_schid);
> >> +
> >> +    /*
> >> +     * Note: we always have to run virtio_is_supported() here to
> >> make
> >> +     * sure that the vdev.senseid data gets pre-initialized
> >> correctly
> >> +     */
> >> +    is_virtio = virtio_is_supported(blk_schid);
> >> +
> >> +    /* No specific devno given, just return 1st possibly bootable
> >> device */
> >> +    if (dev_no < 0) {
> >> +        switch (cutype) {
> >> +        case CU_TYPE_VIRTIO:
> >> +            if (is_virtio) {
> >> +                /*
> >> +                 * Skip net devices since no IPLB is created and
> >> therefore
> >> +                 * no network bootloader has been loaded
> >> +                 */
> >> +                if (virtio_get_device_type() != VIRTIO_ID_NET) {
> >> +                    return true;
> >> +                }  
> > 
> > here it seems you are returning true for any non-network virtio
> > device, is this the intended behaviour? (I know it was like this in
> > the old code) like, non-block devices?  
> 
> Yes. Other devices are already ignored by the virtio_is_supported()
> call some lines earlier in this function.

ah, that makes sense


Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>