ibm-s390-linux / s390-tools

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

Cannot use the Menu feature with BLS entries #111

Closed rmetrich closed 2 years ago

rmetrich commented 3 years ago

Trying to create a menu sourcing one of more BLS entries, zipl fails to generate the configuration and reports this:

# zipl -V -b . -c zipl.conf.menu --dry-run
Using config file 'zipl.conf.menu' (from command line)
Using BLS config file './bls1.conf'
Error: Config file 'zipl.conf.menu': Line 0: missing keyword 'target' in section 'bls1

The official documentation https://www.ibm.com/docs/en/linux-on-systems?topic=loader-bls-files has been followed.

Reproducer

bls1.conf

title bls1
version 4.18.0-240.el8.s390x
linux /boot/vmlinuz-4.18.0-240.el8.s390x
initrd /boot/initramfs-4.18.0-240.el8.s390x.img

zipl.conf.menu

[defaultboot]
defaultmenu=blsmenu

:blsmenu
1="bls1"
prompt=1
timeout=5
secure=auto
target=/boot

Source code (scan.c)

1035         /* Check keywords */
1036         for (i=0; i < SCAN_KEYWORD_NUM; i++) {
1037                 switch (scan_key_table[(int) *type][i]) {
1038                 case req:
1039                         /* Check for missing data */
1040                         if ((keyword[i] == 0) && (line != NULL)) {
1041                                 /* Missing keyword in config file section */
1042                                 error_reason("Line %d: missing keyword '%s' "
1043                                              "in section '%s'", section_line,
1044                                              scan_keyword_name(
1045                                                      (enum scan_keyword_id) i),
1046                                              name);
1047                                 return -1;

Executing the command under GDB shows that type is section_ipl, which indeed requires the target keyword:

  45 enum scan_key_state scan_key_table[SCAN_SECTION_NUM][SCAN_KEYWORD_NUM] = {
  46 /*       defa dump dump imag para parm ramd segm targ prom time defa tape mv
  47  *       ult  to   tofs e    mete file isk  ent  et   pt   out  ultm      dump
  48  *                           rs                                 enu
 :
  64 /* ipl          */
  65         {inv, inv, inv, req, opt, opt, opt, inv, req, inv, inv, inv, inv, inv,
  66          opt, opt, opt, opt, opt, inv, opt, opt},

The command fails as soon as BLS entries are read.

sharkcz commented 3 years ago

cc @martinezjavier

martinezjavier commented 3 years ago

@rmetrich did you try adding the target keyword to the defaultboot section ? i.e:

[defaultboot]
defaultmenu=blsmenu
target=/boot

:blsmenu
1="bls1"
prompt=1
timeout=5
secure=auto
rmetrich commented 3 years ago

Yep, fails because of incompatible keywords:

# zipl -V -b . -c zipl.conf.menu --dry-run
Using config file 'zipl.conf.menu' (from command line)
Using BLS config file './bls1.conf'
Error: Config file 'zipl.conf.menu': Line 3: Only one of keywords 'default', 'defaultmenu' and 'target' allowed in section 'defaultboot'
martinezjavier commented 3 years ago

Yep, fails because of incompatible keywords:

Sigh, I'll investigate. Wonder if this is a corner case that's not correctly handled by BLS.

rmetrich commented 3 years ago

I see this code:

1803 static int
1804 get_job_from_config_file(struct command_line* cmdline, struct job_data* job)
1805 {
 :
1871         rc = scan_check(scan);
1872         if (rc) {
1873                 error_text("Config file '%s'", filename);
1874                 scan_free(scan);
1875                 return rc;
1876         }
1877         rc = scan_check_bls(scan);
1878         if (rc) {
1879                 error_text("BLS parsing '%s'", blsdir);
1880                 scan_free(scan);
1881                 return rc;
1882         }

Adding a breakpoint at the time the error occurs shows we have the following backtrace:

(gdb) bt
#0  scan_check_section_data (keyword=<optimized out>, line=0x3ffffffeb40, name=0x2aa00032e00 "bls1", 
    section_line=<optimized out>, type=0x3ffffffeb3c) at scan.c:1047
#1  0x000002aa00007936 in check_section (scan=<optimized out>, index=<optimized out>) at scan.c:1419
#2  0x000002aa00007ed2 in scan_check (scan=scan@entry=0x2aa000328a0) at scan.c:1639
#3  0x000002aa0000b27a in get_job_from_config_file (job=0x2aa000322a0, cmdline=0x3ffffffedb8) at job.c:1871
#4  job_get (argc=<optimized out>, argv=<optimized out>, data=<optimized out>) at job.c:1936
#5  0x000002aa00003746 in main (argc=<optimized out>, argv=0x3fffffff2f8) at zipl.c:144

So clearly the scan_check_bls() which apparently sets the target keyword doesn't even run (it's on line 1877 and we die returning from line 1871).

martinezjavier commented 3 years ago

So the problem is that when using the following:

[defaultboot]
defaultauto
target=/boot

the IPL sections inherit the target field but that's not the case when using a menu, like in your example:

[defaultboot]
defaultmenu=blsmenu

:blsmenu
1="bls1"
target=/boot

I think the correct thing to do would be to not make the target field a requirement for IPL sections but instead making it optional and fill with the default section target if is not present.

martinezjavier commented 3 years ago

Ok, I looked a little bit more to this today and probably what we should do is to make the IPL sections created from BLS snippets to inherit the target field defined in either the defaultmenu or defaultboot sections. By doing that, we can keep this field to be required when checking the sections.

I'll work on a patch for this.

martinezjavier commented 3 years ago

@stefan-haberland could you please take a look to the mentioned pull-request #113 ?

stefan-haberland commented 3 years ago

@martinezjavier Thanks for the code. I am just back at work and noticed your pull request. I already had a quick look but I will have a closer look this week.

martinezjavier commented 3 years ago

@stefan-haberland great, thanks a lot!