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

[Harsh Prateek Bora <harshpb@linux.ibm.com>]

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