On 05/24/2014 05:21 PM, Eric Blake wrote:
> From: Laine Stump <laine@xxxxxxxxx>
>
> Since there isn't a single libc API to get this value, this patch
> supplies one which gets the value by grabbing current time, then
> converting that into a struct tm with gmtime_r(), then back to a
> time_t using mktime.
>
> The returned value is the difference between UTC and localtime in
> seconds. If localtime is ahead of UTC (east) the offset will be a
> positive number, and if localtime is behind UTC (west) the offset will
> be negative.
>
> This function should be POSIX-compliant, and is threadsafe, but not
> async signal safe. If it was ever necessary to know this value in a
> child process, we could cache it with a one-time init function when
> libvirtd starts, then just supply the cached value, but that
> complexity isn't needed for current usage; that would also have the
> problem that it might not be accurate after a local daylight savings
> boundary.
>
> (If it weren't for DST, we could simply replace this entire function
> with "-timezone"; timezone contains the offset of the current timezone
> (negated from what we want) but doesn't account for DST. And in spite
> of being guaranteed by POSIX, it isn't available on older versions of
> mingw.)
>
> Signed-off-by: Eric Blake <eblake@xxxxxxxxxx>
> ---
>
> After some more playing around, I learned we don't need localtime_r
> at all: time() and mktime() both honor the current timezone, so the
> original and resulting value were always the same. I also figured
> out how to force a timezone with a daylight savings different than
> an hour; the code still works without having to hardcode any guess
> based on tm_isdst.
Actually this patch doesn't work as well as an initial test might
indicate. When I first reviewed it on Saturday afternoon (UTC+3), it
passed the tests, but when I went back and ran it again in the morning
on Monday, it failed the tests that had DST set.
In the interest of getting as much testing time as possible on the code
(and with DV's agreement), I disabled the failing tests and pushed the
patches, then began investigating in more detail.
I ran some custom-written tests based on this patch on a Fedora guest
while modifying the system date and found that at different times of the
day the tests that involved DST failed, yet at some times they failed
(the failure times of different tests were different, and as the clock
ticked forward, TZs with larger offsets would begin to succeed).
I spent a significant amount of time experimenting and found that the
function as written results in a *lot* of failures over the course of a
year (although there are certain times of certain days when it is
successful for some or all of the tests).
So I tried setting gmtimeinfo.tm_isdst = -1 prior to calling mktime and
Yay! It seemed to succeed for all tests on all dates within a year. Then
I added a few more tests, and changed my test script to change the
system to a different timezone prior to calling the test - after seeing
a pattern during a couple tests, to save time I changed the test to only
run through the clock on Jan 1, May 31, and Dec 31, but to do that both
with the shell environment set to
TZ="MST07:00MDT06:00,0/00:00:00,366/23:59:59"
and to
TZ="EET-02:00EEDT-03:00,0/00:00:00,366/23:59:59".
(in recognition of my timezone and Eric's timezone :-)
I've found that in the both cases only three of the 4 new tests I'd
added will fail - these two new tests:
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);
will fail for:
Dec 31 18:00-18:59 for MST
Jan 1 from 03:00 - 03:59 for EET
and this test:
TEST_LOCALOFFSET("VIR02:45VID00:45,0/00:00:00,366/23:59:59", -45 * 60);
fails for
Dec 31 18:00-19:59 for MST
Jan 1 from 03:00 - 04:59 for EET
(Note that the output of the date command *always* showed "MDT" or
"EEDT", indicating that the trick of forcing DST for every time of every
day was working properly, at least for the time in the shell).
The failure in all cases is that the offset returned is the offset that
would be correct if DST hadn't been in effect. Since the method of
setting permanent DST appears to work correctly, and it's happening for
an entire 1-2 hour stretch, I'm guessing that the problem isn't that
time() is called while DST is off and then mktime called while it is on,
but something more insidious.
My conclusions from all this are:
1) *definitely* we need to set gmtimeinfo.tm_isdst = -1 in
virTimeLocalOffsetFromUTC(),
2) I guess we can add all the net test cases I've added that contain
DST, but we should comment out those that fail on Jan 1 / Dec 31.
I'll send a patch in about an hour to do that.
(P.S. if you're interested in the ugly shell script I'm using to flip
through the dates, let me know :-)
>
> src/libvirt_private.syms | 1 +
> src/util/virtime.c | 45 +++++++++++++++++++++++++++++++++++++++-
> src/util/virtime.h | 5 +++--
> tests/virtimetest.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 101 insertions(+), 3 deletions(-)
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 25dab2d..91f13a4 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1984,6 +1984,7 @@ virTimeFieldsNow;
> virTimeFieldsNowRaw;
> virTimeFieldsThen;
> virTimeFieldsThenRaw;
> +virTimeLocalOffsetFromUTC;
> virTimeMillisNow;
> virTimeMillisNowRaw;
> virTimeStringNow;
> diff --git a/src/util/virtime.c b/src/util/virtime.c
> index caa4e24..3a56400 100644
> --- a/src/util/virtime.c
> +++ b/src/util/virtime.c
> @@ -1,7 +1,7 @@
> /*
> * virtime.c: Time handling functions
> *
> - * Copyright (C) 2006-2012 Red Hat, Inc.
> + * Copyright (C) 2006-2014 Red Hat, Inc.
> *
> * This library is free software; you can redistribute it and/or
> * modify it under the terms of the GNU Lesser General Public
> @@ -344,3 +344,46 @@ char *virTimeStringThen(unsigned long long when)
>
> return ret;
> }
> +
> +/**
> + * virTimeLocalOffsetFromUTC:
> + *
> + * This function is threadsafe, but is *not* async signal safe (due to
> + * localtime_r()).
> + *
> + * @offset: pointer to time_t that will be set to the difference
> + * between localtime and UTC in seconds (east of UTC is a
> + * positive number, and west of UTC is a negative number.
> + *
> + * Returns 0 on success, -1 on error with error reported
> + */
> +int
> +virTimeLocalOffsetFromUTC(long *offset)
> +{
> + struct tm gmtimeinfo;
> + time_t current, utc;
> +
> + /* time() gives seconds since Epoch in current timezone */
> + if ((current = time(NULL)) == (time_t)-1) {
> + virReportSystemError(errno, "%s",
> + _("failed to get current system time"));
> + return -1;
> + }
> +
> + /* treat current as if it were in UTC */
> + if (!gmtime_r(¤t, &gmtimeinfo)) {
> + virReportSystemError(errno, "%s",
> + _("gmtime_r failed"));
> + return -1;
> + }
> +
right here is where I added:
gmtimeinfo.tm_isdst = -1;
1, 0, and leaving it alone all caused more failures.
> + /* mktime() also obeys current timezone rules */
> + if ((utc = mktime(&gmtimeinfo)) == (time_t)-1) {
> + virReportSystemError(errno, "%s",
> + _("mktime failed"));
> + return -1;
> + }
> +
> + *offset = current - utc;
> + return 0;
> +}
> +#define TEST_LOCALOFFSET(tz, off) \
> + do { \
> + testTimeLocalOffsetData data = { \
> + .zone = tz, \
> + .offset = off, \
> + }; \
> + if (virtTestRun("Test localtime offset for " #tz, \
> + testTimeLocalOffset, &data) < 0) \
> + ret = -1; \
> + } while (0)
> +
> + TEST_LOCALOFFSET("VIR00:30", -30 * 60);
> + TEST_LOCALOFFSET("VIR01:30", -90 * 60);
> + TEST_LOCALOFFSET("UTC", 0);
> + TEST_LOCALOFFSET("VIR-00:30", 30 * 60);
> + TEST_LOCALOFFSET("VIR-01:30", 90 * 60);
> + /* test DST processing with timezones that always
> + * have DST in effect; what's more, cover a zone with
> + * with an unusual DST different than a usual one hour
> + */
> + TEST_LOCALOFFSET("VIR-00:30VID,0,365", 90 * 60);
> + TEST_LOCALOFFSET("VIR-02:30VID,0,365", 210 * 60);
> + TEST_LOCALOFFSET("VIR-02:30VID-04:30,0,365", 270 * 60);
Here are my new tests:
TEST_LOCALOFFSET("VIR-00:30VID,0/00:00:00,366/23:59:59",((1 * 60) +
30) * 60);
TEST_LOCALOFFSET("VIR-02:30VID,0/00:00:00,366/23:59:59", ((3 *
60) + 30) * 60);
TEST_LOCALOFFSET("VIR-02:30VID-04:30,0/00:00:00,366/23:59:59", ((4 *
60) + 30) * 60);
TEST_LOCALOFFSET("VIR-02:30VID-04:30,0/00:00:00,366/23:59:59", ((4 *
60) + 30) * 60);
*
TEST_LOCALOFFSET("VIR02:45VID00:45,0/00:00:00,366/23:59:59",
-45 * 60);
TEST_LOCALOFFSET("VIR-12:00VID-13:00,0/00:00:00,366/23:59:59",((13 *
60) + 0) * 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);
(those marked with "*" fail on Dec 31 or Jan 1, depending on TZ setting
of the shell that is running virtimetest)
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list