On 05/22/2014 05:07 AM, Laine Stump wrote: > For a clock element as above, libvirt simply converts current system > time with localtime_r(), then starts qemu with a time string that > doesn't contain any timezone information. So, from qemu's point of > view, the -rtc string it gets for: > > <clock offset='variable' basis='utc' adjustment='10800'/> > > is identical to the -rtc string it gets for: > > <clock offset='variable' basis='localtime' adjustment='0'/> > > (assuming the host is in a timezone that is 10800 seconds ahead of > UTC, as is the case on the machine where this message is being > written). > > Since the commandlines are identical, qemu will behave identically > after this point in either case. > So basically, we are arguing that basis='localtime' should be treated as syntactic sugar which we rewrite to a more convenient form at the time we start the guest, and not as a permanent basis that we attempt to migrate. I can live with that. > There are two problems in the case of basis='localtime' though: > > Problem 1) If the guest modifies its RTC, for example to add 20 > seconds, the RTC_CHANGE event from qemu will then contain offset:20 in > both cases. But libvirt will have saved the original adjustment into > adjustment0, and will add that value onto the offset in the > event. This means that in the case of basis=;utc', it will properly > emit an event with offset:10820, but in the case of basis='localtime' > the event will contain offset:20, which is *not* the new offset of the > RTC from UTC (as the event it documented to provide). > > Problem 2) If the guest is migrated to another host that is in a > different timezone, or if it is migrated or saved/restored after the > DST status has changed from what it was when the guest was originally > started, the newly restarted guest will have a different RTC (since it > will be based on the new localtime, which could have shifted by > several hours). > > The solution to both of these problems is simple - rather than > maintaining the original adjustment value along with > "basis='localtime'" in the domain status, when the domain is started > we convert the adjustment offset to one relative to UTC, and set the > status to "basis='utc'". Thus, whatever the RTC offset was from UTC > when it was initially started, that offset will be maintained when > migrating across timezones and DST settings, and the RTC_CHANGE events > will automatically contain the proper offset (which should by > definition always be relative to UTC). Okay, so this patch is cementing some of the arguments I made in response to 3/4 (before I had read this patch). > > This fixes a problem that was implied but not openly stated in: > > https://bugzilla.redhat.com/show_bug.cgi?id=964177 > --- > New in V2 > > src/qemu/qemu_command.c | 33 ++++++++++++++++++++++----------- > 1 file changed, 22 insertions(+), 11 deletions(-) > > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c > index 4998bca..28885f2 100644 > --- a/src/qemu/qemu_command.c > +++ b/src/qemu/qemu_command.c > @@ -5976,19 +5976,30 @@ qemuBuildClockArgStr(virDomainClockDefPtr def) > time_t now = time(NULL); > struct tm nowbits; > > - switch ((enum virDomainClockBasis) def->data.variable.basis) { > - case VIR_DOMAIN_CLOCK_BASIS_UTC: > - now += def->data.variable.adjustment; > - gmtime_r(&now, &nowbits); > - break; > - case VIR_DOMAIN_CLOCK_BASIS_LOCALTIME: > - now += def->data.variable.adjustment; > - localtime_r(&now, &nowbits); > - break; > - case VIR_DOMAIN_CLOCK_BASIS_LAST: > - break; > + if (def->data.variable.basis == VIR_DOMAIN_CLOCK_BASIS_LOCALTIME) { > + time_t localOffset; > + > + /* in the case of basis='localtime', rather than trying to > + * keep that basis (and associated offset from UTC) in the > + * status and deal with adding in the difference each time > + * there is an RTC_CHANGE event, it is simpler and less > + * error prone to just convert the adjustment an offset s/an/to an/ > + * from UTC right now (and change the status to > + * "basis='utc' to reflect this). This eliminates > + * potential errors in both RTC_CHANGE events and in > + * migration (in the case that the status of DST, or the > + * timezone of the destination host, changed relative to > + * startup). including migration to file, across a single host that changes localtime RTC across daylight savings between when the file was saved and when it gets loaded :) > + */ > + if (virTimeLocalOffsetFromUTC(&localOffset) < 0) > + goto error; > + def->data.variable.adjustment += localOffset; > + def->data.variable.basis = VIR_DOMAIN_CLOCK_BASIS_UTC; > } > > + now += def->data.variable.adjustment; > + gmtime_r(&now, &nowbits); Seems to make sense. But I'd like feedback from qemu on 3/4 before giving outright ack to this patch. > + > /* when an RTC_CHANGE event is received from qemu, we need to > * have the adjustment used at domain start time available to > * compute the new offset from UTC. As this new value is > -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list