search

ibm-s390-linux / s390-tools

Tools for use with the s390 Linux kernel and device drivers
MIT License
63 stars 59 forks source link

BLS error message for non BLS errors #84

Closed stefan-haberland closed 4 years ago

stefan-haberland commented 4 years ago

In some cases zipl throws a BLS error message for errors that are not BLS related. For example a wrong initrd is specified and leads to

Error: BLS parsing '/boot/loader/entries': No such file or directory

It should lead to the correct error message:

Error: Ramdisk file '/boot/initrd-missing' in section 'head': No such file or directory

stefan-haberland commented 4 years ago

@martinezjavier

maybe we can just remove the call to scan_check_bls() in job.c.

check_job_ipl_data() does the checking for file existence as well and provides the correct error message. Just without the "BLS parsing" prefix.

For example

missing file in zipl.conf:

./zipl
Using config file '/etc/zipl.conf'
Using BLS config file '/boot/loader/entries/477ab1bedca54e958a2805841b097235-5.0.9-301.fc30.s390x01.conf'
Error: Ramdisk file '/boot/initrd-missing' in section 'head': No such file or directory

missing file in a bls.conf:

./zipl
Using config file '/etc/zipl.conf'
Using BLS config file '/boot/loader/entries/477ab1bedca54e958a2805841b097235-5.0.9-301.fc30.s390x01.conf'
Error: Image file '/boot/vmlinuz-5.0.9-301.fc30.s390x' in section 'Fedora (5.0.9-301.fc30.s390x) 30 (Thirty)': No such file or directory

The user just has to look up where the section comes from. Which most likely is the last config that has been edited. For me this would be suitable.

What do you think?

sharkcz commented 4 years ago

@stefan-haberland, I like the idea, the "Error: BLS parsing '/boot/loader/entries': No such file or directory" message is often confusing.

martinezjavier commented 4 years ago

@martinezjavier

maybe we can just remove the call to scan_check_bls() in job.c.

Sorry for not answering this before, I missed the mention somehow...

This was added in commit d71628326d8 ("zipl: add value of target= as search path for BLS case") by @tuan-hoang1, so I would like to know his opinion to make sure that we don't regress his use case.

stefan-haberland commented 4 years ago

Thanks for the hint. I missed that I should have added @tuan-hoang1 as well. Will wait what he says.

tuan-hoang1 commented 4 years ago

My only intention of scan_check_bls() was to silently add the value of target= in zipl.conf, usually /boot string, into search paths of kernel and initrd fields in zipl.conf, if searching from / is not found.

At the time I thought of 2 places to do this: 1 is scan_check_bls(), and 2 is scan_bls_field().

IIRC in scan_bls_field() it was harder to get the value of target= as easy as in https://github.com/ibm-s390-tools/s390-tools/commit/d71628326d80e623fc9f008fe4ea93edb5592b2e#diff-fd10c873b0fe602d702a149a450b0a8eR1587. So I went with the first choice.

I'm open to any suggestion that keeps this functionality (new ones or goes with scan_bls_field()) . Currently trying to understand Stefan's suggestion with check_job_ipl_data().