[Qemu-devel] [PATCH] re-set rtc date on reset handler

[Qemu-devel] [PATCH] re-set rtc date on reset handler

[Glauber Costa]

guests without a stable timesource such as kvm-clock will grab the wallclock
from our rtc chip. However, we only sync the date when we first launch qemu.
If a guest goes through a series of reboot cycles, it will slowly see time
getting far behind the host.

The proposal of this patch is to set the date to host clock again in the reset
handler. With this patch, I see a Fedora guest keeping its clock in sync upon
an ulimited number of reboots.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 hw/mc146818rtc.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
index 5c8676e..e71a9da 100644
--- a/hw/mc146818rtc.c
+++ b/hw/mc146818rtc.c
@@ -586,6 +586,8 @@ static void rtc_reset(void *opaque)
     if (rtc_td_hack)
 	    s->irq_coalesced = 0;
 #endif
+
+    rtc_set_date_from_host(s);
 }
 
 RTCState *rtc_init_sqw(int base, qemu_irq irq, qemu_irq sqw_irq, int base_year)
-- 
1.6.2.2

Re: [Qemu-devel] [PATCH] re-set rtc date on reset handler

[Blue Swirl]

On Tue, Sep 1, 2009 at 7:22 PM, Glauber Costa<glommer@redhat.com> wrote:
> guests without a stable timesource such as kvm-clock will grab the wallclock
> from our rtc chip. However, we only sync the date when we first launch qemu.
> If a guest goes through a series of reboot cycles, it will slowly see time
> getting far behind the host.
>
> The proposal of this patch is to set the date to host clock again in the reset
> handler. With this patch, I see a Fedora guest keeping its clock in sync upon
> an ulimited number of reboots.

A different approach is used in m48t59.c: the guest clock is generated
directly from host clock without any timers and only a fixed offset
(to account for time when guest was stopped) is added, so the clock
will always in sync.

Re: [Qemu-devel] [PATCH] re-set rtc date on reset handler

[Glauber Costa]

On Tue, Sep 1, 2009 at 1:56 PM, Blue Swirl<blauwirbel@gmail.com> wrote:
> On Tue, Sep 1, 2009 at 7:22 PM, Glauber Costa<glommer@redhat.com> wrote:
>> guests without a stable timesource such as kvm-clock will grab the wallclock
>> from our rtc chip. However, we only sync the date when we first launch qemu.
>> If a guest goes through a series of reboot cycles, it will slowly see time
>> getting far behind the host.
>>
>> The proposal of this patch is to set the date to host clock again in the reset
>> handler. With this patch, I see a Fedora guest keeping its clock in sync upon
>> an ulimited number of reboots.
>
> A different approach is used in m48t59.c: the guest clock is generated
> directly from host clock without any timers and only a fixed offset
You mean at every ioport read? I actually thought about that, but was
too lazy...


-- 
Glauber  Costa.
"Free as in Freedom"
http://glommer.net

"The less confident you are, the more serious you have to act."

Re: [Qemu-devel] [PATCH] re-set rtc date on reset handler

[Dor Laor]

On 09/02/2009 02:00 AM, Glauber Costa wrote:
> On Tue, Sep 1, 2009 at 1:56 PM, Blue Swirl<blauwirbel@gmail.com>  wrote:
>> On Tue, Sep 1, 2009 at 7:22 PM, Glauber Costa<glommer@redhat.com>  wrote:
>>> guests without a stable timesource such as kvm-clock will grab the wallclock
>>> from our rtc chip. However, we only sync the date when we first launch qemu.
>>> If a guest goes through a series of reboot cycles, it will slowly see time
>>> getting far behind the host.
>>>
>>> The proposal of this patch is to set the date to host clock again in the reset
>>> handler. With this patch, I see a Fedora guest keeping its clock in sync upon
>>> an ulimited number of reboots.
>>
>> A different approach is used in m48t59.c: the guest clock is generated
>> directly from host clock without any timers and only a fixed offset
> You mean at every ioport read? I actually thought about that, but was
> too lazy...
>
>

You also need to handle the -startdate flag of qemu.
In this case you need to keep the offset and not read the host time.

