Re: [PATCH 04/12] migration/cpr: Clean up error reporting in cpr_resave_fd()

[Markus Armbruster <armbru@redhat.com>]

Steven Sistare <steven.sistare@oracle.com> writes:

> On 8/8/2025 10:43 AM, Markus Armbruster wrote:
>> Steven Sistare <steven.sistare@oracle.com> writes:
>> 
>>> On 8/8/2025 9:55 AM, Philippe Mathieu-Daudé wrote:
>>>> On 8/8/25 10:08, Markus Armbruster wrote:
>>>>> qapi/error.h advises:
>>>>>
>>>>>   * Please don't error_setg(&error_fatal, ...), use error_report() and
>>>>>   * exit(), because that's more obvious.
>>>>>
>>>>> Do that.
>>>>>
>>>>> The error message starts with "internal error: ", so maybe this should
>>>>> assert() instead.
>>>>>
>>>>> Cc: Steve Sistare <steven.sistare@oracle.com>
>>>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>

[...]

>>>> My 2 cents, I'm not sure this information is more helpful than a plain
>>>> assertion (at least for users). No objection for this change.
>>>
>>> The message gives more information.  It has helped me debug
>>> problems in the past, in concert with enabling cpr traces.
>>
>> Is it a programming error?
>
> Yes.
>
>> If no, then "internal error: " is wrong.
>>
>> If yes, then exit(1) is wrong.  I'd use assert() myself, but you're the
>> maintainer here, and if you want this message rather than the one
>> assert() gives you for free, we just replace exit(1) by abort() or
>> assert(0) or, if we're feeling particularly fancy
>> g_assert_not_reached().
>
> I would like the full message.
> I have no preference on how to exit.

Will adjust v2 accordingly.

Thanks!