On 05/27/2014 05:30 PM, Eric Blake wrote:
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.
except that "366" is Dec 31 + 1 (Dec 32? or Jan 1 of the next year?) on
a leap year, or Dec 31 + 2 on a non-leap year..
The odd part is that if you set such a TZ, then do "date --set ..." for
all dates/times around Dec 31 - Jan 1, the output of the new date will
*always* show that DST is set (e.g. "VID")
(Oh, and also that the failure seems to be from midnight to 2AM *UTC*
regardless of the offset of the TZ (even though the DST changes are
supposed to happen at 00:00:00 of localtime)).
> ---
>
> 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?
Yes, after a lot of trials (both with this code and other separate test
programs and shell scripts) and thought, I believe that is the case.
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?
I like that idea!
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.
Okay, I will replace the #ifdef with the check you mention above, and
send an inter-diff for ACK before pushing.
Thanks for all the time and thought you've put into this. It has been a
real eye opening experience :-P