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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list