From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37791) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XJmiz-0005mj-N6 for qemu-devel@nongnu.org; Tue, 19 Aug 2014 12:57:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XJmiu-0006Op-Rh for qemu-devel@nongnu.org; Tue, 19 Aug 2014 12:57:33 -0400 Received: from mx1.redhat.com ([209.132.183.28]:45625) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XJmiu-0006O4-KT for qemu-devel@nongnu.org; Tue, 19 Aug 2014 12:57:28 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s7JGvRis011682 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Tue, 19 Aug 2014 12:57:27 -0400 Date: Tue, 19 Aug 2014 13:57:13 -0300 From: Marcelo Tosatti Message-ID: <20140819165713.GA4488@amt.cnet> References: <075846faf30f2244fbaaf2c9f47cbae58a9524db.1408004697.git.mprivozn@redhat.com> <53F21B82.4050006@redhat.com> <53F229F6.6090602@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <53F229F6.6090602@redhat.com> Subject: Re: [Qemu-devel] [libvirt] [PATCHv2 libvirt] qemu: Issue rtc-reset-reinjection command after guest-set-time List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Michal Privoznik Cc: libvir-list@redhat.com, qemu-devel On Mon, Aug 18, 2014 at 06:29:42PM +0200, Michal Privoznik wrote: > On 18.08.2014 17:28, Eric Blake wrote: > >On 08/14/2014 02:24 AM, Michal Privoznik wrote: > >>https://bugzilla.redhat.com/show_bug.cgi?id=1103245 > >> > >>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 > >>--- > >> > >>Notes: > >> diff to v1: > >> -fixed command name in subject > >> -added testcase > >> > > > >>+++ 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); > > > >We have four combinations: > > > >1. old qemu, old qga: command fails because qga doesn't support it, qemu > >tries to catch up time manually (might eventually match real time) > > > >2. new qemu, old qga: command fails because qga doesn't support it, qemu > >tries to catch up time manually (might eventually match real time) > > > >3. new qemu, new qga: both qga and qemu commands work, no additional > >catchup attempted and guest is now accurate > > > >4. old qemu, new qga: qga succeeds, but qemu command fails, so we have > >overcorrected and qemu is trying to catch up time manually > >(overcorrected, so it cannot match real time) > > > >I guess reporting failure in those three cases is fine, although I'm > >still worried about case 4. I'd feel a lot better if there were a > >qemu_capabilities.h bit that detects if the qemu command is present, and > >skip even attempting the qga command unless we ALSO know the qemu > >command is present (that is, use the capability check to completely > >avoid case 4, by turning it into the same behavior as case 1). > > Okay. Although I've just realized one (corner) case. From my > understanding of rtc-reset-reinjection time it's only necessary if > guest was suspended for a while and the guest's RTC clock skewed. > But what if I start fresh new guest and just want to set its time > (leave aside the reasoning why would I do that for a while)? Is the > rtc-reset-reinjection necessary? I wouldn't say. But on the other > hand - libvirt doesn't know if the RTC is synced already or not. > Hence it's safer for libvirt to issue the command every single time. > > In fact, there are two ways to set guest time: > > a) {"execute":"guest-set-time"} > > b) {"execute":"guest-set-time, "arguments":{"time":1234567890}} > > While in the case a) guest time is set by reading from guest's RTC, > in case of b) guest time is set by calling settimeofday() and RTC is > written thereafter. > > So is the rtc-reset-reinjection necessary only for case a) and in > case b) QEMU somehow detects RTC write and cancels the reinjection > itself? > > Michal rtc-reset-reinjection has been introduced because certain Windows versions will advance the guest system time (via rtc interrupt reinjection). So if libvirt adjusts the guest system time via guest-set-time, allowing rtc interrupt reinjection to compensate for lost time, as well, will cause an incorrect guest system time. So you should always use the guest-set-time rtc-reset-reinjection pair.