Re: [libvirt] [PATCH] util: fix virTimeLocalOffsetFromUTC DST processing
On 05/27/2014 08:11 AM, Laine Stump wrote:
The original version of virTimeLocalOffsetFromUTC() (commit
1cddaea7aeca441b733c31990a3f139dd2d346f6) would fail for certain times
of the day if daylight savings time was active. This could most easily
be seen by uncommenting the TEST_LOCALOFFSET() cases that invlude a
s/invlude/include/
DST setting.
After a lot of experimenting, I found that the way to solve it in
almost all test cases is to set tm_isdst = -1 in the stuct tm prior to
calling mktime(). Once this is done, the correct offset is returned
for all test cases at all times except the two hours just after
00:00:00 Jan 1 UTC - during that time, any timezone that is *behind*
UTC, and that is supposed to always be in DST will not have DST
accounted for in its offset. I still do not know the source of this
problem, but it appears to be either a bug in glibc, or my improper
specification of a TZ setting, so I am leaving the offending tests
listed in virtimetest.c, but disabling them for now.
Per POSIX, the default time for swapping in or out of DST is at 2 am
unless $TZ says otherwise. So in your testing,
VIR05:00VID04:00,0/00:00:00,366/23:59:59
has only a 1 minute window where DST was not active, while
VIR05:00VID04:00,0,366
has a 24 hour window that overlaps two days.
---
See https://www.redhat.com/archives/libvir-list/2014-May/msg00898.html
for my earlier comments on this problem.
src/util/virtime.c | 3 +++
tests/virtimetest.c | 24 +++++++++++++++++-------
2 files changed, 20 insertions(+), 7 deletions(-)
+#ifdef __BROKEN_DST_TESTS
+ TEST_LOCALOFFSET("VIR02:45VID00:45,0/00:00:00,366/23:59:59",
+ -45 * 60);
+ TEST_LOCALOFFSET("VIR05:00VID04:00,0/00:00:00,366/23:59:59",
+ ((-4 * 60) + 0) * 60);
+ TEST_LOCALOFFSET("VIR11:00VID10:00,0/00:00:00,366/23:59:59",
+ ((-10 * 60) + 0) * 60);
#endif
So is the point that these tests are only broken as a year rolls over,
while the code itself is robust, and all because we can't _quite_ force
our fake timezone to have daylight savings where we want it? Might it
be worth doing a simple filter (if time()/localtime_r() says today is
Jan 1 or Dec 31, skip these tests, otherwise run them), so that we are
at least testing the code 363 days a year?
At any rate, the change to virtime.c is essential (you convinced me of
that in the other thread), and this counts as a bug fix; it also passes
the testsuite. So regardless of whether you keep the #ifdef or convert
it to a time-dependent filter, ACK to getting this in before the release.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 604 bytes)