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

[Markus Armbruster]

John Snow  writes:

> On 09/27/2016 02:56 PM, Dr. David Alan Gilbert (git) wrote:
>> From: "Dr. David Alan Gilbert" 
>>
>> 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 

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

[John Snow]

On 09/27/2016 02:56 PM, Dr. David Alan Gilbert (git) wrote:

From: "Dr. David Alan Gilbert" 

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.


Meh. Existing errors here simply use error_report anyway, so:

Reviewed-by: John Snow 

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

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" 

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

-- 
2.7.4