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

[Steven Sistare <steven.sistare@oracle.com>]

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>
>> ---
>>   migration/cpr.c | 9 +++++----
>>   1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/migration/cpr.c b/migration/cpr.c
>> index 42ad0b0d50..908bcf83b2 100644
>> --- a/migration/cpr.c
>> +++ b/migration/cpr.c
>> @@ -7,6 +7,7 @@
>>   #include "qemu/osdep.h"
>>   #include "qapi/error.h"
>> +#include "qemu/error-report.h"
>>   #include "hw/vfio/vfio-device.h"
>>   #include "migration/cpr.h"
>>   #include "migration/misc.h"
>> @@ -100,10 +101,10 @@ void cpr_resave_fd(const char *name, int id, int fd)
>>       if (old_fd < 0) {
>>           cpr_save_fd(name, id, fd);
>>       } else if (old_fd != fd) {
>> -        error_setg(&error_fatal,
>> -                   "internal error: cpr fd '%s' id %d value %d "
>> -                   "already saved with a different value %d",
>> -                   name, id, fd, old_fd);
>> +        error_report("internal error: cpr fd '%s' id %d value %d "
>> +                     "already saved with a different value %d",
>> +                     name, id, fd, old_fd);
>> +        exit(1);
> 
> 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.

- Steve