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

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

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(+)

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 */

+ 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;

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.

On 04/06/13 21:13, Daniel P. Berrange wrote: >On Tue, Jun 04, 2013 at 06:59:02AM -0600, Eric Blake wrote: >>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. >It isn't the start time of the QEMU process that we care about >here. The offset is relative to the timestamp specified via the >-clock command line arg. So using QEMU procss startup would be >wrong. > hm, I don't see libvirt uses "-clock" option. And the offset is from the specified of "-rtc base=$date" with my testing.