Re: [Qemu-devel] [PATCH 0/2] Add error reporting in migration

[Markus Armbruster]

John Snow <address@hidden> writes:

> On 09/27/2016 02:56 PM, Dr. David Alan Gilbert (git) wrote:
>> From: "Dr. David Alan Gilbert" <address@hidden>
>>
>> At the moment if you use a VMSTATE_*_EQUAL macro and the value
>> doesn't match you just get an error about the section that failed
>>
>> e.g.
>>   qemu-system-ppc64: error while loading state for instance 0x0 of device 
>> 'cpu'
>>   qemu-system-ppc64: load of migration failed: Invalid argument
>>
>> with this pair you get the field and the mismatched values.
>> e.g.
>>   qemu-system-ppc64: 8000600FE1FF7AE1 != 8000600FE1FF3A21
>>   qemu-system-ppc64: Failed to load cpu:env.insns_flags
>>   qemu-system-ppc64: error while loading state for instance 0x0 of device 
>> 'cpu'
>>   qemu-system-ppc64: load of migration failed: Invalid argument
>>
>> which is much more likely to point you at the culprit.
>>
>> (Broken out from a larger vmstatification series, the only change since
>> then is the values are printed in hex except for the le case).
>>
>> Dave
>>
>> Dr. David Alan Gilbert (2):
>>   migration: report an error giving the failed field
>>   migration: Report values for comparisons
>>
>>  migration/vmstate.c | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>
> I see this as a strict improvement; though I don't know if there will
> be complaints about printing error messages instead of adding pathways
> for the Error object.

Drive-by comment without having studied the patch: if a function can run
within a function that takes an Error * parameter, then error_report()
is probably wrong.  It's less wrong than not reporting the error at all,
though.  It's okay to point this out to the poster.  Perhaps he's
willing to go all the way, once aware.  Sometimes, going all the way is
more work than the poster can give.  Badgering him for it would be
ungrateful.  A small incremental improvement is still better than
nothing.  Recording the imperfect nature of the change in commit message
or comments may be in order then.

> Meh. Existing errors here simply use error_report anyway, so:
>
> Reviewed-by: John Snow <address@hidden>