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

[Eric Blake]

On 06/04/2013 05:49 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 privided

s/privided/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 covert

s/covert/convert/

> 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?).
> ---
>  src/conf/domain_conf.h  |  3 +++
>  src/qemu/qemu_command.c |  3 +++
>  src/qemu/qemu_process.c | 12 ++++++++++++
>  3 files changed, 18 insertions(+)

Incomplete.  You need to track the start time across libvirtd restarts
(in internal XML) for this to reliably work for an event received after
a restart; you also have to cope with a libvirtd restart not finding the
field in internal XML (because the libvirtd restart was due to upgrading
libvirt in the meantime).

> 
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 3a71d6c..3947a56 100644
> --- a/src/conf/domain_conf.h
> +++ 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, internaly only */

Spelling; either 'internal' or 'internally'

> +            time_t starttime;
>          } variable;
>  
>          /* Timezone name, when
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index c4a162a..9254525 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -5518,6 +5518,9 @@ qemuBuildClockArgStr(virDomainClockDefPtr def)
>          now += def->data.variable.adjustment;
>          gmtime_r(&now, &nowbits);
>  
> +        /* Store the starttime of qemu process */
> +        def->data.variable.starttime = now;

Is there anything we can read out of /proc/nnn for the qemu process that
would give us a more accurate start time?  In fact, why not use
virProcessGetStartTime()?  And if virProcessGetStartTime is reliable
across libvirtd restarts, then you might not need to store a time_t
starttime in _virDomainClockDef.

-- 
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/20130604/64facfbd/attachment-0001.sig>