[PATCH v2 22/71] efi: Improve logging in efi_disk

[Simon Glass]

Hi Tom, Heinrich,

On Fri, 13 Jan 2023 at 13:40, Tom Rini <trini at konsulko.com> wrote:
>
> On Fri, Jan 13, 2023 at 09:35:36PM +0100, Heinrich Schuchardt wrote:
> > On 1/8/23 03:49, Simon Glass wrote:
> > > When this fails it can be time-consuming to debug. Add some debugging
> > > to help with this. Also try to return error codes instead of just using
> > > -1.
> > >
> > > Signed-off-by: Simon Glass <sjg at chromium.org>
> > > ---
> > >
> > > (no changes since v1)
> > >
> > >   lib/efi_loader/efi_disk.c | 30 +++++++++++++++++++++---------
> > >   1 file changed, 21 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
> > > index 7ea0334083f..37123dd2474 100644
> > > --- a/lib/efi_loader/efi_disk.c
> > > +++ b/lib/efi_loader/efi_disk.c
> > > @@ -421,13 +421,16 @@ static efi_status_t efi_disk_add_dev(
> > >
> > >             if (!node) {
> > >                     ret = EFI_OUT_OF_RESOURCES;
> > > +                   log_debug("no node\n");
> >
> > Please, provide a descriptive message. I would not know what "no node"
> > might mean if I were to read it.
> >
> > There is a reason why we set ret = EFI_OUT_OF_RESOURCES?
> >
> > The caller should know what this means.
>
> There's a fine balance to be struck here. An error message should be
> grep'd through in the code, for debug messages. With CONFIG_LOG we're
> already getting file, line and function.
>

Yes, and people need to read the code to figure out what went wrong. I
did this patch because the EFI code returns the same error message for
lots of situations and logging helped me work out what was wrong.

But feel free to drop it if is isn't needed.

Regards,
Simon