[libvirt] [PATCHv2 4/4] qemu: fix <clock offset='variable' basis='localtime'/>

[Laine Stump]

For a clock element as above, libvirt simply converts current system
time with localtime_r(), then starts qemu with a time string that
doesn't contain any timezone information. So, from qemu's point of
view, the -rtc string it gets for:

   <clock offset='variable' basis='utc' adjustment='10800'/>

is identical to the -rtc string it gets for:

   <clock offset='variable' basis='localtime' adjustment='0'/>

(assuming the host is in a timezone that is 10800 seconds ahead of
UTC, as is the case on the machine where this message is being
written).

Since the commandlines are identical, qemu will behave identically
after this point in either case.

There are two problems in the case of basis='localtime' though:

Problem 1) If the guest modifies its RTC, for example to add 20
seconds, the RTC_CHANGE event from qemu will then contain offset:20 in
both cases. But libvirt will have saved the original adjustment into
adjustment0, and will add that value onto the offset in the
event. This means that in the case of basis=;utc', it will properly
emit an event with offset:10820, but in the case of basis='localtime'
the event will contain offset:20, which is *not* the new offset of the
RTC from UTC (as the event it documented to provide).

Problem 2) If the guest is migrated to another host that is in a
different timezone, or if it is migrated or saved/restored after the
DST status has changed from what it was when the guest was originally
started, the newly restarted guest will have a different RTC (since it
will be based on the new localtime, which could have shifted by
several hours).

The solution to both of these problems is simple - rather than
maintaining the original adjustment value along with
"basis='localtime'" in the domain status, when the domain is started
we convert the adjustment offset to one relative to UTC, and set the
status to "basis='utc'". Thus, whatever the RTC offset was from UTC
when it was initially started, that offset will be maintained when
migrating across timezones and DST settings, and the RTC_CHANGE events
will automatically contain the proper offset (which should by
definition always be relative to UTC).

This fixes a problem that was implied but not openly stated in:

  https://bugzilla.redhat.com/show_bug.cgi?id=964177
---
New in V2

 src/qemu/qemu_command.c | 33 ++++++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 4998bca..28885f2 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5976,19 +5976,30 @@ qemuBuildClockArgStr(virDomainClockDefPtr def)
         time_t now = time(NULL);
         struct tm nowbits;
 
-        switch ((enum virDomainClockBasis) def->data.variable.basis) {
-        case VIR_DOMAIN_CLOCK_BASIS_UTC:
-            now += def->data.variable.adjustment;
-            gmtime_r(&now, &nowbits);
-            break;
-        case VIR_DOMAIN_CLOCK_BASIS_LOCALTIME:
-            now += def->data.variable.adjustment;
-            localtime_r(&now, &nowbits);
-            break;
-        case VIR_DOMAIN_CLOCK_BASIS_LAST:
-            break;
+        if (def->data.variable.basis == VIR_DOMAIN_CLOCK_BASIS_LOCALTIME) {
+            time_t localOffset;
+
+            /* in the case of basis='localtime', rather than trying to
+             * keep that basis (and associated offset from UTC) in the
+             * status and deal with adding in the difference each time
+             * there is an RTC_CHANGE event, it is simpler and less
+             * error prone to just convert the adjustment an offset
+             * from UTC right now (and change the status to
+             * "basis='utc' to reflect this). This eliminates
+             * potential errors in both RTC_CHANGE events and in
+             * migration (in the case that the status of DST, or the
+             * timezone of the destination host, changed relative to
+             * startup).
+             */
+            if (virTimeLocalOffsetFromUTC(&localOffset) < 0)
+               goto error;
+            def->data.variable.adjustment += localOffset;
+            def->data.variable.basis = VIR_DOMAIN_CLOCK_BASIS_UTC;
         }
 
+        now += def->data.variable.adjustment;
+        gmtime_r(&now, &nowbits);
+
         /* when an RTC_CHANGE event is received from qemu, we need to
          * have the adjustment used at domain start time available to
          * compute the new offset from UTC. As this new value is
-- 
1.9.0