On 12/11/2014 18:58, Maciej W. Rozycki wrote:
> On Wed, 12 Nov 2014, Peter Maydell wrote:
>
>>> @@ -208,12 +206,12 @@ int cpu_load(QEMUFile *f, void *opaque,
>>> MIPSCPU *cpu = mips_env_get_cpu(env);
>>> int i;
>>>
>>> - if (version_id < 3) {
>>> + if (version_id != CPU_SAVE_VERSION) {
>>> return -EINVAL;
>>> }
>>
>> Shouldn't this read "if (version_id < 6)" ?
>> Otherwise next time somebody bumps the CPU_SAVE_VERSION it
>> will give another migration compatibility break without that
>> being very obvious.
>
> I gave it a thought before making this change and concluded it would be
> the lesser evil (plus loudly manifesting and easily correctable) if
> someone accidentally makes QEMU refuse to load older images where in
> fact no compatibility issue exists, than if the reverse is the case,
> that is older incompatible images are accepted where they should not
> (causing a silent misinterpretation of data), simply because someone
> missed the need to change the condition in addition to bumping up
> CPU_SAVE_VERSION. WDYT?
Finally I got round to reviewing v2 of this patch. Above sounds
reasonable for me and I haven't seen any objections.
Hopefully we will convert it into VMState during 2.3 development (Peter,
thanks for pointing at the commit, it will certainly help).
Maciej, the only question I have is why you removed only some of "if
(version_id >= X)" lines in machine.c?
Regards,
Leon
