Re: [Qemu-devel] [Qemu-ppc] [PATCH] target/ppc: only save guest timebase once after stopping

[Laurent Vivier]

On 04/05/2018 15:50, Greg Kurz wrote:
> On Fri, 04 May 2018 07:18:13 -0500
> Michael Roth <address@hidden> wrote:
> 
>> Quoting Greg Kurz (2018-05-04 04:37:24)
>>> On Thu,  3 May 2018 23:20:44 -0500
>>> Michael Roth <address@hidden> wrote:
>>>   
>>>> In some cases (e.g. spapr) we record guest timebase after qmp_stop()
>>>> via a runstate hook so we can restore it on qmp_cont(). If a migration
>>>> occurs in between those events we end up saving it again, this time
>>>> based on the current timebase the guest would be seeing had it been
>>>> running. This has the effect of advancing the guest timebase while
>>>> it is stopped, which is not what the code intends.
>>>>   
>>>
>>> Hi Mike,
>>>
>>> The current behavior was introduced by:
>>>
>>> commit 42043e4f1241eeb77f87f5816b5cf0b6e9583ed7
>>> Author: Laurent Vivier <address@hidden>
>>> Date:   Fri Jan 27 13:24:58 2017 +0100
>>>
>>>     spapr: clock should count only if vm is running
>>>
>>> and we have this in the changelog:
>>>
>>>     We keep timebase_pre_save to reduce the clock difference on
>>>     migration like in:
>>>         6053a86 kvmclock: reduce kvmclock difference on migration
>>>
>>>
>>> So your patch totally negates ^^ ? Also, I can't see a case where  
>>
>> Yah... this is a bit confusing. On one hand, the patch/summary is clearly
>> trying to avoid the guest time from advancing while it is stopped, which
>> is in the spirit of this patch. But at the same time it is trying to
>> compensate for loss of time (relative to host) due to downtime window.
>>
> 
> Yeah... not sure why Laurent decided to address both in the same patch...
> maybe just because we already had the pre_save hook ?

Well, it seemed to be a good idea to do like it is done for x86 [1]

But I think you should remove the timebase_pre_save() function (and the
comment) instead of adding a new flag.

Thanks,
Laurent

[1] the idea was proposed by Paolo:
https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg05862.html