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

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. > >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.