[Qemu-devel] Re: [PATCH] re-set rtc date on reset handler

[Jan Kiszka]

Blue Swirl wrote:
> On Tue, Sep 1, 2009 at 7:22 PM, Glauber Costa<glommer@redhat.com> wrote:
>> guests without a stable timesource such as kvm-clock will grab the wallclock
>> from our rtc chip. However, we only sync the date when we first launch qemu.
>> If a guest goes through a series of reboot cycles, it will slowly see time
>> getting far behind the host.
>>
>> The proposal of this patch is to set the date to host clock again in the reset
>> handler. With this patch, I see a Fedora guest keeping its clock in sync upon
>> an ulimited number of reboots.
> 
> A different approach is used in m48t59.c: the guest clock is generated
> directly from host clock without any timers and only a fixed offset
> (to account for time when guest was stopped) is added, so the clock
> will always in sync.

Ah, that looks like a useful approach! We currently face the issue of
vRTC drifting away from the host time (as the latter is tuned by NTP).

Do you or anyone else know if switching the PC RTC over to the scheme
used in the m48t59 may have some downsides? If not, I would happily hack
up a patch ASAP and suggest it for mainline.

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux

[Qemu-devel] Re: [PATCH] re-set rtc date on reset handler

[Jan Kiszka]

Jan Kiszka wrote:
> Blue Swirl wrote:
>> On Tue, Sep 1, 2009 at 7:22 PM, Glauber Costa<glommer@redhat.com> wrote:
>>> guests without a stable timesource such as kvm-clock will grab the wallclock
>>> from our rtc chip. However, we only sync the date when we first launch qemu.
>>> If a guest goes through a series of reboot cycles, it will slowly see time
>>> getting far behind the host.
>>>
>>> The proposal of this patch is to set the date to host clock again in the reset
>>> handler. With this patch, I see a Fedora guest keeping its clock in sync upon
>>> an ulimited number of reboots.
>> A different approach is used in m48t59.c: the guest clock is generated
>> directly from host clock without any timers and only a fixed offset
>> (to account for time when guest was stopped) is added, so the clock
>> will always in sync.
> 
> Ah, that looks like a useful approach! We currently face the issue of
> vRTC drifting away from the host time (as the latter is tuned by NTP).
> 
> Do you or anyone else know if switching the PC RTC over to the scheme
> used in the m48t59 may have some downsides? If not, I would happily hack
> up a patch ASAP and suggest it for mainline.

Clearly, one downside is that a jumping host time will cause the RTC to
jump as well. However, there might by setups where this does not happen,
so optionally switching the RTC over rt_clock seems like a reasonable
feature to me.

What about consolidating -localtime, -starttime and -rtc-td-hack at this
chance? Something like

 -rtc base=utc|localtime|date,clock=vm|rt

with 'date' specifying the base as in -starttime. 'clock' would then
allow to drive the RTC via the host's CLOCK_REALTIME (with all its pros
and cons).

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux

[Qemu-devel] Re: [PATCH] re-set rtc date on reset handler

[Jan Kiszka]

Jan Kiszka wrote:
> Jan Kiszka wrote:
>> Blue Swirl wrote:
>>> On Tue, Sep 1, 2009 at 7:22 PM, Glauber Costa<glommer@redhat.com> wrote:
>>>> guests without a stable timesource such as kvm-clock will grab the wallclock
>>>> from our rtc chip. However, we only sync the date when we first launch qemu.
>>>> If a guest goes through a series of reboot cycles, it will slowly see time
>>>> getting far behind the host.
>>>>
>>>> The proposal of this patch is to set the date to host clock again in the reset
>>>> handler. With this patch, I see a Fedora guest keeping its clock in sync upon
>>>> an ulimited number of reboots.
>>> A different approach is used in m48t59.c: the guest clock is generated
>>> directly from host clock without any timers and only a fixed offset
>>> (to account for time when guest was stopped) is added, so the clock
>>> will always in sync.
>> Ah, that looks like a useful approach! We currently face the issue of
>> vRTC drifting away from the host time (as the latter is tuned by NTP).
>>
>> Do you or anyone else know if switching the PC RTC over to the scheme
>> used in the m48t59 may have some downsides? If not, I would happily hack
>> up a patch ASAP and suggest it for mainline.
> 
> Clearly, one downside is that a jumping host time will cause the RTC to
> jump as well. However, there might by setups where this does not happen,
> so optionally switching the RTC over rt_clock seems like a reasonable
> feature to me.
> 
> What about consolidating -localtime, -starttime and -rtc-td-hack at this
> chance? Something like
> 
>  -rtc base=utc|localtime|date,clock=vm|rt

