qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] hw/ppc/pnv: Improve kernel/initrd load failure error messages
@ 2025-10-07  9:12 Vishal Chourasia
  2025-10-07 13:29 ` Peter Maydell
  0 siblings, 1 reply; 8+ messages in thread
From: Vishal Chourasia @ 2025-10-07  9:12 UTC (permalink / raw)
  To: npiggin, adityag, milesg, qemu-ppc, qemu-devel; +Cc: Vishal Chourasia

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



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] hw/ppc/pnv: Improve kernel/initrd load failure error messages
  2025-10-07  9:12 [PATCH] hw/ppc/pnv: Improve kernel/initrd load failure error messages Vishal Chourasia
@ 2025-10-07 13:29 ` Peter Maydell
  2025-10-13  7:01   ` Vishal Chourasia
  0 siblings, 1 reply; 8+ messages in thread
From: Peter Maydell @ 2025-10-07 13:29 UTC (permalink / raw)
  To: Vishal Chourasia; +Cc: npiggin, adityag, milesg, qemu-ppc, qemu-devel

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] hw/ppc/pnv: Improve kernel/initrd load failure error messages
  2025-10-07 13:29 ` Peter Maydell
@ 2025-10-13  7:01   ` Vishal Chourasia
  2025-10-13  8:35     ` Peter Maydell
  2025-10-13  9:50     ` Vishal Chourasia
  0 siblings, 2 replies; 8+ messages in thread
From: Vishal Chourasia @ 2025-10-13  7:01 UTC (permalink / raw)
  To: Peter Maydell; +Cc: npiggin, adityag, milesg, qemu-ppc, qemu-devel

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
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] hw/ppc/pnv: Improve kernel/initrd load failure error messages
  2025-10-13  7:01   ` Vishal Chourasia
@ 2025-10-13  8:35     ` Peter Maydell
  2025-10-13 10:18       ` Aditya Gupta
  2025-10-13  9:50     ` Vishal Chourasia
  1 sibling, 1 reply; 8+ messages in thread
From: Peter Maydell @ 2025-10-13  8:35 UTC (permalink / raw)
  To: Vishal Chourasia; +Cc: npiggin, adityag, milesg, qemu-ppc, qemu-devel

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] hw/ppc/pnv: Improve kernel/initrd load failure error messages
  2025-10-13  7:01   ` Vishal Chourasia
  2025-10-13  8:35     ` Peter Maydell
@ 2025-10-13  9:50     ` Vishal Chourasia
  1 sibling, 0 replies; 8+ messages in thread
From: Vishal Chourasia @ 2025-10-13  9:50 UTC (permalink / raw)
  To: Peter Maydell; +Cc: harshpb, npiggin, adityag, milesg, qemu-ppc, qemu-devel

+ 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
> > 
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] hw/ppc/pnv: Improve kernel/initrd load failure error messages
  2025-10-13  8:35     ` Peter Maydell
@ 2025-10-13 10:18       ` Aditya Gupta
  2025-10-13 10:37         ` Peter Maydell
  0 siblings, 1 reply; 8+ messages in thread
From: Aditya Gupta @ 2025-10-13 10:18 UTC (permalink / raw)
  To: Peter Maydell, Vishal Chourasia
  Cc: npiggin, milesg, qemu-ppc, qemu-devel, Harsh Prateek Bora

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] hw/ppc/pnv: Improve kernel/initrd load failure error messages
  2025-10-13 10:18       ` Aditya Gupta
@ 2025-10-13 10:37         ` Peter Maydell
  2025-10-13 10:54           ` Harsh Prateek Bora
  0 siblings, 1 reply; 8+ messages in thread
From: Peter Maydell @ 2025-10-13 10:37 UTC (permalink / raw)
  To: Aditya Gupta
  Cc: Vishal Chourasia, npiggin, milesg, qemu-ppc, qemu-devel,
	Harsh Prateek Bora

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] hw/ppc/pnv: Improve kernel/initrd load failure error messages
  2025-10-13 10:37         ` Peter Maydell
@ 2025-10-13 10:54           ` Harsh Prateek Bora
  0 siblings, 0 replies; 8+ messages in thread
From: Harsh Prateek Bora @ 2025-10-13 10:54 UTC (permalink / raw)
  To: Aditya Gupta, Vishal Chourasia, Peter Maydell
  Cc: npiggin, milesg, qemu-ppc, qemu-devel



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


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2025-10-13 10:54 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-07  9:12 [PATCH] hw/ppc/pnv: Improve kernel/initrd load failure error messages Vishal Chourasia
2025-10-07 13:29 ` Peter Maydell
2025-10-13  7:01   ` Vishal Chourasia
2025-10-13  8:35     ` Peter Maydell
2025-10-13 10:18       ` Aditya Gupta
2025-10-13 10:37         ` Peter Maydell
2025-10-13 10:54           ` Harsh Prateek Bora
2025-10-13  9:50     ` Vishal Chourasia

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).