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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list