Since there isn't a single libc API to get this value, this patch supplies one which gets the value by grabbing current UTC, then converting that into a struct tm with localtime_r(), then back to a time_t using mktime; it again does the same operation, but using gmtime_r() instead (for UTC). It then subtracts utc time from the localtime, and finally adjusts if dst is set in the localtime timeinfo (because for some reason mktime doesn't take that into account). This function should be POSIX-compliant, and is threadsafe, but not async signal safe. If it was ever necessary to know this value in a child process, we could cache it with a one-time init function when libvirtd starts, then just supply the cached value, but that complexity isn't needed for current usage. --- src/libvirt_private.syms | 1 + src/util/virtime.c | 41 ++++++++++++++++++++++++++++++++++++++++- src/util/virtime.h | 5 +++-- 3 files changed, 44 insertions(+), 3 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c3332c9..cb635cd 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1984,6 +1984,7 @@ virTimeFieldsNow; virTimeFieldsNowRaw; virTimeFieldsThen; virTimeFieldsThenRaw; +virTimeLocalOffsetFromUTC; virTimeMillisNow; virTimeMillisNowRaw; virTimeStringNow; diff --git a/src/util/virtime.c b/src/util/virtime.c index caa4e24..c487eba 100644 --- a/src/util/virtime.c +++ b/src/util/virtime.c @@ -1,7 +1,7 @@ /* * virtime.c: Time handling functions * - * Copyright (C) 2006-2012 Red Hat, Inc. + * Copyright (C) 2006-2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -344,3 +344,42 @@ char *virTimeStringThen(unsigned long long when) return ret; } + +/** + * virTimeLocalOffsetFromUTC: + * + * This function is threadsafe, but is *not* async signal safe (due to + * localtime_r()). + * + * @offset: pointer to time_t that will difference between localtime + * and UTC in seconds. + * + * Returns 0 on success, -1 on error with error reported + */ +int +virTimeLocalOffsetFromUTC(time_t *offset) +{ + struct tm gmtimeinfo, localtimeinfo; + time_t current, utc, local; + + if ((current = time(NULL)) < 0) { + virReportSystemError(errno, "%s", + _("failed to get current system time")); + return -1; + } + + localtime_r(&current, &localtimeinfo); + gmtime_r(&current, &gmtimeinfo); + + if ((local = mktime(&localtimeinfo)) < 0 || + (utc = mktime(&gmtimeinfo)) < 0) { + virReportSystemError(errno, "%s", + _("mktime failed")); + return -1; + } + + *offset = local - utc; + if (localtimeinfo.tm_isdst) + *offset += 3600; + return 0; +} diff --git a/src/util/virtime.h b/src/util/virtime.h index a5bd652..cda9707 100644 --- a/src/util/virtime.h +++ b/src/util/virtime.h @@ -1,7 +1,7 @@ /* * virtime.h: Time handling functions * - * Copyright (C) 2006-2011 Red Hat, Inc. + * Copyright (C) 2006-2011, 2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -50,7 +50,6 @@ int virTimeStringNowRaw(char *buf) int virTimeStringThenRaw(unsigned long long when, char *buf) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; - /* These APIs are *not* async signal safe and return -1, * raising a libvirt error on failure */ @@ -63,5 +62,7 @@ int virTimeFieldsThen(unsigned long long when, struct tm *fields) char *virTimeStringNow(void); char *virTimeStringThen(unsigned long long when); +int virTimeLocalOffsetFromUTC(time_t *offset) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; #endif -- 1.9.0

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. In the case of a clock that has offset='variable' basis='localtime', the original adjustment in the XML will not have accounted for UTC (i.e. it will be the offset of the domain's RTC from *local* time). So although the value we want to save back into adjustment is still the addition of "latest offset from qemu" + adjustment0, we still need to adjust that value before sending to an event handler; in order to convert the offset from localtime into offset from UTC, we need to add the difference between UTC and localtime. (NB: to eliminate confusion caused by a change in daylight savings status between when the domain was started and when the RTC is changed, the offset from UTC to localtime is retrieved only once when the domain is started, and saved in the domain's status as "localtimeAdjustment0"). This patch (*not* e31b5cf393857, which should be reverted prior to applying this patch) fixes: https://bugzilla.redhat.com/show_bug.cgi?id=964177 --- src/conf/domain_conf.c | 27 +++++++++++++++++++++++---- src/conf/domain_conf.h | 8 ++++++++ src/qemu/qemu_command.c | 13 +++++++++++++ src/qemu/qemu_process.c | 40 +++++++++++++++++++++++++++++++++++----- 4 files changed, 79 insertions(+), 9 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a7863c3..9c6c80c 100644 --- 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, @@ -12002,6 +12003,12 @@ 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; + if (virXPathInt("number(./clock/@localtimeAdjustment0)", ctxt, + &def->clock.data.variable.localtimeAdjustment0) < 0) + def->clock.data.variable.localtimeAdjustment0 = 0; tmp = virXPathString("string(./clock/@basis)", ctxt); if (tmp) { if ((def->clock.data.variable.basis = virDomainClockBasisTypeFromString(tmp)) < 0) { @@ -17086,7 +17093,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_*, @@ -17108,7 +17116,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))) { @@ -17642,6 +17651,14 @@ 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); + if (def->clock.data.variable.localtimeAdjustment0) + virBufferAsprintf(buf, " localtimeAdjustment0='%d'", + def->clock.data.variable.localtimeAdjustment0); + } break; case VIR_DOMAIN_CLOCK_OFFSET_TIMEZONE: virBufferEscapeString(buf, " timezone='%s'", def->clock.data.timezone); @@ -18072,7 +18089,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; @@ -18160,7 +18178,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 94285e3..6e45915 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1727,6 +1727,14 @@ 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; + int localtimeAdjustment0; } variable; /* Timezone name, when diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 95f747f..ffec3ad 100644 --- 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" @@ -5974,6 +5975,15 @@ qemuBuildClockArgStr(virDomainClockDefPtr def) case VIR_DOMAIN_CLOCK_OFFSET_VARIABLE: { time_t now = time(NULL); struct tm nowbits; + time_t localOffset; + + /* 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; switch ((enum virDomainClockBasis) def->data.variable.basis) { case VIR_DOMAIN_CLOCK_BASIS_UTC: @@ -5983,6 +5993,9 @@ qemuBuildClockArgStr(virDomainClockDefPtr def) case VIR_DOMAIN_CLOCK_BASIS_LOCALTIME: now += def->data.variable.adjustment; localtime_r(&now, &nowbits); + if (virTimeLocalOffsetFromUTC(&localOffset) < 0) + goto error; + def->data.variable.localtimeAdjustment0 = localOffset; break; case VIR_DOMAIN_CLOCK_BASIS_LAST: break; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 6dcb737..f618462 100644 --- 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,44 @@ 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 (vm->def->clock.data.variable.basis == VIR_DOMAIN_CLOCK_BASIS_LOCALTIME) { + /* in the case of basis='localtime', the adjustment stored + * in the domain status contains the adjustment to move + * the RTC to localtime, *not* UTC. But the offset sent in + * the RTCChange event is defined to always send the new + * offset from UTC, regardless of libvirt domain clock + * configuration. So we must add the additional offset + * between localtime and UTC to the offset sent in the + * event. + */ + offset += vm->def->clock.data.variable.localtimeAdjustment0; + } + + if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) + VIR_WARN("unable to save domain status with RTC change"); + } + + event = virDomainEventRTCChangeNewFromObj(vm, offset); virObjectUnlock(vm); -- 1.9.0