[PATCH] hw/ppc/pnv: Improve kernel/initrd load failure error messages

Vishal Chourasia posted 1 patch 1 month, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20251007091214.403430-2-vishalc@linux.ibm.com
Maintainers: Nicholas Piggin <npiggin@gmail.com>, Aditya Gupta <adityag@linux.ibm.com>, Glenn Miles <milesg@linux.ibm.com>
There is a newer version of this series
hw/ppc/pnv.c | 8 ++++++++
1 file changed, 8 insertions(+)
[PATCH] hw/ppc/pnv: Improve kernel/initrd load failure error messages
Posted by Vishal Chourasia 1 month, 1 week ago
When QEMU fails to load the kernel or initrd image, it previously emitted
a generic error message such as:

  qemu-system-ppc64: Could not load kernel 'vmlinux'

This provides little context on why the failure occurred, which can make
debugging difficult, especially for new users or when dealing with large
images.

The new messages also include the configured size limits (in MiB) to help
users verify that their image files are within acceptable bounds.

Signed-off-by: Vishal Chourasia <vishalc@linux.ibm.com>
---
 hw/ppc/pnv.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index f0469cdb8b..dbecb721c1 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -1084,6 +1084,10 @@ static void pnv_init(MachineState *machine)
         if (kernel_size < 0) {
             error_report("Could not load kernel '%s'",
                          machine->kernel_filename);
+            error_report(
+                "Possible reasons: file not found, permission denied, or size "
+                "exceeds the maximum supported limit (%ld MiB).",
+                KERNEL_MAX_SIZE / 1024 / 1024);
             exit(1);
         }
     }
@@ -1096,6 +1100,10 @@ static void pnv_init(MachineState *machine)
         if (pnv->initrd_size < 0) {
             error_report("Could not load initial ram disk '%s'",
                          machine->initrd_filename);
+            error_report(
+                "Possible reasons: file not found, permission denied, or size "
+                "exceeds the maximum supported limit (%ld MiB).",
+                INITRD_MAX_SIZE / 1024 / 1024);
             exit(1);
         }
     }
-- 
2.51.0
Re: [PATCH] hw/ppc/pnv: Improve kernel/initrd load failure error messages
Posted by Peter Maydell 1 month, 1 week ago
On Tue, 7 Oct 2025 at 13:59, Vishal Chourasia <vishalc@linux.ibm.com> wrote:
>
> When QEMU fails to load the kernel or initrd image, it previously emitted
> a generic error message such as:
>
>   qemu-system-ppc64: Could not load kernel 'vmlinux'
>
> This provides little context on why the failure occurred, which can make
> debugging difficult, especially for new users or when dealing with large
> images.
>
> The new messages also include the configured size limits (in MiB) to help
> users verify that their image files are within acceptable bounds.

>          if (kernel_size < 0) {
>              error_report("Could not load kernel '%s'",
>                           machine->kernel_filename);
> +            error_report(
> +                "Possible reasons: file not found, permission denied, or size "
> +                "exceeds the maximum supported limit (%ld MiB).",
> +                KERNEL_MAX_SIZE / 1024 / 1024);
>              exit(1);
>          }

Rather than printing a list of reasons why the load might
have failed, I think it would be better if we enhanced
the error handling in load_image_targphys() and friends
(i.e. use Error), so that these functions can report back
to the caller exactly why they failed and then the caller
can give that error message to the user. That way we can
improve the error reporting for every board that uses
these load functions.

