[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] qemu: Issue rtc_reset_reinjection command after guest-set-time



On 13.08.2014 18:30, Eric Blake wrote:
On 08/13/2014 06:51 AM, Michal Privoznik wrote:

s/_/-/2 in the subject line

An advice appeared there on the qemu-devel list [1]. When a domain is
suspended and then resumed guest kernel is not aware of this. So we've
introduced virDomainSetTime API that resets the time within guest
using qemu-ga. On the other hand, qemu itself is trying to make RTC
beat faster to catch the difference. But if we don't tell qemu that
guest's time was reset via the other method, both mechanisms are
applied resulting in again wrong guest time. In order to avoid summing
both corrections we need to tell qemu that it should not use the RTC
injection if the guest time is set via guest agent.

1: http://www.mail-archive.com/qemu-devel nongnu org/msg236435.html

Signed-off-by: Michal Privoznik <mprivozn redhat com>
---

+++ b/src/qemu/qemu_driver.c
@@ -16879,6 +16879,16 @@ qemuDomainSetTime(virDomainPtr dom,
      rv = qemuAgentSetTime(priv->agent, seconds, nseconds, rtcSync);
      qemuDomainObjExitAgent(vm);

+    if (!virDomainObjIsActive(vm)) {
+        virReportError(VIR_ERR_OPERATION_INVALID,
+                       "%s", _("domain is not running"));
+        goto endjob;
+    }
+
+    qemuDomainObjEnterMonitor(driver, vm);
+    rv = qemuMonitorRTCResetReinjection(priv->mon);
+    qemuDomainObjExitMonitor(driver, vm);

This forces the command to fail if qemu is too old to have
rtc-reset-reinjection but the agent is new enough to set time.  Should
you make this code conditional on whether qemu supports the QMP command?

I'm not sure. If that's the case, both corrections will apply so guest ends up with incorrect time anyway. And if the API is to guarantee correctly set time, it must fail if such guarantees can't be made IMO.



  }
+
+int
+qemuMonitorJSONRTCResetReinjection(qemuMonitorPtr mon)
+{
+    int ret = -1;
+    virJSONValuePtr cmd;
+    virJSONValuePtr reply = NULL;
+
+    if (!(cmd = qemuMonitorJSONMakeCommand("rtc-reset-reinjection",
+                                           NULL)))
+        return ret;
+
+    ret = qemuMonitorJSONCommand(mon, cmd, &reply);
+
+    if (ret == 0)
+        ret = qemuMonitorJSONCheckError(cmd, reply);
+
+    virJSONValueFree(cmd);
+    virJSONValueFree(reply);
+    return ret;
+}

Is it worth enhancing the testsuite to add coverage for this command and
expected response?

Yeah. I'll post v2.

Michal


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]