qemu: fix RTC_CHANGE event for <clock offset='variable' basis='utc'/>

commit e31b5cf393857 attempted to fix libvirt's
VIR_DOMAIN_EVENT_ID_RTC_CHANGE, which is documentated to always
provide the new offset of the domain's real time clock from UTC. The
problem was that, in the case that qemu is provided with an "-rtc
base=x" where x is an absolute time (rather than "utc" or
"localtime"), the offset sent by qemu's RTC_CHANGE event is *not* the
new offset from UTC, but rather is the sum of all changes to the
domain's RTC since it was started with base=x.

So, despite what was said in commit e31b5cf393857, if we assume that
the original value stored in "adjustment" was the offset from UTC at
the time the domain was started, we can always determine the current
offset from UTC by simply adding the most recent (i.e. current) offset
from qemu to that original adjustment.

This patch accomplishes that by storing the initial adjustment in the
domain's status as "adjustment0". Each time a new RTC_CHANGE event is
received from qemu, we simply add adjustment0 to the value sent by
qemu, store that as the new adjustment, and forward that value on to
any event handler.

This patch (*not* e31b5cf393857, which should be reverted prior to
applying this patch) fixes:

https://bugzilla.redhat.com/show_bug.cgi?id=964177

(for the case where basis='utc'. It does not fix basis='localtime')

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 437c7457f03fea003d144acbef8a52b765893050..f1df0929ad68dc8029acc0e5cc858bbfbb41b457 100644 (file)
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -99,6 +99,7 @@ typedef enum {
    VIR_DOMAIN_XML_INTERNAL_PCI_ORIG_STATES = (1<<18),
    VIR_DOMAIN_XML_INTERNAL_ALLOW_ROM = (1<<19),
    VIR_DOMAIN_XML_INTERNAL_ALLOW_BOOT = (1<<20),
+   VIR_DOMAIN_XML_INTERNAL_CLOCK_ADJUST = (1<<21),
 } virDomainXMLInternalFlags;
 
 VIR_ENUM_IMPL(virDomainTaint, VIR_DOMAIN_TAINT_LAST,
@@ -12023,6 +12024,9 @@ virDomainDefParseXML(xmlDocPtr xml,
         if (virXPathLongLong("number(./clock/@adjustment)", ctxt,
                              &def->clock.data.variable.adjustment) < 0)
             def->clock.data.variable.adjustment = 0;
+        if (virXPathLongLong("number(./clock/@adjustment0)", ctxt,
+                             &def->clock.data.variable.adjustment0) < 0)
+            def->clock.data.variable.adjustment0 = 0;
         tmp = virXPathString("string(./clock/@basis)", ctxt);
         if (tmp) {
             if ((def->clock.data.variable.basis = virDomainClockBasisTypeFromString(tmp)) < 0) {
@@ -17117,7 +17121,8 @@ virDomainResourceDefFormat(virBufferPtr buf,
 
 verify(((VIR_DOMAIN_XML_INTERNAL_STATUS |
          VIR_DOMAIN_XML_INTERNAL_ACTUAL_NET |
-         VIR_DOMAIN_XML_INTERNAL_PCI_ORIG_STATES)
+         VIR_DOMAIN_XML_INTERNAL_PCI_ORIG_STATES |
+         VIR_DOMAIN_XML_INTERNAL_CLOCK_ADJUST)
         & DUMPXML_FLAGS) == 0);
 
 /* This internal version can accept VIR_DOMAIN_XML_INTERNAL_*,
@@ -17139,7 +17144,8 @@ virDomainDefFormatInternal(virDomainDefPtr def,
     virCheckFlags(DUMPXML_FLAGS |
                   VIR_DOMAIN_XML_INTERNAL_STATUS |
                   VIR_DOMAIN_XML_INTERNAL_ACTUAL_NET |
-                  VIR_DOMAIN_XML_INTERNAL_PCI_ORIG_STATES,
+                  VIR_DOMAIN_XML_INTERNAL_PCI_ORIG_STATES |
+                  VIR_DOMAIN_XML_INTERNAL_CLOCK_ADJUST,
                   -1);
 
     if (!(type = virDomainVirtTypeToString(def->virtType))) {
@@ -17673,6 +17679,11 @@ virDomainDefFormatInternal(virDomainDefPtr def,
         virBufferAsprintf(buf, " adjustment='%lld' basis='%s'",
                           def->clock.data.variable.adjustment,
                           virDomainClockBasisTypeToString(def->clock.data.variable.basis));
+        if (flags & VIR_DOMAIN_XML_INTERNAL_CLOCK_ADJUST) {
+            if (def->clock.data.variable.adjustment0)
+                virBufferAsprintf(buf, " adjustment0='%lld'",
+                                  def->clock.data.variable.adjustment0);
+        }
         break;
     case VIR_DOMAIN_CLOCK_OFFSET_TIMEZONE:
         virBufferEscapeString(buf, " timezone='%s'", def->clock.data.timezone);
@@ -18103,7 +18114,8 @@ virDomainSaveStatus(virDomainXMLOptionPtr xmlopt,
     unsigned int flags = (VIR_DOMAIN_XML_SECURE |
                           VIR_DOMAIN_XML_INTERNAL_STATUS |
                           VIR_DOMAIN_XML_INTERNAL_ACTUAL_NET |
-                          VIR_DOMAIN_XML_INTERNAL_PCI_ORIG_STATES);
+                          VIR_DOMAIN_XML_INTERNAL_PCI_ORIG_STATES |
+                          VIR_DOMAIN_XML_INTERNAL_CLOCK_ADJUST);
 
     int ret = -1;
     char *xml;
@@ -18191,7 +18203,8 @@ virDomainObjListLoadStatus(virDomainObjListPtr doms,
     if (!(obj = virDomainObjParseFile(statusFile, caps, xmlopt, expectedVirtTypes,
                                       VIR_DOMAIN_XML_INTERNAL_STATUS |
                                       VIR_DOMAIN_XML_INTERNAL_ACTUAL_NET |
-                                      VIR_DOMAIN_XML_INTERNAL_PCI_ORIG_STATES)))
+                                      VIR_DOMAIN_XML_INTERNAL_PCI_ORIG_STATES |
+                                      VIR_DOMAIN_XML_INTERNAL_CLOCK_ADJUST)))
         goto error;
 
     virUUIDFormat(obj->def->uuid, uuidstr);

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 94285e3438ef584131ad7f6ec06a8c8e71bac52e..8f17c74010fcef10cfc524a35c129b3ae90fbfbc 100644 (file)
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1727,6 +1727,13 @@ struct _virDomainClockDef {
         struct {
             long long adjustment;
             int basis;
+
+            /* domain start-time adjustment. This is a
+             * private/internal read-only value that only exists when
+             * a domain is running, and only if the clock
+             * offset='variable'
+             */
+            long long adjustment0;
         } variable;
 
         /* Timezone name, when

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index ddf83299b0c00e94fd7afab33c1de0bfa2309ca8..9ba7b9377763da7cc5ab097862185061e7827995 100644 (file)
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -36,6 +36,7 @@
 #include "virfile.h"
 #include "virnetdev.h"
 #include "virstring.h"
+#include "virtime.h"
 #include "viruuid.h"
 #include "c-ctype.h"
 #include "domain_nwfilter.h"
@@ -5999,6 +6000,14 @@ qemuBuildClockArgStr(virDomainClockDefPtr def)
             break;
         }
 
+        /* 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
+         * itself stored in def->data.variable.adjustment, we need to
+         * save a copy of it now.
+        */
+        def->data.variable.adjustment0 = def->data.variable.adjustment;
+
         virBufferAsprintf(&buf, "base=%d-%02d-%02dT%02d:%02d:%02d",
                           nowbits.tm_year + 1900,
                           nowbits.tm_mon + 1,

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index bf354626a6ebe901449e93f59a826d75555f5e85..510fd8693f9ddf612acdf067f145ff621ebba2dc 100644 (file)
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -831,7 +831,6 @@ qemuProcessHandleResume(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
     return 0;
 }
 
-
 static int
 qemuProcessHandleRTCChange(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
                            virDomainObjPtr vm,
@@ -843,13 +842,31 @@ qemuProcessHandleRTCChange(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
     virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
 
     virObjectLock(vm);
-    event = virDomainEventRTCChangeNewFromObj(vm, offset);
 
-    if (vm->def->clock.offset == VIR_DOMAIN_CLOCK_OFFSET_VARIABLE)
+    if (vm->def->clock.offset == VIR_DOMAIN_CLOCK_OFFSET_VARIABLE) {
+        /* when a basedate is manually given on the qemu commandline
+         * rather than simply "-rtc base=utc", the offset sent by qemu
+         * in this event is *not* the new offset from UTC, but is
+         * instead the new offset from the *original basedate* +
+         * uptime. For example, if the original offset was 3600 and
+         * the guest clock has been advanced by 10 seconds, qemu will
+         * send "10" in the event - this means that the new offset
+         * from UTC is 3610, *not* 10. If the guest clock is advanced
+         * by another 10 seconds, qemu will now send "20" - i.e. each
+         * event is the sum of the most recent change and all previous
+         * changes since the domain was started. Fortunately, we have
+         * saved the initial offset in "adjustment0", so to arrive at
+         * the proper new "adjustment", we just add the most recent
+         * offset to adjustment0.
+         */
+        offset += vm->def->clock.data.variable.adjustment0;
         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");
+        if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
+           VIR_WARN("unable to save domain status with RTC change");
+    }
+
+    event = virDomainEventRTCChangeNewFromObj(vm, offset);
 
     virObjectUnlock(vm);