[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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.

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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]