Re: [Qemu-devel] [PATCH]MC146818 RTC: coordinate guest clock base to destination host after migration

[address@hidden]

Hi, Paolo
Thanks for your suggestions, although I don't comply with the idea of rtc_delta 
yet.
But beyond this, I understand your position, and I think the principle you 
insist on,
I mean being simpler, is necessary for the health and sustainability of the 
project.
I'll submit a simpler patch for review, but at the same time, I'll also company 
it with
another more comprehensive patch, which include most of things I have mentioned 
that we want to do besides this patch I'm discussing with you. Then it's all 
after you 
to decide which one is worthy of living.

Thanks again, sincerely.



address@hidden
 
From: Paolo Bonzini
Date: 2016-09-21 23:15
To: address@hidden; qemu-devel
CC: Michael S. Tsirkin
Subject: Re: [PATCH]MC146818 RTC: coordinate guest clock base to destination 
host after migration
 
 
On 21/09/2016 09:36, address@hidden wrote:
> If we change base_rtc while migration, thngs look work fine as 
> normal, but in fact some important information lost. QEMU lost 
> knowledge of time when the guest starts. This infomation maybe hasn't
> users nowadays.
 
I see your point.  However, I think it's simpler to have a single
"delta" between qemu_clock_get_ns(rtc_clock) and the guest RTC clock.
 
To do this, base_rtc and last_update could even be merged in a single
variable (offset is different).
 
> But maybe in the futhure we need, on the other hand, maybe some 
> derived release spawned from community version needs and depends on 
> this feature, who knows!
 
This is not a problem now.  If it's needed in the future one can
complicate the code later.  If it's needed by some downstream release,
they will deal with it.  The goal of QEMU is to have simple code.
 
> Should we merge offset into last_update? I suggest better not, that 
> disrupts time axis model
 
I agree.  But I don't see a reason to keep base_rtc and last_update
separate.  Together they are "rtc_delta".  They are separate mostly for
debugging.
 
> After this patch, our next step is to eliminate deviation during 
> migration. That time meaning of s->offset will be extended. In the
> further future, we may implement some functions on statistics of
> guest lifetime despite it has been migrated or not. That needs a
> fixed base_rtc.
 
Extending the meaning of s->offset is fine, but a fixed s->base_rtc
is not good.  The guest can already modify base_rtc: it will do it every
time it calls rtc_set_time, for example if the time of day is changed in
the guest.
 
I suggest that you post the simplest possible patch to fix your issue,
which calls rtc_update_time pre-save and rtc_set_time post-load
(resetting s->offset after the rtc_set_time, as in the version <= 2
code).  I'm interested in the code to eliminate the pause during
migration, too.
 
Thanks,
 
Paolo