On 29/07/2020 10.54, Cornelia Huck wrote: > On Tue, 28 Jul 2020 20:37:31 +0200 > Thomas Huth <th...@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 <th...@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) > > check_subchannel()? You verify dev_no as well, if supplied.
Ok. >> +{ >> + 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; > > -ENODEV? It means that you either have no devices, or an invalid > subchannel set. We don't have ENODEV in the s390-ccw bios... but I could introduce it, I guess :-) >> + } >> + 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 */ > > "just return whether the device is possibly bootable" ? That makes more sense now, indeed. >> + 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; >> + } >> + } >> + return false; >> + case CU_TYPE_DASD_3990: >> + case CU_TYPE_DASD_2107: >> + return true; >> + default: >> + return false; >> + } >> + } >> + >> + /* Caller asked for a specific devno */ >> + if (schib.pmcw.dev == dev_no) { >> + return true; >> + } >> + >> + return false; >> +} >> + >> /* >> * Find the subchannel connected to the given device (dev_no) and fill in >> the >> * subchannel information block (schib) with the connected subchannel's >> info. > (...) > > Otherwise, LGTM. Thanks! Thomas