[U-Boot] [PATCH] efi_loader: bootmgr: print a message when loading from BootNext failed

[Heinrich Schuchardt]

On 5/14/19 10:01 AM, AKASHI Takahiro wrote:
> On Tue, May 14, 2019 at 08:27:13AM +0200, Heinrich Schuchardt wrote:
>> On 5/14/19 6:58 AM, AKASHI Takahiro wrote:
>>> If a user defines BootNext but not BootOrder and loading from BootNext
>>> fails, you will see only a message like this:
>>> 	BootOrder not defined
>>>
>>> This may confuse a user. Adding an error message will be helpful.
>>>
>>> Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
>>> ---
>>>   lib/efi_loader/efi_bootmgr.c | 2 ++
>>>   1 file changed, 2 insertions(+)
>>>
>>> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
>>> index 7bf51874c1c1..6a4a478473c3 100644
>>> --- a/lib/efi_loader/efi_bootmgr.c
>>> +++ b/lib/efi_loader/efi_bootmgr.c
>>> @@ -215,6 +215,8 @@ efi_status_t efi_bootmgr_load(efi_handle_t *handle)
>>>   				ret = try_load_entry(bootnext, handle);
>>>   				if (ret == EFI_SUCCESS)
>>>   					return ret;
>>> +				printf("Loading from Boot%04X failed, falling back into BootOrder...\n",
>>
>> Seeing an error makes sense.
>>
>> When multiple entries in BootOder fail your would write:
>>
>> Loading from Boot0000 failed, falling back into BootOrder...\
>> Loading from Boot0001 failed, falling back into BootOrder...\
>> Loading from Boot0002 failed, falling back into BootOrder...\
>> Loading from Boot0003 failed, falling back into BootOrder...\
>
> Are you sure?
> I don't believe we will see such repeated messages because
> "Loading from Boot%04X failed" message will be only shown
> when evaluating BootNext.

Yes, but printing a message here and none when BootOrder entries fail
makes no sense either.

So replacing the debug() statement in try_load_entry() by a

     printf("Loading from %ls failed\n", varname);

would my preferred choice.

Regards

Heinrich

>
> -Takahiro Akashi
>
>> As a user I would wonder why you would fall back to BootOrder multiple
>> times.
>>
>> I think "Loading from Boot%04X failed" conveys all the information the
>> user needs and is less distracting.
>>
>> Best regards
>>
>> Heinrich
>>
>>> +				       bootnext);
>>>   			}
>>>   		} else {
>>>   			printf("Deleting BootNext failed\n");
>>>
>>
>