[SLOF] [PATCH v4] slof/fs/packages/disk-label.fs: improve checking for DOS boot partitions

[Alexey Kardashevskiy]

First, sorry I am late into the discussion. Comments below.


On Thu, 28 Mar 2024, at 17:00, Kautuk Consul wrote:
> While testing with a qcow2 with a DOS boot partition it was found that
> when we set the logical_block_size in the guest XML to >512 then the
> boot would fail in the following interminable loop:

Why would anyone tweak this? And when you do, what happens inside the SLOF, does it keep using 512?

> <SNIP>
> Trying to load:  from: /pci at 800000020000000/scsi at 3 ...
> virtioblk_transfer: Access beyond end of device!
> virtioblk_transfer: Access beyond end of device!
> virtioblk_transfer: Access beyond end of device!
> virtioblk_transfer: Access beyond end of device!
> virtioblk_transfer: Access beyond end of device!
> virtioblk_transfer: Access beyond end of device!
> virtioblk_transfer: Access beyond end of device!
> virtioblk_transfer: Access beyond end of device!
> virtioblk_transfer: Access beyond end of device!
> virtioblk_transfer: Access beyond end of device!
> virtioblk_transfer: Access beyond end of device!
> virtioblk_transfer: Access beyond end of device!
> virtioblk_transfer: Access beyond end of device!
> virtioblk_transfer: Access beyond end of device!
> virtioblk_transfer: Access beyond end of device!
> virtioblk_transfer: Access beyond end of device!
> virtioblk_transfer: Access beyond end of device!
> </SNIP>
> 
> Change the "read-sector" Forth subroutine to throw an exception whenever
> it fails to read a full block-size length of sector from the disk.

Why not throwing an exception from the "beyond end" message point?
Or fail to open a device if SLOF does not like the block size? I forgot the internals :(

> Also change the "open" method to initiate CATCH exception handling for the calls to
> try-partitions/try-files which will also call read-sector which could potentially
> now throw this new exception.
> 
> After making the above changes, it fails properly with the correct error
> message as follows:
> <SNIP>
> Trying to load:  from: /pci at 800000020000000/scsi at 3 ...
> virtioblk_transfer: Access beyond end of device!
> virtioblk_transfer: Access beyond end of device!
> virtioblk_transfer: Access beyond end of device!
> virtioblk_transfer: Access beyond end of device!
> virtioblk_transfer: Access beyond end of device!
> 
> E3404: Not a bootable device!
> 
> E3407: Load failed
> 
>   Type 'boot' and press return to continue booting the system.
>   Type 'reset-all' and press return to reboot the system.
> 
> Ready!
> 0 >
> </SNIP>
> 
> Signed-off-by: Kautuk Consul <kconsul at linux.ibm.com>
> ---
> slof/fs/packages/disk-label.fs | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/slof/fs/packages/disk-label.fs b/slof/fs/packages/disk-label.fs
> index 661c6b0..a6fb231 100644
> --- a/slof/fs/packages/disk-label.fs
> +++ b/slof/fs/packages/disk-label.fs
> @@ -136,7 +136,8 @@ CONSTANT /gpt-part-entry
> : read-sector ( sector-number -- )
>     \ block-size is 0x200 on disks, 0x800 on cdrom drives
>     block-size * 0 seek drop      \ seek to sector
> -   block block-size read drop    \ read sector
> +   block block-size read         \ read sector
> +   block-size < IF throw THEN    \ if we read less than the block-size then throw an exception

When it fails, is the number of bytes ever non zero? Thanks,

> ;
>  
> : (.part-entry) ( part-entry )
> @@ -723,10 +724,15 @@ CREATE GPT-LINUX-PARTITION 10 allot
>     THEN
>  
>     partition IF
> -       try-partitions
> +       ['] try-partitions
>     ELSE
> -       try-files
> +       ['] try-files
>     THEN
> +
> +   \ Catch any exception that might happen due to read-sector failing to read
> +   \ block-size number of bytes from any sector of the disk.
> +   CATCH IF false THEN
> +
>     dup 0= IF debug-disk-label? IF ." not found." cr THEN close THEN \ free memory again
> ;
>  
> -- 
> 2.31.1
> 
>