Re: [qemu-s390x] [Qemu-devel] [PULL 09/15] pc-bios/s390-ccw: fix non-sequential boot entries (eckd)

[Laszlo Ersek]

On 05/04/18 09:25, Cornelia Huck wrote:
> From: Collin Walling <address@hidden>
> 
> zIPL boot menu entries can be non-sequential. Let's account
> for this issue for the s390 zIPL boot menu. Since this boot
> menu is actually an imitation and is not completely capable
> of everything the real zIPL menu can do, let's also print a
> different banner to the user.
> 
> Signed-off-by: Collin Walling <address@hidden>
> Reported-by: Vasily Gorbik <address@hidden>
> Reviewed-by: Thomas Huth <address@hidden>
> Reviewed-by: Janosch Frank <address@hidden>
> Signed-off-by: Thomas Huth <address@hidden>
> ---
>  pc-bios/s390-ccw/menu.c | 29 ++++++++++++++++++++---------
>  1 file changed, 20 insertions(+), 9 deletions(-)
> 
> diff --git a/pc-bios/s390-ccw/menu.c b/pc-bios/s390-ccw/menu.c
> index 96eec81e84..aaf5d61ae6 100644
> --- a/pc-bios/s390-ccw/menu.c
> +++ b/pc-bios/s390-ccw/menu.c
> @@ -158,7 +158,7 @@ static void boot_menu_prompt(bool retry)
>      }
>  }
>  
> -static int get_boot_index(int entries)
> +static int get_boot_index(bool *valid_entries)
>  {
>      int boot_index;
>      bool retry = false;
> @@ -168,7 +168,8 @@ static int get_boot_index(int entries)
>          boot_menu_prompt(retry);
>          boot_index = get_index();
>          retry = true;
> -    } while (boot_index < 0 || boot_index >= entries);
> +    } while (boot_index < 0 || boot_index >= MAX_BOOT_ENTRIES ||
> +             !valid_entries[boot_index]);
>  
>      sclp_print("\nBooting entry #");
>      sclp_print(uitoa(boot_index, tmp, sizeof(tmp)));
> @@ -176,7 +177,8 @@ static int get_boot_index(int entries)
>      return boot_index;
>  }
>  
> -static void zipl_println(const char *data, size_t len)
> +/* Returns the entry number that was printed */
> +static int zipl_print_entry(const char *data, size_t len)
>  {
>      char buf[len + 2];
>  
> @@ -185,12 +187,15 @@ static void zipl_println(const char *data, size_t len)
>      buf[len + 1] = '\0';
>  
>      sclp_print(buf);
> +
> +    return buf[0] == ' ' ? atoui(buf + 1) : atoui(buf);
>  }

The return type of this function should likely be "unsigned int", judged
from the function name atoui(). Same for the "entry" variable in
menu_get_zipl_boot_index() below, consequently.

Thanks
Laszlo

>  
>  int menu_get_zipl_boot_index(const char *menu_data)
>  {
>      size_t len;
> -    int entries;
> +    int entry;
> +    bool valid_entries[MAX_BOOT_ENTRIES] = {false};
>      uint16_t zipl_flag = *(uint16_t *)(menu_data - ZIPL_FLAG_OFFSET);
>      uint16_t zipl_timeout = *(uint16_t *)(menu_data - ZIPL_TIMEOUT_OFFSET);
>  
> @@ -202,19 +207,25 @@ int menu_get_zipl_boot_index(const char *menu_data)
>          timeout = zipl_timeout * 1000;
>      }
>  
> -    /* Print and count all menu items, including the banner */
> -    for (entries = 0; *menu_data; entries++) {
> +    /* Print banner */
> +    sclp_print("s390-ccw zIPL Boot Menu\n\n");
> +    menu_data += strlen(menu_data) + 1;
> +
> +    /* Print entries */
> +    while (*menu_data) {
>          len = strlen(menu_data);
> -        zipl_println(menu_data, len);
> +        entry = zipl_print_entry(menu_data, len);
>          menu_data += len + 1;
>  
> -        if (entries < 2) {
> +        valid_entries[entry] = true;
> +
> +        if (entry == 0) {
>              sclp_print("\n");
>          }
>      }
>  
>      sclp_print("\n");
> -    return get_boot_index(entries - 1); /* subtract 1 to exclude banner */
> +    return get_boot_index(valid_entries);
>  }
>  
>  
>