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

On 06/05/2013 04:32 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 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 convert Unclear grammar. Not sure if you meant: instead of replaying the value from qemu or: instead of relying on the value from qemu

but either alternative makes more sense. s/convert/converts/ > 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?). > > Internal only XML tag "starttime" is introduced to not lose the > domain process's starttime after libvirt restarting/reloading: > > <clock offset='variable' adjustment='304' basis='utc' starttime='1370423588'/> > --- > src/conf/domain_conf.c | 27 +++++++++++++++++++++++---- > src/conf/domain_conf.h | 3 +++ > src/qemu/qemu_command.c | 3 +++ > src/qemu/qemu_process.c | 13 +++++++++++++ > 4 files changed, 42 insertions(+), 4 deletions(-) > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index a16ebd1..7773abf 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -96,6 +96,7 @@ typedef enum { > VIR_DOMAIN_XML_INTERNAL_PCI_ORIG_STATES = (1<<18), > VIR_DOMAIN_XML_INTERNAL_ALLOW_ROM = (1<<19), > VIR_DOMAIN_XML_INTERNAL_ALLOW_BOOT = (1<<20), > + VIR_DOMAIN_XML_INTERNAL_STARTTIME = (1 << 21) Please add the trailing comma, so that the next edit to this code can also be a one-liner. > +++ 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, internal only */ > + unsigned long long starttime; Might be worth mentioning how to interpret the value - is it seconds since Epoch?