+ ,td-hack=on|off

of course. Or let's call it "drift-hack".

> 
> with 'date' specifying the base as in -starttime. 'clock' would then
> allow to drive the RTC via the host's CLOCK_REALTIME (with all its pros
> and cons).
> 
> Jan
> 

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] Re: [PATCH] re-set rtc date on reset handler

[Dor Laor]

On 09/08/2009 08:00 PM, Jan Kiszka wrote:
> Jan Kiszka wrote:
>> Jan Kiszka wrote:
>>> Blue Swirl wrote:
>>>> On Tue, Sep 1, 2009 at 7:22 PM, Glauber Costa<glommer@redhat.com>  wrote:
>>>>> guests without a stable timesource such as kvm-clock will grab the wallclock
>>>>> from our rtc chip. However, we only sync the date when we first launch qemu.
>>>>> If a guest goes through a series of reboot cycles, it will slowly see time
>>>>> getting far behind the host.
>>>>>
>>>>> The proposal of this patch is to set the date to host clock again in the reset
>>>>> handler. With this patch, I see a Fedora guest keeping its clock in sync upon
>>>>> an ulimited number of reboots.
>>>> A different approach is used in m48t59.c: the guest clock is generated
>>>> directly from host clock without any timers and only a fixed offset
>>>> (to account for time when guest was stopped) is added, so the clock
>>>> will always in sync.
>>> Ah, that looks like a useful approach! We currently face the issue of
>>> vRTC drifting away from the host time (as the latter is tuned by NTP).
>>>
>>> Do you or anyone else know if switching the PC RTC over to the scheme
>>> used in the m48t59 may have some downsides? If not, I would happily hack
>>> up a patch ASAP and suggest it for mainline.
>>
>> Clearly, one downside is that a jumping host time will cause the RTC to
>> jump as well. However, there might by setups where this does not happen,
>> so optionally switching the RTC over rt_clock seems like a reasonable
>> feature to me.
>>
>> What about consolidating -localtime, -starttime and -rtc-td-hack at this
>> chance? Something like
>>
>>   -rtc base=utc|localtime|date,clock=vm|rt
>
> + ,td-hack=on|off
>
> of course. Or let's call it "drift-hack".

btw: this is not a hack, its virtualization support for rtc.
It should be the default when qemu runs along with kvm.

>
>>
>> with 'date' specifying the base as in -starttime. 'clock' would then
>> allow to drive the RTC via the host's CLOCK_REALTIME (with all its pros
>> and cons).
>>
>> Jan
>>
>
> Jan
>

Re: [Qemu-devel] Re: [PATCH] re-set rtc date on reset handler

[Jan Kiszka]

