Re: [PATCH v3 07/19] pc-bios/s390-ccw: Remove panics from ISO IPL path

[Jared Rossi]

On 10/9/24 5:46 AM, Thomas Huth wrote:
> On 08/10/2024 03.15, jrossi@linux.ibm.com wrote:
> > From: Jared Rossi <jrossi@linux.ibm.com>
> >
> > Remove panic-on-error from IPL ISO El Torito specific functions so 
> > that error
> > recovery may be possible in the future.
> >
> > Functions that would previously panic now provide a return code.
> >
> > Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
> > ---
> ...
> > diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c
> > index 414c3f1b47..7984de62fe 100644
> > --- a/pc-bios/s390-ccw/bootmap.c
> > +++ b/pc-bios/s390-ccw/bootmap.c
> > @@ -678,8 +678,10 @@ static bool 
> > is_iso_bc_entry_compatible(IsoBcSection *s)
> >       if (s->unused || !s->sector_count) {
> >           return false;
> >       }
> > -    read_iso_sector(bswap32(s->load_rba), magic_sec,
> > -                    "Failed to read image sector 0");
> > +    if (virtio_read(bswap32(s->load_rba), magic_sec)) {
> > +        puts("Failed to read image sector 0");
> > +        return false;
> > +    }
> >         /* Checking bytes 8 - 32 for S390 Linux magic */
> >       return !memcmp(magic_sec + 8, linux_s390_magic, 24);
> > @@ -699,21 +701,28 @@ static inline uint32_t 
> > iso_get_file_size(uint32_t load_rba)
> >       uint8_t *temp = sec + ISO_SECTOR_SIZE;
> >       int level = 0;
> >   -    read_iso_sector(ISO_PRIMARY_VD_SECTOR, sec,
> > -                    "Failed to read ISO primary descriptor");
> > +    if (virtio_read(ISO_PRIMARY_VD_SECTOR, sec)) {
> > +        puts("Failed to read ISO primary descriptor");
> > +        return -EIO;
> > +    }
>
> iso_get_file_size() seems to return an uint32_t value, so returning a 
> negative error code won't work here. I think you either have to change 
> the return type to signed "long" and fix the caller site, too, or 
> return 0 for errors here?
>
>  Thomas

Fixed by using "long" as suggested.  Also found and fixed an error with 
real_size < 0 not being handled correctly in a later part of the code 
that used "if (real_size)"

    Jared