From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:49835)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <Leon.Alrae@imgtec.com>) id 1Xwtyr-0001Et-8Q
	for qemu-devel@nongnu.org; Fri, 05 Dec 2014 09:35:43 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <Leon.Alrae@imgtec.com>) id 1Xwtyk-0002Pr-W1
	for qemu-devel@nongnu.org; Fri, 05 Dec 2014 09:35:37 -0500
Received: from mailapp01.imgtec.com ([195.59.15.196]:16284)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <Leon.Alrae@imgtec.com>) id 1Xwtyk-0002P1-Qn
	for qemu-devel@nongnu.org; Fri, 05 Dec 2014 09:35:30 -0500
Message-ID: <5481C2AC.5000609@imgtec.com>
Date: Fri, 5 Dec 2014 14:35:24 +0000
From: Leon Alrae <leon.alrae@imgtec.com>
MIME-Version: 1.0
References: <alpine.DEB.1.10.1411121544140.2881@tp.orcam.me.uk>
	<CAFEAcA_sUBw47drdKDt-LGOo271o+1PtbyR9nvL6e_K6Fm4Cdw@mail.gmail.com>
	<alpine.DEB.1.10.1411121847260.2881@tp.orcam.me.uk>
In-Reply-To: <alpine.DEB.1.10.1411121847260.2881@tp.orcam.me.uk>
Content-Type: text/plain; charset="ISO-8859-1"
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH] mips: Correctly save/restore the FP
 flush-to-zero state
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: "Maciej W. Rozycki" <macro@codesourcery.com>, Peter Maydell <peter.maydell@linaro.org>
Cc: QEMU Developers <qemu-devel@nongnu.org>, Aurelien Jarno <aurelien@aurel32.net>

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