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

[Eric Blake]

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 604 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140527/7c706644/attachment-0001.sig>