On 02/09/2012 04:36 AM, Philipp Hahn wrote:
Before version 3.1 xen only implemented clock/@offset='utc'
and 'localtime'.
With the introduction of managed domains in 3.1 xend keeps track of the
rtc_timeoffset, even over reboots. This translates to libvirts
clock/@offset='variable' variant. Be advised that only HV domains have a RTC.
In addition xen also supports a variant where the offset is tracked to
'localtime', which is currently not supported by libvirt. To make matters
worse, this was somehow broken in some versions of xen and was finally fixed
with version xen-3.4.
The following patch set ...
* adds support for handling variable offsets relative to localtime,
* fixes libvirt to use clock/@offset='variable' for newer xen versions,
* adapts the test suit accordingly
I've tested this on CenOS5 (xend-3.0.3 + 3.1.2 hypervisor?), UCS-2.3
(xen-3.2.1), UCS-2.4 (xen-3.4.3) and UCS-3.0 (xen-4.1.2).
Since v1:
+ fix handling of direct-PV-domains
+ added handling of localtime=1 + rtc_timeoffset
+ fixed test suite
Questions:
1. Handling of legacy XML domain configurations
I'm reluctant to define LIBVIRT_XEND_CLOCK_STRICT, since this would probably
break many existing XML configuration files, since they would be rejected
libvirt now for still using clock/@offset='utc'/'localtime'. Luckily
for us
libvirt doesn't support snapshots with xen, because then this XML
configuration would be stored in the snapshot meta data, rendering them all
invalid.
We absolutely cannot reject existing older xml files; but must load them
with the semantics that make sense.
If I understand correctly, the situation is that old clients had:
<clock offset='localtime'/>
whereas the semantics are more correctly listed as your proposed:
<clock offset='variable' adjustment='nnn'
basis='localtime'/>
and you are worried that if you rewrite the former into the latter, you
are preserving the semantics that make the most sense as a default, but
you are making it impossible to implement alternate semantics that are
also possible with xen and which are documented as the current behavior.
Your idea was to compile two versions of libvirt, either with or
without -DLIBVIRT_XEND_CLOCK_STRICT.
But a better idea is to represent the XML in a way where the default
conversion makes sense, but where a user can explicitly change the XML
to get what they want. I'm thinking:
If offset is 'utc' or 'localtime', we add a new attribute
'adjustment',
whose value can be either 'reset' to signify that the offset is reset on
each boot, or a timeDelta, as shorthand for specifying offset='variable'
adjustment='nnn' basis='utc|localtime'. If the attribute is missing,
the hypervisor determines whether the default is 'reset' or '0'. Qemu
would pick 'reset', xend would pick '0'.
That way, we would convert domains by default, but a user that _wants_
the documented reset abilities can add adjustment='reset' to their XML
and force us to leave things alone, without having a compilation switch.
When writing XML back out via dumpxml, libvirt would generate either the
offset='utc' adjustment='reset', or the offset='variable'
adjustment='nnn' basis='utc'; and would not generate offset='utc'
adjustment='nnn', even though that is a valid input form.
That will touch a bit more in the testsuite (all qemu tests would also
end up being converted by the new output), but would make things more
specific, and still give the user runtime control over XML behavior
without having to resort to recompilation of libvirt with a new compiler
macro.
RNG-wise, this would be:
<element name='clock'>
<choice>
<group>
<attribute name='offset'>
<choice>
<value>localtime</value>
<value>utc</value>
</choice>
</attribute>
<optional>
<attribute name='adjustment'>
<choice>
<ref name='timeDelta'/>
<value>reset</value>
</choice>
</attribute>
</optional>
</group>
<group>
<attribute name='offset'>
<value>timezone</value>
</attribute>
<optional>
<attribute name='timezone'>
<ref name='timeZone'/>
</attribute>
</optional>
</group>
<group>
<attribute name='offset'>
<value>variable</value>
</attribute>
<optional>
<attribute name='adjustment'>
<ref name='timeDelta'/>
</attribute>
</optional>
</group>
</choice>
<zeroOrMore>
<ref name='timer'/>
</zeroOrMore>
</element>
2. Error path handling
There are now a lot of different cases (PV, HV<3.1, HV>3.1), which all have
the same error path. This is somehow ugly to code in C, I hope it's okay to
use vmlocaltime=-1 to indicate an error instead of using "goto". But if you
have a better idea, please feel free to comments.
I didn't look closely at that part; I'd rather get consensus on the XML
first.
3. libvirt-0.9.10 or .11
This patch-series should probably receive some more testing, so for 0.9.10
it's too late. If it's not included, I'd need to update the reference to
the
version when this patch will get in.
Agreed on this point - we are now at post-0.9.10 material.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library
http://libvirt.org