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

On 05/23/2014 01:36 PM, Eric Blake wrote: 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