On Tue, Jun 04, 2013 at 09:46:14PM +0800, Osier Yang wrote:
> 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.
Sorry, yes, i meant -rtc, not -clock
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|