src: replace gmtime_r/localtime_r/strftime with GDateTime

gmtime_r/localtime_r are mostly used in combination with
strftime to format timestamps in libvirt. This can all
be replaced with GDateTime resulting in simpler code
that is also more portable.

There is some boundary condition problem in parsing POSIX
timezone offsets in GLib which tickles our test suite.
The test suite is hacked to avoid the problem. The upsteam
GLib bug report is

  https://gitlab.gnome.org/GNOME/glib/issues/1999

Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

src/conf/domain_conf.c

@@ -26790,11 +26790,12 @@ virDomainGraphicsAuthDefFormatAttr(virBufferPtr buf,
                               def->passwd);
 
     if (def->expires) {
-        char strbuf[100];
+        g_autoptr(GDateTime) then = NULL;
-        struct tm tmbuf, *tm;
+        g_autofree char *thenstr = NULL;
-        tm = gmtime_r(&def->validTo, &tmbuf);
+
-        strftime(strbuf, sizeof(strbuf), "%Y-%m-%dT%H:%M:%S", tm);
+        then = g_date_time_new_from_unix_utc(def->validTo);
-        virBufferAsprintf(buf, " passwdValidTo='%s'", strbuf);
+        thenstr = g_date_time_format(then, "%Y-%m-%dT%H:%M:%S");
+        virBufferAsprintf(buf, " passwdValidTo='%s'", thenstr);
     }
 
     if (def->connected)

src/libxl/libxl_domain.c

@@ -943,16 +943,14 @@ libxlDomainAutoCoreDump(libxlDriverPrivatePtr driver,
                         virDomainObjPtr vm)
 {
     g_autoptr(libxlDriverConfig) cfg = libxlDriverConfigGet(driver);
-    time_t curtime = time(NULL);
+    g_autoptr(GDateTime) now = g_date_time_new_now_local();
-    char timestr[100];
+    g_autofree char *nowstr = NULL;
-    struct tm time_info;
     char *dumpfile = NULL;
 
-    localtime_r(&curtime, &time_info);
+    nowstr = g_date_time_format(now, "%Y-%m-%d-%H:%M:%S");
-    strftime(timestr, sizeof(timestr), "%Y-%m-%d-%H:%M:%S", &time_info);
 
     dumpfile = g_strdup_printf("%s/%s-%s", cfg->autoDumpDir, vm->def->name,
-                               timestr);
+                               nowstr);
 
     /* Unlock virDomainObj while dumping core */
     virObjectUnlock(vm);

src/qemu/qemu_command.c

@@ -6070,8 +6070,9 @@ qemuBuildClockArgStr(virDomainClockDefPtr def)
         break;
 
     case VIR_DOMAIN_CLOCK_OFFSET_VARIABLE: {
-        time_t now = time(NULL);
+        g_autoptr(GDateTime) now = g_date_time_new_now_utc();
-        struct tm nowbits;
+        g_autoptr(GDateTime) then = NULL;
+        g_autofree char *thenstr = NULL;
 
         if (def->data.variable.basis == VIR_DOMAIN_CLOCK_BASIS_LOCALTIME) {
             long localOffset;
@@ -6094,8 +6095,8 @@ qemuBuildClockArgStr(virDomainClockDefPtr def)
             def->data.variable.basis = VIR_DOMAIN_CLOCK_BASIS_UTC;
         }
 
-        now += def->data.variable.adjustment;
+        then = g_date_time_add_seconds(now, def->data.variable.adjustment);
-        gmtime_r(&now, &nowbits);
+        thenstr = g_date_time_format(then, "%Y-%m-%dT%H:%M:%S");
 
         /* when an RTC_CHANGE event is received from qemu, we need to
          * have the adjustment used at domain start time available to
@@ -6105,13 +6106,7 @@ qemuBuildClockArgStr(virDomainClockDefPtr def)
         */
         def->data.variable.adjustment0 = def->data.variable.adjustment;
 
-        virBufferAsprintf(&buf, "base=%d-%02d-%02dT%02d:%02d:%02d",
+        virBufferAsprintf(&buf, "base=%s", thenstr);
-                          nowbits.tm_year + 1900,
-                          nowbits.tm_mon + 1,
-                          nowbits.tm_mday,
-                          nowbits.tm_hour,
-                          nowbits.tm_min,
-                          nowbits.tm_sec);
     }   break;
 
     default:

