[libvirt] [PATCH v2 0/2] qemu: Refresh guest's RTC on daemon restart

v2 of: https://www.redhat.com/archives/libvir-list/2016-April/msg02001.html diff to v1: -Peter's review suggestions worked in Michal Privoznik (2): qemu: Introduce qemuMonitorGetRTCTime qemu: Refresh RTC adjustment on qemuProcessReconnect src/qemu/qemu_monitor.c | 11 +++++++++++ src/qemu/qemu_monitor.h | 3 +++ src/qemu/qemu_monitor_json.c | 41 +++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 3 +++ src/qemu/qemu_process.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 102 insertions(+) -- 2.8.1

Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_monitor.c | 11 +++++++++++ src/qemu/qemu_monitor.h | 3 +++ src/qemu/qemu_monitor_json.c | 41 +++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 3 +++ 4 files changed, 58 insertions(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 0170850..1f402ee 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3726,3 +3726,14 @@ qemuMonitorMigrateStartPostCopy(qemuMonitorPtr mon) return qemuMonitorJSONMigrateStartPostCopy(mon); } + +int +qemuMonitorGetRTCTime(qemuMonitorPtr mon, + struct tm *tm) +{ + VIR_DEBUG("mon=%p", mon); + + QEMU_CHECK_MONITOR_JSON(mon); + + return qemuMonitorJSONGetRTCTime(mon, tm); +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 871ebe0..4d8f21f 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -933,4 +933,7 @@ int qemuMonitorMigrateIncoming(qemuMonitorPtr mon, int qemuMonitorMigrateStartPostCopy(qemuMonitorPtr mon); +int qemuMonitorGetRTCTime(qemuMonitorPtr mon, + struct tm *tm); + #endif /* QEMU_MONITOR_H */ diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index e0dbda1..a48a263 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -6905,3 +6905,44 @@ qemuMonitorJSONMigrateStartPostCopy(qemuMonitorPtr mon) virJSONValueFree(reply); return ret; } + +int +qemuMonitorJSONGetRTCTime(qemuMonitorPtr mon, + struct tm *tm) +{ + int ret = -1; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + virJSONValuePtr data; + + if (!(cmd = qemuMonitorJSONMakeCommand("qom-get", + "s:path", "/machine", + "s:property", "rtc-time", + NULL))) + return -1; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; + + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + + data = virJSONValueObjectGet(reply, "return"); + + if (virJSONValueObjectGetNumberInt(data, "tm_year", &tm->tm_year) < 0 || + virJSONValueObjectGetNumberInt(data, "tm_mon", &tm->tm_mon) < 0 || + virJSONValueObjectGetNumberInt(data, "tm_mday", &tm->tm_mday) < 0 || + virJSONValueObjectGetNumberInt(data, "tm_hour", &tm->tm_hour) < 0 || + virJSONValueObjectGetNumberInt(data, "tm_min", &tm->tm_min) < 0 || + virJSONValueObjectGetNumberInt(data, "tm_sec", &tm->tm_sec) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("qemu returned malformed time")); + goto cleanup; + } + + ret = 0; + cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 1587a03..b7aff73 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -483,4 +483,7 @@ int qemuMonitorJSONMigrateIncoming(qemuMonitorPtr mon, int qemuMonitorJSONMigrateStartPostCopy(qemuMonitorPtr mon) ATTRIBUTE_NONNULL(1); +int qemuMonitorJSONGetRTCTime(qemuMonitorPtr mon, + struct tm *tm) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); #endif /* QEMU_MONITOR_JSON_H */ -- 2.8.1

On Tue, May 03, 2016 at 10:33:42 +0200, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_monitor.c | 11 +++++++++++ src/qemu/qemu_monitor.h | 3 +++ src/qemu/qemu_monitor_json.c | 41 +++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 3 +++ 4 files changed, 58 insertions(+)
ACK

https://bugzilla.redhat.com/show_bug.cgi?id=1139766 Thing is, for some reasons you can have your domain's RTC to be in something different than UTC. More weirdly, it's not only time zone what you can shift it of, but an arbitrary value. So, if domain is configured that way, libvirt will correctly put it onto qemu cmd line and moreover track it as this offset changes during domain's life time (e.g. because guest OS decides the best thing to do is set new time to RTC). Anyway, they way in which this tracking is implemented is events. But we've got a problem if change in guest's RTC occurs and the daemon is not running. The event is lost and we end up reporting invalid value in domain XML. Therefore, when the daemon is starting up again and it is reconnecting to all running domains, re-fetch their RTC so the correct offset value can be computed. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_process.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 527300a..220a803 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1992,6 +1992,47 @@ qemuRefreshVirtioChannelState(virQEMUDriverPtr driver, return ret; } +static int +qemuRefreshRTC(virQEMUDriverPtr driver, + virDomainObjPtr vm) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + time_t now, then; + struct tm thenbits; + long localOffset; + int ret, rv; + + if (vm->def->clock.offset != VIR_DOMAIN_CLOCK_OFFSET_VARIABLE) + return 0; + + memset(&thenbits, 0, sizeof(thenbits)); + qemuDomainObjEnterMonitor(driver, vm); + now = time(NULL); + rv = qemuMonitorGetRTCTime(priv->mon, &thenbits); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + rv = -1; + + if (rv < 0) + goto cleanup; + + thenbits.tm_isdst = -1; + if ((then = mktime(&thenbits)) == (time_t) -1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to convert time")); + goto cleanup; + } + + /* Thing is, @now is in local TZ but @then in UTC. */ + if (virTimeLocalOffsetFromUTC(&localOffset) < 0) + goto cleanup; + + vm->def->clock.data.variable.adjustment = then - now + localOffset; + + ret = 0; + + cleanup: + return ret; +} int qemuProcessRefreshBalloonState(virQEMUDriverPtr driver, @@ -3673,6 +3714,9 @@ qemuProcessReconnect(void *opaque) if (qemuRefreshVirtioChannelState(driver, obj) < 0) goto error; + /* If querying of guest's RTC failed, report error, but do not kill the domain. */ + ignore_value(qemuRefreshRTC(driver, obj)); + if (qemuProcessRefreshBalloonState(driver, obj, QEMU_ASYNC_JOB_NONE) < 0) goto error; -- 2.8.1

On Tue, May 03, 2016 at 10:33:43 +0200, Michal Privoznik wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1139766
Thing is, for some reasons you can have your domain's RTC to be in something different than UTC. More weirdly, it's not only time zone what you can shift it of, but an arbitrary value. So, if domain is configured that way, libvirt will correctly put it onto qemu cmd line and moreover track it as this offset changes during domain's life time (e.g. because guest OS decides the best thing to do is set new time to RTC). Anyway, they way in which this tracking is implemented is events. But we've got a problem if change in guest's RTC occurs and the daemon is not running. The event is lost and we end up reporting invalid value in domain XML. Therefore, when the daemon is starting up again and it is reconnecting to all running domains, re-fetch their RTC so the correct offset value can be computed.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_process.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 527300a..220a803 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1992,6 +1992,47 @@ qemuRefreshVirtioChannelState(virQEMUDriverPtr driver, return ret; }
+static int
Does this even need to return a value, since ... [1]
+qemuRefreshRTC(virQEMUDriverPtr driver, + virDomainObjPtr vm) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + time_t now, then; + struct tm thenbits; + long localOffset; + int ret, rv; + + if (vm->def->clock.offset != VIR_DOMAIN_CLOCK_OFFSET_VARIABLE) + return 0; + + memset(&thenbits, 0, sizeof(thenbits)); + qemuDomainObjEnterMonitor(driver, vm); + now = time(NULL); + rv = qemuMonitorGetRTCTime(priv->mon, &thenbits); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + rv = -1; + + if (rv < 0) + goto cleanup; + + thenbits.tm_isdst = -1; + if ((then = mktime(&thenbits)) == (time_t) -1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to convert time")); + goto cleanup; + } + + /* Thing is, @now is in local TZ but @then in UTC. */ + if (virTimeLocalOffsetFromUTC(&localOffset) < 0) + goto cleanup; + + vm->def->clock.data.variable.adjustment = then - now + localOffset; + + ret = 0; + + cleanup: + return ret; +}
int qemuProcessRefreshBalloonState(virQEMUDriverPtr driver, @@ -3673,6 +3714,9 @@ qemuProcessReconnect(void *opaque) if (qemuRefreshVirtioChannelState(driver, obj) < 0) goto error;
+ /* If querying of guest's RTC failed, report error, but do not kill the domain. */ + ignore_value(qemuRefreshRTC(driver, obj));
[1] ... you ignore it? Do you expect this function to be used anywhere else? If not then it doesn't make sense to return anything from the func. ACK with the return value removed. Peter
participants (2)
-
Michal Privoznik
-
Peter Krempa