[libvirt] [PATCH] util: fix virTimeLocalOffsetFromUTC DST processing

[Laine Stump]

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