8 a.m.
Domain's clock xml is as below.
<clock offset='variable' basis='utc' adjustment='10'/>
If the guest modifies its RTC, libvirt will hanlde the time offset
and save the active status in qemuProcessHandleRTCChange(). However,
libvirt won't save the persistent config. So next time when vm is
restarted(shutdown and start), the time adjuestment(RTC change)
set by user will be lost.
This patch make the adjustment persistent for persistent domain.
Signed-off-by: Wang Rui <moon.wangrui(a)huawei.com>
---
I'm not sure about the current purpose so I sent a RFC patch. Is it
for some reason that RTC change from guest isn't saved to persistent
config ?
I have tested this patch by changing RTC, starting, shutting down and
migrating. It seems good. There's only a nit I have found. Some guests
will set hardware clock to current system time when shut down. So if
hardware clock is different from system clock(the difference doesn't
come from user setting, maybe from clock shift by system), the
persistent config file will be saved to a new adjustment after shutdown
with this patch. But I think the hypervisor or guest OS should fix
the nit, not libvirt.
---
src/qemu/qemu_process.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 407da5e..b03bf02 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -863,7 +863,13 @@ qemuProcessHandleRTCChange(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
vm->def->clock.data.variable.adjustment = offset;
if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
- VIR_WARN("unable to save domain status with RTC change");
+ VIR_WARN("unable to save domain status with RTC change");
+
+ if (vm->persistent && vm->newDef) {
+ vm->newDef->clock.data.variable.adjustment = offset;
+ if (virDomainSaveConfig(cfg->configDir, vm->newDef) < 0)
+ VIR_WARN("unable to save domain config with RTC change");
+ }
}
event = virDomainEventRTCChangeNewFromObj(vm, offset);
--
1.7.12.4
12:40 p.m.
On 08/20/2014 09:00 AM, Wang Rui wrote:
Domain's clock xml is as below.
<clock offset='variable' basis='utc' adjustment='10'/>
If the guest modifies its RTC, libvirt will hanlde the time offset
and save the active status in qemuProcessHandleRTCChange(). However,
libvirt won't save the persistent config. So next time when vm is
restarted(shutdown and start), the time adjuestment(RTC change)
set by user will be lost.
This patch make the adjustment persistent for persistent domain.
Signed-off-by: Wang Rui <moon.wangrui(a)huawei.com>
---
I'm not sure about the current purpose so I sent a RFC patch. Is it
for some reason that RTC change from guest isn't saved to persistent
config ?
I have tested this patch by changing RTC, starting, shutting down and
migrating. It seems good. There's only a nit I have found. Some guests
will set hardware clock to current system time when shut down. So if
hardware clock is different from system clock(the difference doesn't
come from user setting, maybe from clock shift by system), the
persistent config file will be saved to a new adjustment after shutdown
with this patch. But I think the hypervisor or guest OS should fix
the nit, not libvirt.
---
src/qemu/qemu_process.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 407da5e..b03bf02 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -863,7 +863,13 @@ qemuProcessHandleRTCChange(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
vm->def->clock.data.variable.adjustment = offset;
if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
- VIR_WARN("unable to save domain status with RTC change");
+ VIR_WARN("unable to save domain status with RTC change");
+
+ if (vm->persistent && vm->newDef) {
+ vm->newDef->clock.data.variable.adjustment = offset;
+ if (virDomainSaveConfig(cfg->configDir, vm->newDef) < 0)
+ VIR_WARN("unable to save domain config with RTC change");
+ }
Sorry for the delay. I've finally paged back in (parts of) what I
learned about this when I fixed
https://bugzilla.redhat.com/show_bug.cgi?id=964177
(and my head now hurts accordingly :-P). In this message:
https://www.redhat.com/archives/libvir-list/2014-May/msg00832.html
I do point out the existence of exactly the problem that you are fixing.
However, there is a problem with the way you're fixing it if the clock
for the domain is set like this:
<clock offset ='variable' basis='localtime'/>
The problem is that when the domain was started, its *state* (but not
persistent config) was modified to convert the localtime adjustment into
an adjustment relative to UTC (and the basis type in the state was of
course also changed from VIR_DOMAIN_BASIS_TYPE_LOCALTIME to
VIR_DOMAIN_BASIS_TYPE_UTC). See this commit for details:
http://libvirt.org/git/?p=libvirt.git;a=commit;h=cde8ca2dfda6b69eebb70695...
(there is also some discussion of why this was done in the mailing list
archives for that patch and for others in the same series).
So if you just copy that UTC-based adjustment into the persistent
config, it will now be incorrect by the difference between UTC and
localtime. Even doing the math to "adjust the adjustment" by that amount
would end up with an incorrect result if the domain passed through a
daylight time to normal time boundary while it was running (including
migrating from a host in one timezone to a host in another timezone that
currently had a different setting for DST). And you can't modify the
basis type in the persistent config because... well, just *because* :-)
(seriously, I think that would be taking too many liberties with the
user-supplied config).
On the other hand, the documentation for <clock offset='variable' ...>
specifically says that a change to the RTC will be preserved across
domain restarts (well, it says "reboots", which technically is different
as it doesn't involve terminating and restarting the qemu process, but I
think we all know what the author intended to say there).
So, I'm okay with what this patch does *iff* basis='utc', but before we
can push it we need to do *something* for basis='localtime'. The two
options I can see would be:
1) adjust the adjustment by the difference between UTC and localtime
(basically do the opposite of what is done in the patch referenced
above) - this would render correct results as long as the current status
of DST is the same as *what it will be the next time the domain is
re-started* (ugh - how to tell the future...)
2) admit defeat and punt; simply do nothing (i.e. don't update or save
the persistent config) in the case of basis='localtime'.
Option (1) seems better in some ways, because it's at least trying to do
the right thing. On the other hand, it is also giving a false sense of
correctness when that correctness can't be guaranteed. When you think
about it, the entire idea of <clock offset='variable'
basis='localtime'/> is broken and I don't think it's possible to fix
it
100% while allowing for a) migration between machines in different
timezones, and b) modifying the RTC when the status of DST is different
than it was when the domain started, so part of my brain wants to say
"let's just admit that we can't get this right, and make sure it's
obvious by making it *totally* wrong" (i.e. option (2)). (It would
probably also be a good idea to update the documentation in
formatdomain.html.in to reflect that using this type of clock is a very
bad idea unless you never plan to migrate, and live in a location that
doesn't have DST).
Does anyone else have an opinion/idea about this?
5:11 a.m.
On 2014/9/5 1:40, Laine Stump wrote:
I do point out the existence of exactly the problem that you are
fixing.
However, there is a problem with the way you're fixing it if the clock
for the domain is set like this:
<clock offset ='variable' basis='localtime'/>
The problem is that when the domain was started, its *state* (but not
persistent config) was modified to convert the localtime adjustment into
an adjustment relative to UTC (and the basis type in the state was of
course also changed from VIR_DOMAIN_BASIS_TYPE_LOCALTIME to
VIR_DOMAIN_BASIS_TYPE_UTC). See this commit for details:
http://libvirt.org/git/?p=libvirt.git;a=commit;h=cde8ca2dfda6b69eebb70695...
Thanks to your reply.
I think I understand what you mean. If basis is 'localtime', the active config
is modified to basis='utc' and of course 'adjustment' is adjusted to a new
offset
which is relative to UTC.
But I met the opposite result.
My host's timezone was CST(UTC +8:00). Clock configuration is
<clock offset='variable' adjustment='120'
basis='localtime'>.
I started the VM and found the unexpected result.
In the active config(/var/run/libvirt/qemu/*.xml), basis was utc,
but adjustment was 120(which was expected to 120 + 8*3600).
In the persistent config, basis was localtime and adjustment was 120.
Use gdb and set breakpoint to qemuProcessHandleRTCChange. Adjust the VM's
hardware clock. The result also shows me the state(active config) is not
right.
(gdb) p vm->def->clock.data
$3 = {utc_reset = 120, variable = {adjustment = 120, basis = 0, adjustment0 = 120},
timezone = 0x78 <Address 0x78 out of bounds>}
(gdb) p vm->newDef->clock.data
$4 = {utc_reset = 120, variable = {adjustment = 120, basis = 1, adjustment0 = 0}, timezone
= 0x78 <Address 0x78 out of bounds>}
The difference between vm->def and vm->newDef is there clock.data.variable.basis.
Your patch[1] modified the basis to utc, but it seems to only take effect to active
config.
[1]http://libvirt.org/git/?p=libvirt.git;a=commit;h=cde8ca2dfda6b69eebb706...
(there is also some discussion of why this was done in the mailing
list
archives for that patch and for others in the same series).
So if you just copy that UTC-based adjustment into the persistent
config, it will now be incorrect by the difference between UTC and
localtime. Even doing the math to "adjust the adjustment" by that amount
would end up with an incorrect result if the domain passed through a
daylight time to normal time boundary while it was running (including
migrating from a host in one timezone to a host in another timezone that
currently had a different setting for DST). And you can't modify the
basis type in the persistent config because... well, just *because* :-)
(seriously, I think that would be taking too many liberties with the
user-supplied config).
On the other hand, the documentation for <clock offset='variable' ...>
specifically says that a change to the RTC will be preserved across
domain restarts (well, it says "reboots", which technically is different
as it doesn't involve terminating and restarting the qemu process, but I
think we all know what the author intended to say there).
If my result above it true(if I don't miss anything), I think we should
resolve the issue of active config not the persistent one.
And fortunately my patch seems to do the right thing because of the
localtime-basis adjustment in persistent config.
3731
days inactive
3747
days old
2 comments
2 participants
participants (2)
-
Laine Stump -
Wang Rui