src/qemu/qemu_driver.c

@@ -4090,9 +4090,8 @@ getAutoDumpPath(virQEMUDriverPtr driver,
                 virDomainObjPtr vm)
 {
     g_autofree char *domname = virDomainDefGetShortName(vm->def);
-    char timestr[100];
+    g_autoptr(GDateTime) now = g_date_time_new_now_local();
-    struct tm time_info;
+    g_autofree char *nowstr = NULL;
-    time_t curtime = time(NULL);
     g_autoptr(virQEMUDriverConfig) cfg = NULL;
 
     if (!domname)
@@ -4100,10 +4099,9 @@ getAutoDumpPath(virQEMUDriverPtr driver,
 
     cfg = virQEMUDriverGetConfig(driver);
 
-    localtime_r(&curtime, &time_info);
+    nowstr = g_date_time_format(now, "%Y-%m-%d-%H:%M:%S");
-    strftime(timestr, sizeof(timestr), "%Y-%m-%d-%H:%M:%S", &time_info);
 
-    return g_strdup_printf("%s/%s-%s", cfg->autoDumpPath, domname, timestr);
+    return g_strdup_printf("%s/%s-%s", cfg->autoDumpPath, domname, nowstr);
 }
 
 static void

src/util/virtime.c

@@ -302,8 +302,8 @@ char *virTimeStringThen(unsigned long long when)
 /**
  * virTimeLocalOffsetFromUTC:
  *
- * This function is threadsafe, but is *not* async signal safe (due to
+ * This function is threadsafe, but is *not* async signal safe
- * gmtime_r() and mktime()).
+ * due to use of GLib APIs.
  *
  * @offset: pointer to time_t that will be set to the difference
  *          between localtime and UTC in seconds (east of UTC is a
@@ -314,34 +314,11 @@ char *virTimeStringThen(unsigned long long when)
 int
 virTimeLocalOffsetFromUTC(long *offset)
 {
-    struct tm gmtimeinfo;
+    g_autoptr(GDateTime) now = g_date_time_new_now_local();
-    time_t current, utc;
+    GTimeSpan diff = g_date_time_get_utc_offset(now);
 
-    /* time() gives seconds since Epoch in current timezone */
+    /* GTimeSpan measures microseconds, we want seconds */
-    if ((current = time(NULL)) == (time_t)-1) {
+    *offset = diff / 1000000;
-        virReportSystemError(errno, "%s",
-                             _("failed to get current system time"));
-        return -1;
-    }
-
-    /* treat current as if it were in UTC */
-    if (!gmtime_r(&current, &gmtimeinfo)) {
-        virReportSystemError(errno, "%s",
-                             _("gmtime_r failed"));
-        return -1;
-    }
-
-    /* tell mktime to figure out itself whether or not DST is in effect */
-    gmtimeinfo.tm_isdst = -1;
-
-    /* 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;
 }
 

tests/qemuxml2argvmock.c

@@ -47,12 +47,14 @@ long virGetSystemPageSize(void)
     return 4096;
 }
 
-time_t time(time_t *t)
+GDateTime *g_date_time_new_now_utc(void)
 {
-    const time_t ret = 1234567890;
+    return g_date_time_new_from_unix_utc(1234567890);
-    if (t)
+}
-        *t = ret;
+
-    return ret;
+GDateTime *g_date_time_new_now_local(void)
+{
+    return g_date_time_new_from_unix_local(1234567890);
 }
 
 bool

tests/virtimetest.c

@@ -101,20 +101,12 @@ testTimeLocalOffset(const void *args)
 static bool
 isNearYearEnd(void)
 {
-    time_t current = time(NULL);
+    g_autoptr(GDateTime) now = g_date_time_new_now_local();
-    struct tm timeinfo;
 
-    if (current == (time_t)-1) {
+    return ((g_date_time_get_month(now) == 1 &&
-        VIR_DEBUG("time() failed");
+             g_date_time_get_day_of_month(now) == 1) ||
-        return false;
+            (g_date_time_get_month(now) == 12 &&
-    }
+             g_date_time_get_day_of_month(now) == 31));
-    if (!localtime_r(&current, &timeinfo)) {
-        VIR_DEBUG("localtime_r() failed");
-        return false;
-    }
-
-    return (timeinfo.tm_mon == 0 && timeinfo.tm_mday == 1) ||
-            (timeinfo.tm_mon == 11 && timeinfo.tm_mday == 31);
 }
 
 
@@ -186,14 +178,21 @@ mymain(void)
     /* 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
+     *
+     * These tests originally used '0' as the first day,
+     * but changed to '1' due to GLib GTimeZone parsing bug:
+     *  https://gitlab.gnome.org/GNOME/glib/issues/1999
+     *
+     * Once we depend on a new enough GLib, we can put then
+     * back to 0 again.
      */
-    TEST_LOCALOFFSET("VIR-00:30VID,0/00:00:00,365/23:59:59",
+    TEST_LOCALOFFSET("VIR-00:30VID,1/00:00:00,364/23:59:59",
                      ((1 * 60) + 30) * 60);
-    TEST_LOCALOFFSET("VIR-02:30VID,0/00:00:00,365/23:59:59",
+    TEST_LOCALOFFSET("VIR-02:30VID,1/00:00:00,364/23:59:59",
                      ((3 * 60) + 30) * 60);
-    TEST_LOCALOFFSET("VIR-02:30VID-04:30,0/00:00:00,365/23:59:59",
+    TEST_LOCALOFFSET("VIR-02:30VID-04:30,1/00:00:00,364/23:59:59",
                      ((4 * 60) + 30) * 60);
-    TEST_LOCALOFFSET("VIR-12:00VID-13:00,0/00:00:00,365/23:59:59",
+    TEST_LOCALOFFSET("VIR-12:00VID-13:00,1/00:00:00,364/23:59:59",
                      ((13 * 60) +  0) * 60);
 
     if (!isNearYearEnd()) {
@@ -209,11 +208,11 @@ mymain(void)
          * tests, except on Dec 31 and Jan 1.
          */
 
-        TEST_LOCALOFFSET("VIR02:45VID00:45,0/00:00:00,365/23:59:59",
+        TEST_LOCALOFFSET("VIR02:45VID00:45,1/00:00:00,364/23:59:59",
                          -45 * 60);
-        TEST_LOCALOFFSET("VIR05:00VID04:00,0/00:00:00,365/23:59:59",
+        TEST_LOCALOFFSET("VIR05:00VID04:00,1/00:00:00,364/23:59:59",
                          ((-4 * 60) +  0) * 60);
-        TEST_LOCALOFFSET("VIR11:00VID10:00,0/00:00:00,365/23:59:59",
+        TEST_LOCALOFFSET("VIR11:00VID10:00,1/00:00:00,364/23:59:59",
                          ((-10 * 60) +  0) * 60);
     }
 

tools/virsh-checkpoint.c

@@ -719,9 +719,8 @@ cmdCheckpointList(vshControl *ctl,
     char *doc = NULL;
     virDomainCheckpointPtr checkpoint = NULL;
     long long creation_longlong;
-    time_t creation_time_t;
+    g_autoptr(GDateTime) then = NULL;
-    char timestr[100];
+    g_autofree gchar *thenstr = NULL;
-    struct tm time_info;
     bool tree = vshCommandOptBool(cmd, "tree");
     bool name = vshCommandOptBool(cmd, "name");
     bool from = vshCommandOptBool(cmd, "from");
@@ -835,21 +834,16 @@ cmdCheckpointList(vshControl *ctl,
         if (virXPathLongLong("string(/domaincheckpoint/creationTime)", ctxt,
                              &creation_longlong) < 0)
             continue;
-        creation_time_t = creation_longlong;
+
-        if (creation_time_t != creation_longlong) {
+        then = g_date_time_new_from_unix_local(creation_longlong);
-            vshError(ctl, "%s", _("time_t overflow"));
+        thenstr = g_date_time_format(then, "%Y-%m-%d %H:%M:%S %z");
-            continue;
-        }
-        localtime_r(&creation_time_t, &time_info);
-        strftime(timestr, sizeof(timestr), "%Y-%m-%d %H:%M:%S %z",
-                 &time_info);
 
         if (parent) {
-            if (vshTableRowAppend(table, chk_name, timestr,
+            if (vshTableRowAppend(table, chk_name, thenstr,
                                   NULLSTR_EMPTY(parent_chk), NULL) < 0)
                 goto cleanup;
         } else {
-            if (vshTableRowAppend(table, chk_name, timestr, NULL) < 0)
+            if (vshTableRowAppend(table, chk_name, thenstr, NULL) < 0)
                 goto cleanup;
         }
     }

tools/virsh-domain-monitor.c

@@ -1534,17 +1534,13 @@ cmdDomTime(vshControl *ctl, const vshCmd *cmd)
             goto cleanup;
 
         if (pretty) {
-            char timestr[100];
+            g_autoptr(GDateTime) then = NULL;
-            time_t cur_time = seconds;
+            g_autofree char *thenstr = NULL;
-            struct tm time_info;
 
-            if (!gmtime_r(&cur_time, &time_info)) {
+            then = g_date_time_new_from_unix_utc(seconds);
-                vshError(ctl, _("Unable to format time"));
+            thenstr = g_date_time_format(then, "%Y-%m-%d %H:%M:%S");
-                goto cleanup;
-            }
-            strftime(timestr, sizeof(timestr), "%Y-%m-%d %H:%M:%S", &time_info);
 
-            vshPrint(ctl, _("Time: %s"), timestr);
+            vshPrint(ctl, _("Time: %s"), thenstr);
         } else {
             vshPrint(ctl, _("Time: %lld"), seconds);
         }

tools/virsh-domain.c

@@ -5492,9 +5492,8 @@ static const vshCmdOptDef opts_screenshot[] = {
 static char *
 virshGenFileName(vshControl *ctl, virDomainPtr dom, const char *mime)
 {
-    char timestr[100];
+    g_autoptr(GDateTime) now = g_date_time_new_now_local();
-    time_t cur_time;
+    g_autofree char *nowstr = NULL;
-    struct tm time_info;
     const char *ext = NULL;
     char *ret = NULL;
 
@@ -5509,12 +5508,10 @@ virshGenFileName(vshControl *ctl, virDomainPtr dom, const char *mime)
         ext = ".png";
     /* add mime type here */
 
-    time(&cur_time);
+    nowstr = g_date_time_format(now, "%Y-%m-%d-%H:%M:%S");
-    localtime_r(&cur_time, &time_info);
-    strftime(timestr, sizeof(timestr), "%Y-%m-%d-%H:%M:%S", &time_info);
 
-    ret = g_strdup_printf("%s-%s%s", virDomainGetName(dom), timestr,
+    ret = g_strdup_printf("%s-%s%s", virDomainGetName(dom),
-                          NULLSTR_EMPTY(ext));
+                          nowstr, NULLSTR_EMPTY(ext));
 
     return ret;
 }

tools/virsh-network.c

@@ -1433,11 +1433,10 @@ cmdNetworkDHCPLeases(vshControl *ctl, const vshCmd *cmd)
         const char *typestr = NULL;
         g_autofree char *cidr_format = NULL;
         virNetworkDHCPLeasePtr lease = leases[i];
-        time_t expirytime_tmp = lease->expirytime;
+        g_autoptr(GDateTime) then = g_date_time_new_from_unix_local(lease->expirytime);
-        struct tm ts;
+        g_autofree char *thenstr = NULL;
-        char expirytime[32];
+
-        localtime_r(&expirytime_tmp, &ts);
+        thenstr = g_date_time_format(then, "%Y-%m-%d %H:%M:%S");
-        strftime(expirytime, sizeof(expirytime), "%Y-%m-%d %H:%M:%S", &ts);
 
         if (lease->type == VIR_IP_ADDR_TYPE_IPV4)
             typestr = "ipv4";
@@ -1447,7 +1446,7 @@ cmdNetworkDHCPLeases(vshControl *ctl, const vshCmd *cmd)
         cidr_format = g_strdup_printf("%s/%d", lease->ipaddr, lease->prefix);
 
         if (vshTableRowAppend(table,
-                              expirytime,
+                              thenstr,
                               NULLSTR_MINUS(lease->mac),
                               NULLSTR_MINUS(typestr),
                               NULLSTR_MINUS(cidr_format),

tools/virsh-snapshot.c

@@ -1492,9 +1492,8 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
     virDomainSnapshotPtr snapshot = NULL;
     char *state = NULL;
     long long creation_longlong;
-    time_t creation_time_t;
+    g_autoptr(GDateTime) then = NULL;
-    char timestr[100];
+    g_autofree gchar *thenstr = NULL;
-    struct tm time_info;
     bool tree = vshCommandOptBool(cmd, "tree");
     bool name = vshCommandOptBool(cmd, "name");
     bool from = vshCommandOptBool(cmd, "from");
@@ -1624,22 +1623,16 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
         if (virXPathLongLong("string(/domainsnapshot/creationTime)", ctxt,
                              &creation_longlong) < 0)
             continue;
-        creation_time_t = creation_longlong;
+        then = g_date_time_new_from_unix_local(creation_longlong);
-        if (creation_time_t != creation_longlong) {
+        thenstr = g_date_time_format(then, "%Y-%m-%d %H:%M:%S %z");
-            vshError(ctl, "%s", _("time_t overflow"));
-            continue;
-        }
-        localtime_r(&creation_time_t, &time_info);
-        strftime(timestr, sizeof(timestr), "%Y-%m-%d %H:%M:%S %z",
-                 &time_info);
 
         if (parent) {
-            if (vshTableRowAppend(table, snap_name, timestr, state,
+            if (vshTableRowAppend(table, snap_name, thenstr, state,
                                   NULLSTR_EMPTY(parent_snap),
                                   NULL) < 0)
                 goto cleanup;
         } else {
-            if (vshTableRowAppend(table, snap_name, timestr, state,
+            if (vshTableRowAppend(table, snap_name, thenstr, state,
                                   NULL) < 0)
                 goto cleanup;
         }

tools/virt-admin.c

@@ -69,40 +69,6 @@ vshAdmClientTransportToString(int transport)
     return str ? _(str) : _("unknown");
 }
 
-/*
- * vshAdmGetTimeStr:
- *
- * Produces string representation (local time) of @then
- * (seconds since epoch UTC) using format 'YYYY-MM-DD HH:MM:SS+ZZZZ'.
- *
- * Returns 0 if conversion finished successfully, -1 in case of an error.
- * Caller is responsible for freeing the string returned.
- */
-static int
-vshAdmGetTimeStr(vshControl *ctl, time_t then, char **result)
-{
-    char *tmp = NULL;
-    struct tm timeinfo;
-
-    if (!localtime_r(&then, &timeinfo))
-        goto error;
-
-    if (VIR_ALLOC_N(tmp, VIRT_ADMIN_TIME_BUFLEN) < 0)
-        goto error;
-
-    if (strftime(tmp, VIRT_ADMIN_TIME_BUFLEN, "%Y-%m-%d %H:%M:%S%z",
-                 &timeinfo) == 0) {
-        VIR_FREE(tmp);
-        goto error;
-    }
-
-    *result = tmp;
-    return 0;
-
- error:
-    vshError(ctl, "%s", _("Timestamp string conversion failed"));
-    return -1;
-}
 
 /*
  * vshAdmCatchDisconnect:
@@ -646,19 +612,19 @@ cmdSrvClientsList(vshControl *ctl, const vshCmd *cmd)
         goto cleanup;
 
     for (i = 0; i < nclts; i++) {
-        g_autofree char *timestr = NULL;
+        g_autoptr(GDateTime) then = NULL;
+        g_autofree gchar *thenstr = NULL;
         g_autofree char *idStr = NULL;
         virAdmClientPtr client = clts[i];
         id = virAdmClientGetID(client);
+        then = g_date_time_new_from_unix_local(virAdmClientGetTimestamp(client));
         transport = virAdmClientGetTransport(client);
-        if (vshAdmGetTimeStr(ctl, virAdmClientGetTimestamp(client),
-                             &timestr) < 0)
-            goto cleanup;
 
+        thenstr = g_date_time_format(then,  "%Y-%m-%d %H:%M:%S%z");
         idStr = g_strdup_printf("%llu", id);
         if (vshTableRowAppend(table, idStr,
                               vshAdmClientTransportToString(transport),
-                              timestr, NULL) < 0)
+                              thenstr, NULL) < 0)
             goto cleanup;
     }
 
@@ -714,7 +680,8 @@ cmdClientInfo(vshControl *ctl, const vshCmd *cmd)
     size_t i;
     unsigned long long id;
     const char *srvname = NULL;
-    char *timestr = NULL;
+    g_autoptr(GDateTime) then = NULL;
+    g_autofree gchar *thenstr = NULL;
     virAdmServerPtr srv = NULL;
     virAdmClientPtr clnt = NULL;
     virTypedParameterPtr params = NULL;
@@ -739,12 +706,13 @@ cmdClientInfo(vshControl *ctl, const vshCmd *cmd)
         goto cleanup;
     }
 
-    if (vshAdmGetTimeStr(ctl, virAdmClientGetTimestamp(clnt), &timestr) < 0)
+
-        goto cleanup;
+    then = g_date_time_new_from_unix_local(virAdmClientGetTimestamp(clnt));
+    thenstr = g_date_time_format(then,  "%Y-%m-%d %H:%M:%S%z");
 
     /* this info is provided by the client object itself */
     vshPrint(ctl, "%-15s: %llu\n", "id", virAdmClientGetID(clnt));
-    vshPrint(ctl, "%-15s: %s\n", "connection_time", timestr);
+    vshPrint(ctl, "%-15s: %s\n", "connection_time", thenstr);
     vshPrint(ctl, "%-15s: %s\n", "transport",
              vshAdmClientTransportToString(virAdmClientGetTransport(clnt)));
 
@@ -760,7 +728,6 @@ cmdClientInfo(vshControl *ctl, const vshCmd *cmd)
     virTypedParamsFree(params, nparams);
     virAdmServerFree(srv);
     virAdmClientFree(clnt);
-    VIR_FREE(timestr);
     return ret;
 }
 

tools/vsh.c

@@ -2182,8 +2182,8 @@ vshOutputLogFile(vshControl *ctl, int log_level, const char *msg_format,
     char *str = NULL;
     size_t len;
     const char *lvl = "";
-    time_t stTime;
+    g_autoptr(GDateTime) now = g_date_time_new_now_local();
-    struct tm stTm;
+    g_autofree gchar *nowstr = NULL;
 
     if (ctl->log_fd == -1)
         return;
@@ -2193,15 +2193,9 @@ vshOutputLogFile(vshControl *ctl, int log_level, const char *msg_format,
      *
      * [YYYY.MM.DD HH:MM:SS SIGNATURE PID] LOG_LEVEL message
     */
-    time(&stTime);
+    nowstr = g_date_time_format(now, "%Y.%m.%d %H:%M:%S");
-    localtime_r(&stTime, &stTm);
+    virBufferAsprintf(&buf, "[%s %s %d] ",
-    virBufferAsprintf(&buf, "[%d.%02d.%02d %02d:%02d:%02d %s %d] ",
+                      nowstr,
-                      (1900 + stTm.tm_year),
-                      (1 + stTm.tm_mon),
-                      stTm.tm_mday,
-                      stTm.tm_hour,
-                      stTm.tm_min,
-                      stTm.tm_sec,
                       ctl->progname,
                       (int) getpid());
     switch (log_level) {