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

[Eric Blake]

On 05/23/2014 01:36 PM, Eric Blake wrote:

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

> ACK.
> 

ACK still holds on the grounds of strict improvement, even if we end up
changing things further; although given my naming suggestions below you
may still want to do a v3.  More thoughts on the matter:

We NEED to store adjustment0 in XML: both persistent xml (to control
what the next boot uses) and in the runtime xml (so that when converting
RTC change events from qemu into libvirt events, we have the right
conversion).  But if you look, we ALREADY store adjustment0 in the xml:

   <clock offset='variable' adjustment='10962' basis='utc'/>

The adjustment attribute IS adjustment0, when basis is utc (and patch
4/4 makes it so that any other basis is immediately converted to utc at
the first time we start qemu).

Meanwhile, I argued that 'adjustment' is a live-only attribute.  You
could argue that if qemu gave us a way to learn the current adjustment
via a query command, rather than having to listen for the most recent
RTC change event, we would not even need to store 'adjustment' in the
live XML (the only two times we would use 'adjustment' are when
converting an RTC event [if we got the event, we don't need anything
from storage; if we missed the event, no one will ever know] and when
writing the final state of adjustment into the persistent adjustment0
when the live domain shuts down [but we'd use the new qemu query command
for that]).  On the other hand, if qemu doesn't have the new query
command, then storing 'adjustment' in the live xml proves to be a nice
fallback so that guest shutdown still lets us write the correct value to
the persistent xml in all cases where we didn't miss an event.
Furthermore, I think it should be user-visible rather than hidden, so
that VDSM can learn the current adjustment in use by the guest (whether
or not we also solve the problem of letting VDSM do post-mortem queries
after a transient domain shuts down).  So I'm wondering if:

    <clock offset='variable' adjustment='109062' delta='3600' basis='utc'/>

makes sense for live xml - that is, the XML 'adjustment' attribute
matches what you called 'adjustment0' and appears in both live and
persistent, and the XML 'delta' attribute matches what you called
'adjustment' and which exists only for a running domain (the above
domain was started with a command-line offset of 109062, and the current
RTC offset in the guest is 3600 whether it was done via one or a series
of RTC change events; the next time VDSM creates a new transient guest
for the same resources, it should do so with adjustment='112662').

-- 
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/1df012af/attachment-0001.sig>