qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Marcelo Tosatti <mtosatti@redhat.com>
To: Michal Privoznik <mprivozn@redhat.com>
Cc: libvir-list@redhat.com, qemu-devel <qemu-devel@nongnu.org>
Subject: Re: [Qemu-devel] [libvirt] [PATCHv2 libvirt] qemu: Issue rtc-reset-reinjection command after guest-set-time
Date: Tue, 19 Aug 2014 13:57:13 -0300	[thread overview]
Message-ID: <20140819165713.GA4488@amt.cnet> (raw)
In-Reply-To: <53F229F6.6090602@redhat.com>

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 <mprivozn@redhat.com>
> >>---
> >>
> >>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.

  reply	other threads:[~2014-08-19 16:57 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <075846faf30f2244fbaaf2c9f47cbae58a9524db.1408004697.git.mprivozn@redhat.com>
     [not found] ` <53F21B82.4050006@redhat.com>
2014-08-18 16:29   ` [Qemu-devel] [libvirt] [PATCHv2 libvirt] qemu: Issue rtc-reset-reinjection command after guest-set-time Michal Privoznik
2014-08-19 16:57     ` Marcelo Tosatti [this message]
2014-08-19 17:00       ` Eric Blake
2014-08-19 17:23         ` Marcelo Tosatti
2014-08-20  8:11           ` Michal Privoznik

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140819165713.GA4488@amt.cnet \
    --to=mtosatti@redhat.com \
    --cc=libvir-list@redhat.com \
    --cc=mprivozn@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).