thanks
-- PMM
Re: [PATCH] hw/ppc/pnv: Improve kernel/initrd load failure error messages
Posted by Vishal Chourasia 1 month ago
On Tue, Oct 07, 2025 at 02:29:52PM +0100, Peter Maydell wrote:
> On Tue, 7 Oct 2025 at 13:59, Vishal Chourasia <vishalc@linux.ibm.com> wrote:
> >
> > When QEMU fails to load the kernel or initrd image, it previously emitted
> > a generic error message such as:
> >
> >   qemu-system-ppc64: Could not load kernel 'vmlinux'
> >
> > This provides little context on why the failure occurred, which can make
> > debugging difficult, especially for new users or when dealing with large
> > images.
> >
> > The new messages also include the configured size limits (in MiB) to help
> > users verify that their image files are within acceptable bounds.
> 
> >          if (kernel_size < 0) {
> >              error_report("Could not load kernel '%s'",
> >                           machine->kernel_filename);
> > +            error_report(
> > +                "Possible reasons: file not found, permission denied, or size "
> > +                "exceeds the maximum supported limit (%ld MiB).",
> > +                KERNEL_MAX_SIZE / 1024 / 1024);
> >              exit(1);
> >          }
> 
> Rather than printing a list of reasons why the load might
> have failed, I think it would be better if we enhanced
> the error handling in load_image_targphys() and friends
> (i.e. use Error), so that these functions can report back
> to the caller exactly why they failed and then the caller
> can give that error message to the user. That way we can
> improve the error reporting for every board that uses
> these load functions.
Hello Peter,

Wouldn't it be better to print the error inside the function itself.

Thanks
vishalc
> 
> thanks
> -- PMM
>
Re: [PATCH] hw/ppc/pnv: Improve kernel/initrd load failure error messages
Posted by Vishal Chourasia 1 month ago
+ harshpb@linux.ibm.com

On Mon, Oct 13, 2025 at 12:31:58PM +0530, Vishal Chourasia wrote:
> On Tue, Oct 07, 2025 at 02:29:52PM +0100, Peter Maydell wrote:
> > On Tue, 7 Oct 2025 at 13:59, Vishal Chourasia <vishalc@linux.ibm.com> wrote:
> > >
> > > When QEMU fails to load the kernel or initrd image, it previously emitted
> > > a generic error message such as:
> > >
> > >   qemu-system-ppc64: Could not load kernel 'vmlinux'
> > >
> > > This provides little context on why the failure occurred, which can make
> > > debugging difficult, especially for new users or when dealing with large
> > > images.
> > >
> > > The new messages also include the configured size limits (in MiB) to help
> > > users verify that their image files are within acceptable bounds.
> > 
> > >          if (kernel_size < 0) {
> > >              error_report("Could not load kernel '%s'",
> > >                           machine->kernel_filename);
> > > +            error_report(
> > > +                "Possible reasons: file not found, permission denied, or size "
> > > +                "exceeds the maximum supported limit (%ld MiB).",
> > > +                KERNEL_MAX_SIZE / 1024 / 1024);
> > >              exit(1);
> > >          }
> > 
> > Rather than printing a list of reasons why the load might
> > have failed, I think it would be better if we enhanced
> > the error handling in load_image_targphys() and friends
> > (i.e. use Error), so that these functions can report back
> > to the caller exactly why they failed and then the caller
> > can give that error message to the user. That way we can
> > improve the error reporting for every board that uses
> > these load functions.
> Hello Peter,
> 
> Wouldn't it be better to print the error inside the function itself.
> 
> Thanks
> vishalc
> > 
> > thanks
> > -- PMM
> > 
>
Re: [PATCH] hw/ppc/pnv: Improve kernel/initrd load failure error messages
Posted by Peter Maydell 1 month ago
On Mon, 13 Oct 2025 at 08:02, Vishal Chourasia <vishalc@linux.ibm.com> wrote:
>
> On Tue, Oct 07, 2025 at 02:29:52PM +0100, Peter Maydell wrote:
> > On Tue, 7 Oct 2025 at 13:59, Vishal Chourasia <vishalc@linux.ibm.com> wrote:
> > >
> > > When QEMU fails to load the kernel or initrd image, it previously emitted
> > > a generic error message such as:
> > >
> > >   qemu-system-ppc64: Could not load kernel 'vmlinux'
> > >
> > > This provides little context on why the failure occurred, which can make
> > > debugging difficult, especially for new users or when dealing with large
> > > images.
> > >
> > > The new messages also include the configured size limits (in MiB) to help
> > > users verify that their image files are within acceptable bounds.
> >
> > >          if (kernel_size < 0) {
> > >              error_report("Could not load kernel '%s'",
> > >                           machine->kernel_filename);
> > > +            error_report(
> > > +                "Possible reasons: file not found, permission denied, or size "
> > > +                "exceeds the maximum supported limit (%ld MiB).",
> > > +                KERNEL_MAX_SIZE / 1024 / 1024);
> > >              exit(1);
> > >          }
> >
> > Rather than printing a list of reasons why the load might
> > have failed, I think it would be better if we enhanced
> > the error handling in load_image_targphys() and friends
> > (i.e. use Error), so that these functions can report back
> > to the caller exactly why they failed and then the caller
> > can give that error message to the user. That way we can
> > improve the error reporting for every board that uses
> > these load functions.
> Hello Peter,
>
> Wouldn't it be better to print the error inside the function itself.