Dor Laor wrote:
> On 09/08/2009 08:00 PM, Jan Kiszka wrote:
>> Jan Kiszka wrote:
>>> Jan Kiszka wrote:
>>>> Blue Swirl wrote:
>>>>> On Tue, Sep 1, 2009 at 7:22 PM, Glauber Costa<glommer@redhat.com>  wrote:
>>>>>> guests without a stable timesource such as kvm-clock will grab the wallclock
>>>>>> from our rtc chip. However, we only sync the date when we first launch qemu.
>>>>>> If a guest goes through a series of reboot cycles, it will slowly see time
>>>>>> getting far behind the host.
>>>>>>
>>>>>> The proposal of this patch is to set the date to host clock again in the reset
>>>>>> handler. With this patch, I see a Fedora guest keeping its clock in sync upon
>>>>>> an ulimited number of reboots.
>>>>> A different approach is used in m48t59.c: the guest clock is generated
>>>>> directly from host clock without any timers and only a fixed offset
>>>>> (to account for time when guest was stopped) is added, so the clock
>>>>> will always in sync.
>>>> Ah, that looks like a useful approach! We currently face the issue of
>>>> vRTC drifting away from the host time (as the latter is tuned by NTP).
>>>>
>>>> Do you or anyone else know if switching the PC RTC over to the scheme
>>>> used in the m48t59 may have some downsides? If not, I would happily hack
>>>> up a patch ASAP and suggest it for mainline.
>>> Clearly, one downside is that a jumping host time will cause the RTC to
>>> jump as well. However, there might by setups where this does not happen,
>>> so optionally switching the RTC over rt_clock seems like a reasonable
>>> feature to me.
>>>
>>> What about consolidating -localtime, -starttime and -rtc-td-hack at this
>>> chance? Something like
>>>
>>>   -rtc base=utc|localtime|date,clock=vm|rt
>> + ,td-hack=on|off
>>
>> of course. Or let's call it "drift-hack".
> 
> btw: this is not a hack, its virtualization support for rtc.
> It should be the default when qemu runs along with kvm.

OK, will call it 'drift-fix'. But enabling it by default in kvm mode
should be filed as a separate patch on top of my queue (which will
appear soon).

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] Re: [PATCH] re-set rtc date on reset handler

[Avi Kivity]

On 09/09/2009 03:57 PM, Jan Kiszka wrote:
>
>> btw: this is not a hack, its virtualization support for rtc.
>> It should be the default when qemu runs along with kvm.
>>      
> OK, will call it 'drift-fix'. But enabling it by default in kvm mode
> should be filed as a separate patch on top of my queue (which will
> appear soon).
>    

drift-compensation?

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] Re: [PATCH] re-set rtc date on reset handler

[Jan Kiszka]

Avi Kivity wrote:
> On 09/09/2009 03:57 PM, Jan Kiszka wrote:
>>> btw: this is not a hack, its virtualization support for rtc.
>>> It should be the default when qemu runs along with kvm.
>>>      
>> OK, will call it 'drift-fix'. But enabling it by default in kvm mode
>> should be filed as a separate patch on top of my queue (which will
>> appear soon).
>>    
> 
> drift-compensation?
> 

'drift-compensation-for-windows-guests' - this is at least what the
documentation suggests right now. I was looking for a short tag (but not
as short as 'td'...).

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] Re: [PATCH] re-set rtc date on reset handler

[Avi Kivity]

On 09/09/2009 04:31 PM, Jan Kiszka wrote:
> 'drift-compensation-for-windows-guests' - this is at least what the
> documentation suggests right now. I was looking for a short tag (but not
> as short as 'td'...).
>
>    

It would work for any guest that uses rtc as its main clock source.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] Re: [PATCH] re-set rtc date on reset handler

[Anthony Liguori]

Avi Kivity wrote:
> On 09/09/2009 04:31 PM, Jan Kiszka wrote:
>> 'drift-compensation-for-windows-guests' - this is at least what the
>> documentation suggests right now. I was looking for a short tag (but not
>> as short as 'td'...).
>>
>>    
>
> It would work for any guest that uses rtc as its main clock source.

First step is qdev conversion for the RTC.  You can then introduce a 
drift property.  I'd suggest something like drift=none|catchup|gradual.  
The default can be none.  We can also introduce a kvm machine type where 
the default is catchup.  Obviously, we don't support gradual today.

You can modify the default rtc settings using the -set operation (we can 
give the rtc a default id).  This lets localtime, td-hack, etc. all be 
aliases for -set.

-- 
Regards,

Anthony Liguori

Re: [Qemu-devel] Re: [PATCH] re-set rtc date on reset handler

[Gleb Natapov]

On Wed, Sep 09, 2009 at 10:34:54AM -0500, Anthony Liguori wrote:
> Avi Kivity wrote:
> >On 09/09/2009 04:31 PM, Jan Kiszka wrote:
> >>'drift-compensation-for-windows-guests' - this is at least what the
> >>documentation suggests right now. I was looking for a short tag (but not
> >>as short as 'td'...).
> >>
> >
> >It would work for any guest that uses rtc as its main clock source.
> 
> First step is qdev conversion for the RTC.  You can then introduce a
> drift property.  I'd suggest something like
> drift=none|catchup|gradual.  The default can be none.  We can also
> introduce a kvm machine type where the default is catchup.
> Obviously, we don't support gradual today.
> 
What is "catchup" and what is "gradual"?

