[libvirt] [PATCH] qemu driver: sync guest time via qemu-guest-agent when domain is resumed

[Marcelo Tosatti]

On Tue, Feb 11, 2014 at 11:48:58AM +0100, Michal Privoznik wrote:
> On 11.02.2014 01:04, Marcelo Tosatti wrote:
> >On Mon, Feb 10, 2014 at 03:42:45PM -0700, Eric Blake wrote:
> >>On 02/10/2014 03:10 PM, Marcelo Tosatti wrote:
> >>>
> >>>Since QEMU commit "kvmclock: clock should count only if vm is running"
> >>>(http://lists.gnu.org/archive/html/qemu-devel/2013-06/msg01225.html),
> >>>guests have their realtime clock stopped during pause.
> >>>
> >>>To correct the situation, invoke guest agent to sync time from
> >>>host time.
> >>
> >>This needs to be conditional on new API (either a flag to existing API
> >>[except that virDomainResumeFlags is not quite existing yet], or new API
> >>altogether).
> >>
> >>We cannot require an API to depend on guest interaction (which an agent
> >>command does) without an explicit flag mentioning that it is okay.  We
> >>may even decide to have conditional ACL checks (allowing some users the
> >>ability to restart a domain, but not to interact with the guest agent).
> >>
> >>For that matter, when we proposed adding virDomainResumeFlags as the way
> >>to add in a flag for requesting a sync via this API, Dan suggested it
> >>would be better to add a new API altogether instead of piggybacking on
> >>top of the existing API:
> >>
> >>https://www.redhat.com/archives/libvir-list/2014-February/msg00104.html
> >
> >"So there's interesting, and I'm not sure I entirely agree about
> >adding flags here. It seems to me that if the QEMU agent has a
> >"set-time" capability we'd be better off having an explicit API
> >virDomainSetTime(...).  The action of resume + set time cannot
> >be atomic so I don't see any point in overloading the "set time"
> >functionality into the resume API call. Just let apps call the
> >set time method if they so desire."
> >
> >Apps desire the guest time to be synchronized to host time, which is the
> >standard 99% of usecases configuration.
> >
> >Ok, i was thinking of adding a new element GuestSyncTimeOnResume to
> >control this, which would default to on (as users expect their guests
> >time to be correct at all times, without additional commands, and
> >rightly so).
> >
> >It seems unreasonable to me that every API user has to keep track of
> >every location which a guest can possibly resume execution from a paused
> >state: this can be hidden inside libvirt.
> >
> >Daniel, all the logic behind the necessity to set-time after guest
> >resume (*) can be hidden inside libvirt.
> >
> >(*) all instances of guest resume: upon migration continuation, upon
> >ENOSPACE pause continuation, all of them.

^^^^ (1).

> >
> >Don't see the need to force the knowledge and maintenance of this state
> >on libvirt users.
> >
> >Are you OK with the new knob, default on, to control sync time on
> >guest resume, then?
> >
> 
> 
> I think we may need both approaches. I too think that resume with
> syncing guest time is so common use case that is deserves to be
> exposed as a single operation outside the libvirt.
> 
> On the other hand, we certainly want to expose this as a new
> virDomain{Get,Set}Time() API.

Why a new API which requires modifications to every libvirt user (read:
current libvirt users are broken until patched) ? 

Note it requires applications to keep track of every pause/resume
instance (see item 1 above).

Let us please back the suggestion to not perform guest-agent time
sync inside libvirt without app knowledge and instead have a separate
operation with facts.

> BTW: I would have implement this already, but I'm still waiting for
> my qemu patch to be merged upstream:
> 
> http://lists.nongnu.org/archive/html/qemu-devel/2014-01/msg04293.html
> 
> I don't really want to start implementing this feature with need to
> patch qemu myself.
> 
> Michal

No problem.