[libvirt] [PATCH v2] qemu: Report the offset from host UTC for RTC_CHANGE event

[Eric Blake]

On 06/05/2013 04:32 AM, Osier Yang wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=964177
> 
> Though both libvirt and QEMU's document say RTC_CHANGE returns
> the offset from the host UTC, qemu actually returns the offset
> from the specified date instead when specific date is provided
> (-rtc base=$date).
> 
> It's not safe for qemu to fix it in code, it worked like that
> for 3 years, changing it now may break other QEMU use cases.
> What qemu tries to do is to fix the document:
> 
> http://lists.gnu.org/archive/html/qemu-devel/2013-05/msg04782.html
> 
> And in libvirt side, instead of reply on the qemu, this convert

Unclear grammar.  Not sure if you meant:

instead of replaying the value from qemu

or:

instead of relying on the value from qemu

but either alternative makes more sense.

s/convert/converts/

> the offset returned from qemu to the offset from host UTC, by:
> 
>   /*
>    * a: the offset from qemu RTC_CHANGE event
>    * b: The specified date (-rtc base=$date)
>    * c: the host date when libvirt gets the RTC_CHANGE event
>    * offset: What libvirt will report
>    */
> 
>   offset = a + (b - c);
> 
> The specified date (-rtc base=$date) is recorded in clock's def as
> an internal only member (may be useful to exposed outside?).
> 
> Internal only XML tag "starttime" is introduced to not lose the
> domain process's starttime after libvirt restarting/reloading:
> 
> <clock offset='variable' adjustment='304' basis='utc' starttime='1370423588'/>
> ---
>  src/conf/domain_conf.c  | 27 +++++++++++++++++++++++----
>  src/conf/domain_conf.h  |  3 +++
>  src/qemu/qemu_command.c |  3 +++
>  src/qemu/qemu_process.c | 13 +++++++++++++
>  4 files changed, 42 insertions(+), 4 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index a16ebd1..7773abf 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -96,6 +96,7 @@ typedef enum {
>     VIR_DOMAIN_XML_INTERNAL_PCI_ORIG_STATES = (1<<18),
>     VIR_DOMAIN_XML_INTERNAL_ALLOW_ROM = (1<<19),
>     VIR_DOMAIN_XML_INTERNAL_ALLOW_BOOT = (1<<20),
> +   VIR_DOMAIN_XML_INTERNAL_STARTTIME = (1 << 21)

Please add the trailing comma, so that the next edit to this code can
also be a one-liner.

> +++ b/src/conf/domain_conf.h
> @@ -1767,6 +1767,9 @@ struct _virDomainClockDef {
>          struct {
>              long long adjustment;
>              int basis;
> +
> +            /* Store the start time of guest process, internal only */
> +            unsigned long long starttime;

Might be worth mentioning how to interpret the value - is it seconds
since Epoch?

> +++ b/src/qemu/qemu_process.c
> @@ -796,6 +796,19 @@ qemuProcessHandleRTCChange(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
>      virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
>  
>      virObjectLock(vm);
> +
> +    /* QEMU's RTC_CHANGE event returns the offset from the specified
> +     * date instead of the host UTC if a specific date is provided
> +     * (-rtc base=$date). We need to convert it to be offset from
> +     * host UTC.
> +     */
> +    if (vm->def->clock.offset == VIR_DOMAIN_CLOCK_OFFSET_VARIABLE) {
> +        time_t now = time(NULL);
> +
> +        offset += vm->def->clock.data.variable.starttime -
> +                  (unsigned long long)now;
> +    }
> +

Looks correct.  ACK with the nits fixed up.

-- 
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: 621 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20130605/891c5139/attachment-0001.sig>