[libvirt] [PATCHv2 3/4] qemu: fix RTC_CHANGE event for <clock offset='variable' basis='utc'/>

[Eric Blake]

On 05/22/2014 05:07 AM, Laine Stump wrote:

[I've re-read this thread, trying to determine whether this patch is
good enough to apply as-is]

> commit e31b5cf393857 attempted to fix libvirt's
> VIR_DOMAIN_EVENT_ID_RTC_CHANGE, which is documentated to always
> provide the new offset of the domain's real time clock from UTC. The
> problem was that, in the case that qemu is provided with an "-rtc
> base=x" where x is an absolute time (rather than "utc" or
> "localtime"), the offset sent by qemu's RTC_CHANGE event is *not* the
> new offset from UTC, but rather is the sum of all changes to the
> domain's RTC since it was started with base=x.
> 
> So, despite what was said in commit e31b5cf393857, if we assume that
> the original value stored in "adjustment" was the offset from UTC at
> the time the domain was started, we can always determine the current
> offset from UTC by simply adding the most recent (i.e. current) offset
> from qemu to that original adjustment.

I agree that 'adjustment' is a runtime-only value - it only exists as
long as a qemu guest is running; I also agree that the offset from UTC
requires knowing both 'adjustment0' (the command-line offset) and
'adjustment' (the current qemu state, whether learned by RTC Change
event, or by a new [yet-to-be-written] QMP query command).

> 
> This patch accomplishes that by storing the initial adjustment in the
> domain's status as "adjustment0". Each time a new RTC_CHANGE event is
> received from qemu, we simply add adjustment0 to the value sent by
> qemu, store that as the new adjustment, and forward that value on to
> any event handler.

Okay, I'm convinced that this guarantees that the right value is exposed
in the event handler, so this patch is a strict improvement.

I still think we have one (or more) libvirt bugs (basically,
'adjustment0' is the persistent domain's definition of what the next
boot of the guest will start with, and unless the user manually changes
that value, I think libvirt should automatically be adjusting the
persistent 'adjustment0' to the final value of the run-time when the
guest shuts down; I also think VDSM needs a way to query the UTC offset
that was last used by a transient domain even after the domain has shut
down) - but those can (and should) be separate patches.

> 
> This patch (*not* e31b5cf393857, which should be reverted prior to
> applying this patch) fixes:
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=964177
> 
> (for the case where basis='utc'. It does not fix basis='localtime')
> ---
> 
> Changes from V1: remove all attempts to fix basis='localtime' in favor
> of fixing it in a simpler and better manner in a separate patch.
> 
>  src/conf/domain_conf.c  | 21 +++++++++++++++++----
>  src/conf/domain_conf.h  |  7 +++++++
>  src/qemu/qemu_command.c |  9 +++++++++
>  src/qemu/qemu_process.c | 27 ++++++++++++++++++++++-----
>  4 files changed, 55 insertions(+), 9 deletions(-)
> 

In addition to the commit typo found earlier,


> +            /* domain start-time adjustment. This is a
> +             * private/internal read-only value that only exists when
> +             * a domain is running, and only if the clock
> +             * offset='variable'

s/clock/clock uses/

> +++ b/src/qemu/qemu_process.c
> @@ -831,7 +831,6 @@ qemuProcessHandleResume(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
>      return 0;
>  }
>  
> -
>  static int

Spurious line deletion.

ACK.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 604 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140523/20f30ef0/attachment-0001.sig>