--
			Gleb.

Re: [Qemu-devel] Re: [PATCH] re-set rtc date on reset handler

[Anthony Liguori]

Gleb Natapov wrote:
> What is "catchup" and what is "gradual"?
>   

Just placeholders to point out that there are different algorithms to 
replaying interrupts to a guest.  I called what we implemented "catchup" 
since we pretty much immediately send all of the missed ticks to the 
guest in order to catch-up.

I'd imagine that a 'gradual' algorithm would shorten the tick interval 
until you've caught up.  The Hyper-V paravirtualization manual has 
provisions for the guest to request what type of algorithm is used so it 
makes sense to accommodate this in our syntax.

-- 
Regards,

Anthony Liguori

Re: [Qemu-devel] Re: [PATCH] re-set rtc date on reset handler

[Gleb Natapov]

On Wed, Sep 09, 2009 at 12:52:12PM -0500, Anthony Liguori wrote:
> Gleb Natapov wrote:
> >What is "catchup" and what is "gradual"?
> 
> Just placeholders to point out that there are different algorithms
> to replaying interrupts to a guest.  I called what we implemented
> "catchup" since we pretty much immediately send all of the missed
> ticks to the guest in order to catch-up.
> 
> I'd imagine that a 'gradual' algorithm would shorten the tick
> interval until you've caught up.  The Hyper-V paravirtualization
> manual has provisions for the guest to request what type of
> algorithm is used so it makes sense to accommodate this in our
> syntax.
> 
We implement gradual then.

--
			Gleb.

Re: [Qemu-devel] Re: [PATCH] re-set rtc date on reset handler

[Anthony Liguori]

Gleb Natapov wrote:
> On Wed, Sep 09, 2009 at 12:52:12PM -0500, Anthony Liguori wrote:
>   
>> Gleb Natapov wrote:
>>     
>>> What is "catchup" and what is "gradual"?
>>>       
>> Just placeholders to point out that there are different algorithms
>> to replaying interrupts to a guest.  I called what we implemented
>> "catchup" since we pretty much immediately send all of the missed
>> ticks to the guest in order to catch-up.
>>
>> I'd imagine that a 'gradual' algorithm would shorten the tick
>> interval until you've caught up.  The Hyper-V paravirtualization
>> manual has provisions for the guest to request what type of
>> algorithm is used so it makes sense to accommodate this in our
>> syntax.
>>
>>     
> We implement gradual then.
>   

You're right.

Regards,

Anthony Liguori

Re: [Qemu-devel] Re: [PATCH] re-set rtc date on reset handler

[Avi Kivity]

On 09/09/2009 06:34 PM, Anthony Liguori wrote:
> First step is qdev conversion for the RTC.  You can then introduce a 
> drift property.  I'd suggest something like 
> drift=none|catchup|gradual.  The default can be none.  We can also 
> introduce a kvm machine type where the default is catchup.  Obviously, 
> we don't support gradual today.
>

Maybe none|step|slew to avoid alienating mayo lovers.

Should be a global option that applies to all clock sources?

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [Qemu-devel] Re: [PATCH] re-set rtc date on reset handler

[Anthony Liguori]

Avi Kivity wrote:
> On 09/09/2009 06:34 PM, Anthony Liguori wrote:
>> First step is qdev conversion for the RTC.  You can then introduce a 
>> drift property.  I'd suggest something like 
>> drift=none|catchup|gradual.  The default can be none.  We can also 
>> introduce a kvm machine type where the default is catchup.  
>> Obviously, we don't support gradual today.
>>
>
> Maybe none|step|slew to avoid alienating mayo lovers.
>
> Should be a global option that applies to all clock sources?

Should be an option for all clock devices.  And I think it makes sense 
to have a virtualization machine type that is automatically enabled when 
using kvm and that automatically enables drift=step.

-- 
Regards,

Anthony Liguori