| Message ID | 20190514045824.10112-1-takahiro.akashi@linaro.org |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | Heinrich Schuchardt |
| Headers | show |
| Series | [U-Boot] efi_loader: bootmgr: print a message when loadingfrom BootNext failed | expand |
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@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...\ 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"); >
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@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. -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"); > > >
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@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"); >>> >> >
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", + bootnext); } } else { printf("Deleting BootNext failed\n");
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@linaro.org> --- lib/efi_loader/efi_bootmgr.c | 2 ++ 1 file changed, 2 insertions(+)