No, because some users of this family of load functions
use a sequence of calls to handle different possible
formats. We don't want the function to load file format A
to print any errors if we're then going to continue and
successfully load the file as format B.

More generally, our usual coding practice for functions
is that they use Error to tell the caller what went wrong,
and it's the caller that then gets to decide whether they
want to print an error and exit, tell the monitor about
an error, or just continue to try something else instead.

-- PMM
Re: [PATCH] hw/ppc/pnv: Improve kernel/initrd load failure error messages
Posted by Aditya Gupta 1 month ago
On 13/10/25 14:05, Peter Maydell wrote:

> On Mon, 13 Oct 2025 at 08:02, Vishal Chourasia <vishalc@linux.ibm.com> wrote:
>> On Tue, Oct 07, 2025 at 02:29:52PM +0100, Peter Maydell wrote:
>>> On Tue, 7 Oct 2025 at 13:59, Vishal Chourasia <vishalc@linux.ibm.com> wrote:
>>> <...snip...>
>>>
>>> Rather than printing a list of reasons why the load might
>>> have failed, I think it would be better if we enhanced
>>> the error handling in load_image_targphys() and friends
>>> (i.e. use Error), so that these functions can report back
>>> to the caller exactly why they failed and then the caller
>>> can give that error message to the user. That way we can
>>> improve the error reporting for every board that uses
>>> these load functions.
>> Hello Peter,
>>
>> Wouldn't it be better to print the error inside the function itself.
> No, because some users of this family of load functions
> use a sequence of calls to handle different possible
> formats. We don't want the function to load file format A
> to print any errors if we're then going to continue and
> successfully load the file as format B.
>
> More generally, our usual coding practice for functions
> is that they use Error to tell the caller what went wrong,
> and it's the caller that then gets to decide whether they
> want to print an error and exit, tell the monitor about
> an error, or just continue to try something else instead.

In that case, maybe we can have 'load_image_targphys' take an 'enum 
LoadError*' ? Caller can pass that argument if interesting in handling 
errors.

Though i see 71 instances of this function, will have to modify all call 
sites (probably by passing NULL as error* which will be same as previous 
usage).

What do you say ?

Thanks,

- Aditya G

>
> -- PMM
Re: [PATCH] hw/ppc/pnv: Improve kernel/initrd load failure error messages
Posted by Peter Maydell 1 month ago
On Mon, 13 Oct 2025 at 11:18, Aditya Gupta <adityag@linux.ibm.com> wrote:
>
> On 13/10/25 14:05, Peter Maydell wrote:
>
> > On Mon, 13 Oct 2025 at 08:02, Vishal Chourasia <vishalc@linux.ibm.com> wrote:
> >> On Tue, Oct 07, 2025 at 02:29:52PM +0100, Peter Maydell wrote:
> >>> On Tue, 7 Oct 2025 at 13:59, Vishal Chourasia <vishalc@linux.ibm.com> wrote:
> >>> <...snip...>
> >>>
> >>> Rather than printing a list of reasons why the load might
> >>> have failed, I think it would be better if we enhanced
> >>> the error handling in load_image_targphys() and friends
> >>> (i.e. use Error), so that these functions can report back
> >>> to the caller exactly why they failed and then the caller
> >>> can give that error message to the user. That way we can
> >>> improve the error reporting for every board that uses
> >>> these load functions.
> >> Hello Peter,
> >>
> >> Wouldn't it be better to print the error inside the function itself.
> > No, because some users of this family of load functions
> > use a sequence of calls to handle different possible
> > formats. We don't want the function to load file format A
> > to print any errors if we're then going to continue and
> > successfully load the file as format B.
> >
> > More generally, our usual coding practice for functions
> > is that they use Error to tell the caller what went wrong,
> > and it's the caller that then gets to decide whether they
> > want to print an error and exit, tell the monitor about
> > an error, or just continue to try something else instead.
>
> In that case, maybe we can have 'load_image_targphys' take an 'enum
> LoadError*' ? Caller can pass that argument if interesting in handling
> errors.

We have a standard way for functions to report errors with
useful human readable strings attached. That's Error.
We should just use our standard approach if we want to get
better error messages here, rather than inventing an
ad-hoc new thing.

thanks
-- PMM
Re: [PATCH] hw/ppc/pnv: Improve kernel/initrd load failure error messages
Posted by Harsh Prateek Bora 1 month ago

On 10/13/25 16:07, Peter Maydell wrote:
> On Mon, 13 Oct 2025 at 11:18, Aditya Gupta <adityag@linux.ibm.com> wrote:
>>
>> On 13/10/25 14:05, Peter Maydell wrote:
>>
>>> On Mon, 13 Oct 2025 at 08:02, Vishal Chourasia <vishalc@linux.ibm.com> wrote:
>>>> On Tue, Oct 07, 2025 at 02:29:52PM +0100, Peter Maydell wrote:
>>>>> On Tue, 7 Oct 2025 at 13:59, Vishal Chourasia <vishalc@linux.ibm.com> wrote:
>>>>> <...snip...>
>>>>>
>>>>> Rather than printing a list of reasons why the load might
>>>>> have failed, I think it would be better if we enhanced
>>>>> the error handling in load_image_targphys() and friends
>>>>> (i.e. use Error), so that these functions can report back
>>>>> to the caller exactly why they failed and then the caller
>>>>> can give that error message to the user. That way we can
>>>>> improve the error reporting for every board that uses
>>>>> these load functions.
>>>> Hello Peter,
>>>>
>>>> Wouldn't it be better to print the error inside the function itself.
>>> No, because some users of this family of load functions
>>> use a sequence of calls to handle different possible
>>> formats. We don't want the function to load file format A
>>> to print any errors if we're then going to continue and
>>> successfully load the file as format B.
>>>
>>> More generally, our usual coding practice for functions
>>> is that they use Error to tell the caller what went wrong,
>>> and it's the caller that then gets to decide whether they
>>> want to print an error and exit, tell the monitor about
>>> an error, or just continue to try something else instead.
>>
>> In that case, maybe we can have 'load_image_targphys' take an 'enum
>> LoadError*' ? Caller can pass that argument if interesting in handling
>> errors.
> 
> We have a standard way for functions to report errors with
> useful human readable strings attached. That's Error.
> We should just use our standard approach if we want to get
> better error messages here, rather than inventing an
> ad-hoc new thing.

Hi Aditya, Vishal,

Please see below commit for reference (there are many others doing 
similar work though):

commit aa77746602cdf7e29d588d100e27f34bd6e46226
Author: Arun Menon <armenon@redhat.com>
Date:   Thu Sep 18 20:53:39 2025 +0530

     migration: push Error **errp into 
loadvm_postcopy_handle_switchover_start()


Thanks
Harsh

> 
> thanks
> -- PMM