6:38 a.m.
There are rare reports of libvirt deadlocking itself which show a
fork'd, but not exec'd, child process stuck in localtime_r(). This
is because localtime_r() is not an async signal, so if we happened
to fork() at the exact time another thread was calling localtime_r()
we could get stuck with a stale lock in the child.
Although we go to pains to protect the libvirt logging code by
acquiring the mutex before forking, this is not sufficient, because
some non-logging related code could invoke virTimestamp() which
calls localtime_r() too.
The only sane approach to fix this is to make our timestamp
generation APIs thread safe, by doing a clean impl of gmtime()
and strftime() inside libvirt using static buffers and only async
safe POSIX apis.
6:38 a.m.
New subject: [libvirt] [PATCH 1/3] Add internal APIs for dealing with time
From: "Daniel P. Berrange" <berrange(a)redhat.com>
The logging APIs need to be able to generate formatted timestamps
using only async signal safe functions. This rules out using
gmtime/localtime/malloc/gettimeday(!) and much more.
Introduce a new internal API which is async signal safe.
virTimeMillisNow - replacement for gettimeofday. Uses clock_gettime
where available, otherwise falls back to the unsafe
gettimeofday
virTimeFieldsNow - replacements for gmtime(), convert a timestamp
virTimeFieldsThen into a broken out set of fields. No localtime()
replacement is provided, because converting to
local time is not practical with only async signal
safe APIs.
virTimeStringNow - replacements for strftime() which print a timestamp
virTimeStringThen into a string, using a pre-determined format, with
a fixed size buffer (VIR_TIME_STRING_BUFLEN)
virTimeStringNewNow - alternative to above which malloc a string. Not
virTimeStringNewThen to be used in async signal safe scenarios.
* src/Makefile.am, src/util/virtime.c, src/util/virtime.h: New files
* src/libvirt_private.syms: New APis
* configure.ac: Check for clock_gettime in -lrt
* tests/virtimetest.c, tests/Makefile.am: Test new APIs
---
configure.ac | 10 ++
src/Makefile.am | 7 +-
src/libvirt_private.syms | 11 ++
src/util/virtime.c | 290 ++++++++++++++++++++++++++++++++++++++++++++++
src/util/virtime.h | 50 ++++++++
tests/.gitignore | 1 +
tests/Makefile.am | 9 ++-
tests/virtimetest.c | 124 ++++++++++++++++++++
8 files changed, 499 insertions(+), 3 deletions(-)
create mode 100644 src/util/virtime.c
create mode 100644 src/util/virtime.h
create mode 100644 tests/virtimetest.c
diff --git a/configure.ac b/configure.ac
index e03e401..de2f379 100644
--- a/configure.ac
+++ b/configure.ac
@@ -147,6 +147,16 @@ LIBS="$LIBS $LIB_PTHREAD $LIBMULTITHREAD"
AC_CHECK_FUNCS([pthread_mutexattr_init])
LIBS=$old_libs
+old_LIBS=$LIBS
+RT_LIBS=
+LIBS="$LIBS $LIB_PTHREAD -lrt"
+AC_CHECK_FUNC([clock_gettime],[
+ AC_DEFINE([HAVE_CLOCK_GETTIME],[],[Defined if clock_gettime() exists in librt.so])
+ RT_LIBS=-lrt
+])
+LIBS=$old_libs
+AC_SUBST(RT_LIBS)
+
dnl Availability of various common headers (non-fatal if missing).
AC_CHECK_HEADERS([pwd.h paths.h regex.h sys/un.h \
sys/poll.h syslog.h mntent.h net/ethernet.h linux/magic.h \
diff --git a/src/Makefile.am b/src/Makefile.am
index 33a32a8..872639f 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -97,7 +97,8 @@ UTIL_SOURCES = \
util/virnetdevtap.h util/virnetdevtap.c \
util/virnetdevveth.h util/virnetdevveth.c \
util/virnetdevvportprofile.h util/virnetdevvportprofile.c \
- util/virsocketaddr.h util/virsocketaddr.c
+ util/virsocketaddr.h util/virsocketaddr.c \
+ util/virtime.h util/virtime.c
EXTRA_DIST += $(srcdir)/util/virkeymaps.h $(srcdir)/util/keymaps.csv \
$(srcdir)/util/virkeycode-mapgen.py
@@ -561,7 +562,8 @@ libvirt_util_la_SOURCES = \
libvirt_util_la_CFLAGS = $(CAPNG_CFLAGS) $(YAJL_CFLAGS) $(LIBNL_CFLAGS) \
$(AM_CFLAGS) $(AUDIT_CFLAGS) $(DEVMAPPER_CFLAGS)
libvirt_util_la_LIBADD = $(CAPNG_LIBS) $(YAJL_LIBS) $(LIBNL_LIBS) \
- $(THREAD_LIBS) $(AUDIT_LIBS) $(DEVMAPPER_LIBS)
+ $(THREAD_LIBS) $(AUDIT_LIBS) $(DEVMAPPER_LIBS) \
+ $(RT_LIBS)
noinst_LTLIBRARIES += libvirt_conf.la
@@ -1500,6 +1502,7 @@ libvirt_lxc_LDFLAGS = $(WARN_CFLAGS) $(AM_LDFLAGS)
libvirt_lxc_LDADD = $(CAPNG_LIBS) $(YAJL_LIBS) \
$(LIBXML_LIBS) $(NUMACTL_LIBS) $(THREAD_LIBS) \
$(LIBNL_LIBS) $(AUDIT_LIBS) $(DEVMAPPER_LIBS) \
+ $(RT_LIBS) \
../gnulib/lib/libgnu.la
if WITH_DTRACE
libvirt_lxc_LDADD += probes.o
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 0b21cdc..c90210d 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1380,6 +1380,17 @@ virKeycodeSetTypeFromString;
virKeycodeValueFromString;
virKeycodeValueTranslate;
+
+# virtime.h
+virTimeMillisNow;
+virTimeFieldsNow;
+virTimeFieldsThen;
+virTimeStringNow;
+virTimeStringThen;
+virTimeStringNewNow;
+virTimeStringNewThen;
+
+
# xml.h
virXMLParseHelper;
virXMLPropString;
diff --git a/src/util/virtime.c b/src/util/virtime.c
new file mode 100644
index 0000000..c2d907f
--- /dev/null
+++ b/src/util/virtime.c
@@ -0,0 +1,290 @@
+/*
+ * virtime.c: Time handling functions
+ *
+ * Copyright (C) 2006-2011 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
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ * Author: Daniel P. Berrange <berrange(a)redhat.com>
+ *
+ * The intent is that this file provides a set of time APIs which
+ * are async signal safe, to allow use in between fork/exec eg by
+ * the logging code.
+ *
+ * The reality is that wsnprintf is technically unsafe. We ought
+ * to roll out our int -> str conversions to avoid this.
+ *
+ * We do *not* use regular libvirt error APIs for most of the code,
+ * since those are not async signal safe, and we dont want logging
+ * APIs generating timestamps to blow away real errors
+ */
+
+#include <config.h>
+
+#include <stdio.h>
+#ifndef HAVE_CLOCK_GETTIME
+#include <sys/time.h>
+#endif
+
+#include "virtime.h"
+#include "util.h"
+#include "memory.h"
+#include "virterror_internal.h"
+
+#define VIR_FROM_THIS VIR_FROM_NONE
+
+/* We prefer clock_gettime if available because that is officially
+ * async signal safe according to POSIX. Many platforms lack it
+ * though, so fallback to gettimeofday everywhere else
+ */
+
+/**
+ * virTimeMillisNow:
+ * @now: filled with current time in milliseconds
+ *
+ * Retrieves the current system time, in milliseconds since the
+ * epoch
+ *
+ * Returns 0 on success, -1 on error with errno set
+ */
+int virTimeMillisNow(unsigned long long *now)
+{
+#ifdef HAVE_CLOCK_GETTIME
+ struct timespec ts;
+
+ if (clock_gettime(CLOCK_REALTIME, &ts) < 0)
+ return -1;
+
+ *now = (ts.tv_sec * 1000ull) + (ts.tv_nsec / (1000ull * 1000ull));
+#else
+ struct timeval tv;
+
+ if (gettimeofday(&tv, NULL) < 0)
+ return -1;
+
+ *now = (tv.tv_sec * 1000ull) + (tv.tv_usec / 1000ull);
+#endif
+
+ return 0;
+}
+
+
+/**
+ * virTimeFieldsNow:
+ * @fields: filled with current time fields
+ *
+ * Retrieves the current time, in broken-down field format.
+ * The time is always in UTC.
+ *
+ * Returns 0 on success, -1 on error
+ */
+int virTimeFieldsNow(struct tm *fields)
+{
+ unsigned long long now;
+
+ if (virTimeMillisNow(&now) < 0)
+ return -1;
+
+ return virTimeFieldsThen(now, fields);
+}
+
+
+#define SECS_PER_HOUR (60 * 60)
+#define SECS_PER_DAY (SECS_PER_HOUR * 24)
+#define DIV(a, b) ((a) / (b) - ((a) % (b) < 0))
+#define LEAPS_THRU_END_OF(y) (DIV (y, 4) - DIV (y, 100) + DIV (y, 400))
+
+const unsigned short int __mon_yday[2][13] = {
+ /* Normal years. */
+ { 0, 31, 59, 90, 120, 151, 181, 212, 243, 273, 304, 334, 365 },
+ /* Leap years. */
+ { 0, 31, 60, 91, 121, 152, 182, 213, 244, 274, 305, 335, 366 }
+};
+
+/**
+ * virTimeFieldsThen:
+ * @when: the time to convert in milliseconds
+ * @fields: filled with time @when fields
+ *
+ * Converts the timestamp @when into broken-down field format.
+ * Time time is always in UTC
+ *
+ * Returns 0 on success, -1 on error
+ */
+int virTimeFieldsThen(unsigned long long when, struct tm *fields)
+{
+ /* This code is taken from GLibC under terms of LGPLv2+ */
+ long int days, rem, y;
+ const unsigned short int *ip;
+ unsigned long long whenSecs = when / 1000ull;
+ unsigned int offset = 0; /* We hardcoded GMT */
+
+ days = whenSecs / SECS_PER_DAY;
+ rem = whenSecs % SECS_PER_DAY;
+ rem += offset;
+ while (rem < 0) {
+ rem += SECS_PER_DAY;
+ --days;
+ }
+ while (rem >= SECS_PER_DAY) {
+ rem -= SECS_PER_DAY;
+ ++days;
+ }
+ fields->tm_hour = rem / SECS_PER_HOUR;
+ rem %= SECS_PER_HOUR;
+ fields->tm_min = rem / 60;
+ fields->tm_sec = rem % 60;
+ /* January 1, 1970 was a Thursday. */
+ fields->tm_wday = (4 + days) % 7;
+ if (fields->tm_wday < 0)
+ fields->tm_wday += 7;
+ y = 1970;
+
+ while (days < 0 || days >= (__isleap (y) ? 366 : 365)) {
+ /* Guess a corrected year, assuming 365 days per year. */
+ long int yg = y + days / 365 - (days % 365 < 0);
+
+ /* Adjust DAYS and Y to match the guessed year. */
+ days -= ((yg - y) * 365
+ + LEAPS_THRU_END_OF (yg - 1)
+ - LEAPS_THRU_END_OF (y - 1));
+ y = yg;
+ }
+ fields->tm_year = y - 1900;
+
+ fields->tm_yday = days;
+ ip = __mon_yday[__isleap(y)];
+ for (y = 11; days < (long int) ip[y]; --y)
+ continue;
+ days -= ip[y];
+ fields->tm_mon = y;
+ fields->tm_mday = days + 1;
+ return 0;
+}
+
+
+/**
+ * virTimeStringNow:
+ * @buf: a buffer at least VIR_TIME_STRING_BUFLEN in length
+ *
+ * Initializes @buf to contain a formatted timestamp
+ * corresponding to the current time.
+ *
+ * Returns 0 on success, -1 on error
+ */
+int virTimeStringNow(char *buf)
+{
+ unsigned long long now;
+
+ if (virTimeMillisNow(&now) < 0)
+ return -1;
+
+ return virTimeStringThen(now, buf);
+}
+
+
+/**
+ * virTimeStringThen:
+ * @when: the time to format in milliseconds
+ * @buf: a buffer at least VIR_TIME_STRING_BUFLEN in length
+ *
+ * Initializes @buf to contain a formatted timestamp
+ * corresponding to the time @when.
+ *
+ * Returns 0 on success, -1 on error
+ */
+int virTimeStringThen(unsigned long long when, char *buf)
+{
+ struct tm fields;
+
+ if (virTimeFieldsThen(when, &fields) < 0)
+ return -1;
+
+ fields.tm_year += 1900;
+ fields.tm_mon += 1;
+
+ if (snprintf(buf, VIR_TIME_STRING_BUFLEN,
+ "%4d-%02d-%02d %02d:%02d:%02d.%03d +0000",
+ fields.tm_year, fields.tm_mon, fields.tm_mday,
+ fields.tm_hour, fields.tm_min, fields.tm_sec,
+ (int) (when % 1000)) >= VIR_TIME_STRING_BUFLEN) {
+ errno = ERANGE;
+ return -1;
+ }
+
+ return 0;
+}
+
+
+/**
+ * virTimeStringNow:
+ *
+ * Creates a string containing a formatted timestamp
+ * corresponding to the current time.
+ *
+ * This function is not async signal safe
+ *
+ * Returns a formatted allocated string, or NULL on error
+ */
+char *virTimeStringNewNow(void)
+{
+ char *ret;
+
+ if (VIR_ALLOC_N(ret, VIR_TIME_STRING_BUFLEN) < 0) {
+ virReportOOMError();
+ return NULL;
+ }
+
+ if (virTimeStringNow(ret) < 0) {
+ virReportSystemError(errno, "%s",
+ _("Unable to format time"));
+ VIR_FREE(ret);
+ return NULL;
+ }
+
+ return ret;
+}
+
+
+/**
+ * virTimeStringThen:
+ * @when: the time to format in milliseconds
+ *
+ * Creates a string containing a formatted timestamp
+ * corresponding to the time @when.
+ *
+ * This function is not async signal safe
+ *
+ * Returns a formatted allocated string, or NULL on error
+ */
+char *virTimeStringNewThen(unsigned long long when)
+{
+ char *ret;
+
+ if (VIR_ALLOC_N(ret, VIR_TIME_STRING_BUFLEN) < 0) {
+ virReportOOMError();
+ return NULL;
+ }
+
+ if (virTimeStringThen(when, ret) < 0) {
+ virReportSystemError(errno, "%s",
+ _("Unable to format time"));
+ VIR_FREE(ret);
+ return NULL;
+ }
+
+ return ret;
+}
+
diff --git a/src/util/virtime.h b/src/util/virtime.h
new file mode 100644
index 0000000..3955ba6
--- /dev/null
+++ b/src/util/virtime.h
@@ -0,0 +1,50 @@
+/*
+ * virtime.h: Time handling functions
+ *
+ * Copyright (C) 2006-2011 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
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ * Author: Daniel P. Berrange <berrange(a)redhat.com>
+ */
+
+#ifndef __VIR_TIME_H__
+#define __VIR_TIME_H__
+
+#include <time.h>
+
+/* The format string we intend to use is:
+ *
+ * Yr Mon Day Hour Min Sec Ms TZ
+ * %4d-%02d-%02d %02d:%02d:%02d.%03d +0000
+ *
+ */
+#define VIR_TIME_STRING_BUFLEN \
+ (4 + 1 + 2 + 1 + 2 + 1 + 2 + 1 + 2 + 1 + 2 + 1 + 3 + 1 + 5 + 1)
+/* Yr Mon Day Hour Min Sec Ms TZ NULL */
+
+int virTimeMillisNow(unsigned long long *now);
+
+int virTimeFieldsNow(struct tm *fields);
+int virTimeFieldsThen(unsigned long long when, struct tm *fields);
+
+int virTimeStringNow(char *buf);
+int virTimeStringThen(unsigned long long when, char *buf);
+
+char *virTimeStringNewNow(void);
+char *virTimeStringNewThen(unsigned long long when);
+
+
+#endif
diff --git a/tests/.gitignore b/tests/.gitignore
index 7159c37..027b421 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -36,6 +36,7 @@ virnetmessagetest
virnetsockettest
virnettlscontexttest
virshtest
+virtimetest
vmx2xmltest
xencapstest
xmconfigtest
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 6bff670..f3b0c09 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -96,7 +96,8 @@ check_PROGRAMS = virshtest conftest sockettest \
nodeinfotest qparamtest virbuftest \
commandtest commandhelper seclabeltest \
hashtest virnetmessagetest virnetsockettest ssh \
- utiltest virnettlscontexttest shunloadtest
+ utiltest virnettlscontexttest shunloadtest \
+ virtimetest
check_LTLIBRARIES = libshunload.la
@@ -217,6 +218,7 @@ TESTS = virshtest \
virnetmessagetest \
virnetsockettest \
virnettlscontexttest \
+ virtimetest \
shunloadtest \
utiltest \
$(test_scripts)
@@ -495,6 +497,11 @@ else
EXTRA_DIST += pkix_asn1_tab.c
endif
+virtimetest_SOURCES = \
+ virtimetest.c testutils.h testutils.c
+virtimetest_CFLAGS = -Dabs_builddir="\"$(abs_builddir)\""
$(AM_CFLAGS)
+virtimetest_LDADD = ../src/libvirt-net-rpc.la $(LDADDS)
+
seclabeltest_SOURCES = \
seclabeltest.c
diff --git a/tests/virtimetest.c b/tests/virtimetest.c
new file mode 100644
index 0000000..5d56dd3
--- /dev/null
+++ b/tests/virtimetest.c
@@ -0,0 +1,124 @@
+/*
+ * Copyright (C) 2011 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
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ * Author: Daniel P. Berrange <berrange(a)redhat.com>
+ */
+
+#include <config.h>
+
+#include <stdlib.h>
+#include <signal.h>
+
+#include "testutils.h"
+#include "util.h"
+#include "virterror_internal.h"
+#include "memory.h"
+#include "logging.h"
+
+#include "virtime.h"
+
+#define VIR_FROM_THIS VIR_FROM_RPC
+
+struct testTimeFieldsData {
+ unsigned long long when;
+ struct tm fields;
+};
+
+static int testTimeFields(const void *args)
+{
+ const struct testTimeFieldsData *data = args;
+ struct tm actual;
+
+ if (virTimeFieldsThen(data->when, &actual) < 0)
+ return -1;
+
+#define COMPARE(field) \
+ do { \
+ if (data->fields.field != actual.field) { \
+ VIR_DEBUG("Expect " #field " %d got %d", \
+ data->fields.field, actual.field); \
+ return -1; \
+ } \
+ } while (0)
+
+ /* tm_year value 0 is based off epoch 1900 */
+ actual.tm_year += 1900;
+ /* tm_mon is range 0-11, but we want 1-12 */
+ actual.tm_mon += 1;
+
+ COMPARE(tm_year);
+ COMPARE(tm_mon);
+ COMPARE(tm_mday);
+ COMPARE(tm_hour);
+ COMPARE(tm_min);
+ COMPARE(tm_sec);
+
+ return 0;
+}
+
+
+static int
+mymain(void)
+{
+ int ret = 0;
+
+ signal(SIGPIPE, SIG_IGN);
+
+#define TEST_FIELDS(ts, year, mon, day, hour, min, sec) \
+ do { \
+ struct testTimeFieldsData data = { \
+ .when = ts, \
+ .fields = { \
+ .tm_year = year, \
+ .tm_mon = mon, \
+ .tm_mday = day, \
+ .tm_hour = hour, \
+ .tm_min = min, \
+ .tm_sec = sec, \
+ .tm_wday = 0, \
+ .tm_yday = 0, \
+ .tm_isdst = 0, \
+ }, \
+ }; \
+ if (virtTestRun("Test fields " #ts " " #year " ",
1, testTimeFields, &data) < 0) \
+ ret = -1; \
+ } while (0)
+
+ TEST_FIELDS( 0ull, 1970, 1, 1, 0, 0, 0);
+ TEST_FIELDS( 5000ull, 1970, 1, 1, 0, 0, 5);
+ TEST_FIELDS( 3605000ull, 1970, 1, 1, 1, 0, 5);
+ TEST_FIELDS( 86405000ull, 1970, 1, 2, 0, 0, 5);
+ TEST_FIELDS( 31536000000ull, 1971, 1, 1, 0, 0, 0);
+
+ TEST_FIELDS( 30866399000ull, 1970, 12, 24, 5, 59, 59);
+ TEST_FIELDS( 123465599000ull, 1973, 11, 29, 23, 59, 59);
+ TEST_FIELDS( 155001599000ull, 1974, 11, 29, 23, 59, 59);
+
+ TEST_FIELDS( 186537599000ull, 1975, 11, 29, 23, 59, 59);
+ TEST_FIELDS( 344390399000ull, 1980, 11, 29, 23, 59, 59);
+ TEST_FIELDS(1203161493000ull, 2008, 2, 16, 11, 31, 33);
+ TEST_FIELDS(1234567890000ull, 2009, 2, 13, 23, 31, 30);
+
+ TEST_FIELDS(1322524800000ull, 2011, 11, 29, 0, 0, 0);
+ TEST_FIELDS(1322611199000ull, 2011, 11, 29, 23, 59, 59);
+
+ TEST_FIELDS(2147483648000ull, 2038, 1, 19, 3, 14, 8);
+
+ return (ret==0 ? EXIT_SUCCESS : EXIT_FAILURE);
+}
+
+VIRT_TEST_MAIN(mymain)
--
1.7.6.4
6:38 a.m.
New subject: [libvirt] [PATCH 2/3] Make logging async signal safe wrt time stamp generation
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Use the new virTimeStringNow() API for generating log timestamps
in an async signal safe manner
* src/util/logging.c: Use virTimeStringNow
---
src/util/logging.c | 28 ++++------------------------
1 files changed, 4 insertions(+), 24 deletions(-)
diff --git a/src/util/logging.c b/src/util/logging.c
index 17c7e84..332716e 100644
--- a/src/util/logging.c
+++ b/src/util/logging.c
@@ -43,6 +43,7 @@
#include "buf.h"
#include "threads.h"
#include "virfile.h"
+#include "virtime.h"
#define VIR_FROM_THIS VIR_FROM_NONE
@@ -618,26 +619,6 @@ cleanup:
return ret;
}
-static char *
-virLogFormatTimestamp(void)
-{
- struct timeval cur_time;
- struct tm time_info;
- char *str = NULL;
-
- gettimeofday(&cur_time, NULL);
- localtime_r(&cur_time.tv_sec, &time_info);
- time_info.tm_year += 1900;
- time_info.tm_mon += 1;
-
- if (virAsprintf(&str, "%4d-%02d-%02d %02d:%02d:%02d.%03d",
- time_info.tm_year, time_info.tm_mon, time_info.tm_mday,
- time_info.tm_hour, time_info.tm_min, time_info.tm_sec,
- (int) (cur_time.tv_usec / 1000)) < 0)
- return NULL;
-
- return str;
-}
static int
virLogFormatString(char **msg,
@@ -705,7 +686,7 @@ void virLogMessage(const char *category, int priority, const char
*funcname,
static bool logVersionStderr = true;
char *str = NULL;
char *msg = NULL;
- char *timestamp = NULL;
+ char timestamp[VIR_TIME_STRING_BUFLEN];
int fprio, i, ret;
int saved_errno = errno;
int emit = 1;
@@ -746,8 +727,8 @@ void virLogMessage(const char *category, int priority, const char
*funcname,
if (ret < 0)
goto cleanup;
- if (!(timestamp = virLogFormatTimestamp()))
- goto cleanup;
+ if (virTimeStringNow(timestamp) < 0)
+ timestamp[0] = '\0';
/*
* Log based on defaults, first store in the history buffer,
@@ -799,7 +780,6 @@ void virLogMessage(const char *category, int priority, const char
*funcname,
cleanup:
VIR_FREE(msg);
- VIR_FREE(timestamp);
errno = saved_errno;
}
--
1.7.6.4
6:38 a.m.
New subject: [libvirt] [PATCH 3/3] Remove time APIs from src/util/util.h
From: "Daniel P. Berrange" <berrange(a)redhat.com>
The virTimestamp and virTimeMs functions in src/util/util.h
duplicate functionality from virtime.h, in a non-async signal
safe manner. Remove them, and convert all code over to the new
APIs.
* src/util/util.c, src/util/util.h: Delete virTimeMs and virTimestamp
* src/lxc/lxc_driver.c, src/qemu/qemu_domain.c,
src/qemu/qemu_driver.c, src/qemu/qemu_migration.c,
src/qemu/qemu_process.c, src/util/event_poll.c: Convert to use
virtime APIs
---
src/libvirt_private.syms | 2 -
src/lxc/lxc_driver.c | 3 +-
src/qemu/qemu_domain.c | 5 ++-
src/qemu/qemu_driver.c | 7 +++--
src/qemu/qemu_migration.c | 5 ++-
src/qemu/qemu_process.c | 9 ++++---
src/util/event_poll.c | 9 ++++---
src/util/util.c | 53 ---------------------------------------------
src/util/util.h | 4 ---
9 files changed, 22 insertions(+), 75 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c90210d..b910d41 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1133,8 +1133,6 @@ virStrToLong_ul;
virStrToLong_ull;
virStrcpy;
virStrncpy;
-virTimeMs;
-virTimestamp;
virTrimSpaces;
virTypedParameterArrayClear;
virVasprintf;
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 6f44d09..16bdfb9 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -56,6 +56,7 @@
#include "domain_nwfilter.h"
#include "network/bridge_driver.h"
#include "virnetdev.h"
+#include "virtime.h"
#define VIR_FROM_THIS VIR_FROM_LXC
@@ -1846,7 +1847,7 @@ static int lxcVmStart(virConnectPtr conn,
virCommandSetErrorFD(cmd, &logfd);
/* Log timestamp */
- if ((timestamp = virTimestamp()) == NULL) {
+ if ((timestamp = virTimeStringNewNow()) == NULL) {
virReportOOMError();
goto cleanup;
}
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index d33d1d9..b28c734 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -36,6 +36,7 @@
#include "uuid.h"
#include "virfile.h"
#include "domain_event.h"
+#include "virtime.h"
#include <sys/time.h>
#include <fcntl.h>
@@ -728,7 +729,7 @@ qemuDomainObjBeginJobInternal(struct qemud_driver *driver,
priv->jobs_queued++;
- if (virTimeMs(&now) < 0)
+ if (virTimeMillisNow(&now) < 0)
return -1;
then = now + QEMU_JOB_WAIT_TIME;
@@ -934,7 +935,7 @@ qemuDomainObjEnterMonitorInternal(struct qemud_driver *driver,
qemuMonitorLock(priv->mon);
qemuMonitorRef(priv->mon);
- ignore_value(virTimeMs(&priv->monStart));
+ ignore_value(virTimeMillisNow(&priv->monStart));
virDomainObjUnlock(obj);
if (driver_locked)
qemuDriverUnlock(driver);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6cfdd1d..c22b678 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -88,6 +88,7 @@
#include "locking/lock_manager.h"
#include "locking/domain_lock.h"
#include "virkeycode.h"
+#include "virtime.h"
#define VIR_FROM_THIS VIR_FROM_QEMU
@@ -2200,12 +2201,12 @@ qemuDomainGetControlInfo(virDomainPtr dom,
} else if (priv->job.active) {
if (!priv->monStart) {
info->state = VIR_DOMAIN_CONTROL_JOB;
- if (virTimeMs(&info->stateTime) < 0)
+ if (virTimeMillisNow(&info->stateTime) < 0)
goto cleanup;
info->stateTime -= priv->job.start;
} else {
info->state = VIR_DOMAIN_CONTROL_OCCUPIED;
- if (virTimeMs(&info->stateTime) < 0)
+ if (virTimeMillisNow(&info->stateTime) < 0)
goto cleanup;
info->stateTime -= priv->monStart;
}
@@ -8586,7 +8587,7 @@ static int qemuDomainGetJobInfo(virDomainPtr dom,
* of incoming migration which we don't currently
* monitor actively in the background thread
*/
- if (virTimeMs(&info->timeElapsed) < 0)
+ if (virTimeMillisNow(&info->timeElapsed) < 0)
goto cleanup;
info->timeElapsed -= priv->job.start;
} else {
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 8ae989a..8c4ecc8 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -42,6 +42,7 @@
#include "datatypes.h"
#include "fdstream.h"
#include "uuid.h"
+#include "virtime.h"
#include "locking/domain_lock.h"
#include "rpc/virnetsocket.h"
@@ -866,7 +867,7 @@ qemuMigrationUpdateJobStatus(struct qemud_driver *driver,
&memTotal);
qemuDomainObjExitMonitorWithDriver(driver, vm);
- if (ret < 0 || virTimeMs(&priv->job.info.timeElapsed) < 0) {
+ if (ret < 0 || virTimeMillisNow(&priv->job.info.timeElapsed) < 0) {
priv->job.info.type = VIR_DOMAIN_JOB_FAILED;
return -1;
}
@@ -1098,7 +1099,7 @@ qemuMigrationPrepareAny(struct qemud_driver *driver,
bool tunnel = !!st;
char *origname = NULL;
- if (virTimeMs(&now) < 0)
+ if (virTimeMillisNow(&now) < 0)
return -1;
if (!(def = virDomainDefParseString(driver->caps, dom_xml,
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 2563f97..90db8fa 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -60,6 +60,7 @@
#include "locking/domain_lock.h"
#include "network/bridge_driver.h"
#include "uuid.h"
+#include "virtime.h"
#define VIR_FROM_THIS VIR_FROM_QEMU
@@ -849,7 +850,7 @@ qemuConnectMonitor(struct qemud_driver *driver, virDomainObjPtr vm)
* deleted while the monitor is active */
virDomainObjRef(vm);
- ignore_value(virTimeMs(&priv->monStart));
+ ignore_value(virTimeMillisNow(&priv->monStart));
virDomainObjUnlock(vm);
qemuDriverUnlock(driver);
@@ -3053,7 +3054,7 @@ int qemuProcessStart(virConnectPtr conn,
goto cleanup;
}
- if ((timestamp = virTimestamp()) == NULL) {
+ if ((timestamp = virTimeStringNewNow()) == NULL) {
virReportOOMError();
goto cleanup;
} else {
@@ -3331,7 +3332,7 @@ void qemuProcessStop(struct qemud_driver *driver,
VIR_WARN("Unable to open logfile: %s",
virStrerror(errno, ebuf, sizeof ebuf));
} else {
- if ((timestamp = virTimestamp()) == NULL) {
+ if ((timestamp = virTimeStringNewNow()) == NULL) {
virReportOOMError();
} else {
if (safewrite(logfile, timestamp, strlen(timestamp)) < 0 ||
@@ -3601,7 +3602,7 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED,
priv->persistentAddrs = 0;
}
- if ((timestamp = virTimestamp()) == NULL) {
+ if ((timestamp = virTimeStringNewNow()) == NULL) {
virReportOOMError();
goto cleanup;
} else {
diff --git a/src/util/event_poll.c b/src/util/event_poll.c
index 0b75471..30dec74 100644
--- a/src/util/event_poll.c
+++ b/src/util/event_poll.c
@@ -39,6 +39,7 @@
#include "virfile.h"
#include "ignore-value.h"
#include "virterror_internal.h"
+#include "virtime.h"
#define EVENT_DEBUG(fmt, ...) VIR_DEBUG(fmt, __VA_ARGS__)
@@ -214,7 +215,7 @@ int virEventPollAddTimeout(int frequency,
unsigned long long now;
int ret;
- if (virTimeMs(&now) < 0) {
+ if (virTimeMillisNow(&now) < 0) {
return -1;
}
@@ -262,7 +263,7 @@ void virEventPollUpdateTimeout(int timer, int frequency)
return;
}
- if (virTimeMs(&now) < 0) {
+ if (virTimeMillisNow(&now) < 0) {
return;
}
@@ -337,7 +338,7 @@ static int virEventPollCalculateTimeout(int *timeout) {
if (then > 0) {
unsigned long long now;
- if (virTimeMs(&now) < 0)
+ if (virTimeMillisNow(&now) < 0)
return -1;
*timeout = then - now;
@@ -413,7 +414,7 @@ static int virEventPollDispatchTimeouts(void)
int ntimeouts = eventLoop.timeoutsCount;
VIR_DEBUG("Dispatch %d", ntimeouts);
- if (virTimeMs(&now) < 0)
+ if (virTimeMillisNow(&now) < 0)
return -1;
for (i = 0 ; i < ntimeouts ; i++) {
diff --git a/src/util/util.c b/src/util/util.c
index ce697fb..ce8f8ba 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -33,12 +33,10 @@
#include <fcntl.h>
#include <errno.h>
#include <poll.h>
-#include <time.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <sys/ioctl.h>
#include <sys/wait.h>
-#include <sys/time.h>
#if HAVE_MMAP
# include <sys/mman.h>
#endif
@@ -2498,57 +2496,6 @@ int virBuildPathInternal(char **path, ...)
return ret;
}
-/**
- * virTimestamp:
- *
- * Return an allocated string containing the current date and time,
- * followed by ": ". Return NULL on allocation failure.
- */
-char *
-virTimestamp(void)
-{
- struct timeval cur_time;
- struct tm time_info;
- char timestr[100];
- char *timestamp;
-
- gettimeofday(&cur_time, NULL);
- localtime_r(&cur_time.tv_sec, &time_info);
-
- strftime(timestr, sizeof(timestr), "%Y-%m-%d %H:%M:%S", &time_info);
-
- if (virAsprintf(×tamp, "%s.%03d",
- timestr, (int) cur_time.tv_usec / 1000) < 0) {
- return NULL;
- }
-
- return timestamp;
-}
-
-#define timeval_to_ms(tv) (((tv).tv_sec * 1000ull) + ((tv).tv_usec / 1000))
-
-/**
- * virTimeMs:
- *
- * Get current time in milliseconds.
- *
- * Returns 0 on success, -1 on failure.
- */
-int
-virTimeMs(unsigned long long *ms)
-{
- struct timeval now;
-
- if (gettimeofday(&now, NULL) < 0) {
- virReportSystemError(errno, "%s",
- _("cannot get time of day"));
- return -1;
- }
-
- *ms = timeval_to_ms(now);
- return 0;
-}
-
#if HAVE_LIBDEVMAPPER_H
bool
virIsDevMapperDevice(const char *dev_name)
diff --git a/src/util/util.h b/src/util/util.h
index 5afcf58..f8feb6c 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -249,10 +249,6 @@ void virFileWaitForDevices(void);
# define virBuildPath(path, ...) virBuildPathInternal(path, __VA_ARGS__, NULL)
int virBuildPathInternal(char **path, ...) ATTRIBUTE_SENTINEL;
-char *virTimestamp(void);
-
-int virTimeMs(unsigned long long *ms) ATTRIBUTE_NONNULL(1);
-
bool virIsDevMapperDevice(const char *dev_name) ATTRIBUTE_NONNULL(1);
int virEmitXMLWarning(int fd,
--
1.7.6.4
7:10 a.m.
New subject: [libvirt] [PATCH 3/3] Remove time APIs from src/util/util.h
On Tue, Nov 29, 2011 at 12:38:24 +0000, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange(a)redhat.com>
The virTimestamp and virTimeMs functions in src/util/util.h
duplicate functionality from virtime.h, in a non-async signal
safe manner. Remove them, and convert all code over to the new
APIs.
* src/util/util.c, src/util/util.h: Delete virTimeMs and virTimestamp
* src/lxc/lxc_driver.c, src/qemu/qemu_domain.c,
src/qemu/qemu_driver.c, src/qemu/qemu_migration.c,
src/qemu/qemu_process.c, src/util/event_poll.c: Convert to use
virtime APIs
---
src/libvirt_private.syms | 2 -
src/lxc/lxc_driver.c | 3 +-
src/qemu/qemu_domain.c | 5 ++-
src/qemu/qemu_driver.c | 7 +++--
src/qemu/qemu_migration.c | 5 ++-
src/qemu/qemu_process.c | 9 ++++---
src/util/event_poll.c | 9 ++++---
src/util/util.c | 53 ---------------------------------------------
src/util/util.h | 4 ---
9 files changed, 22 insertions(+), 75 deletions(-)
...
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index d33d1d9..b28c734 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -36,6 +36,7 @@
#include "uuid.h"
#include "virfile.h"
#include "domain_event.h"
+#include "virtime.h"
#include <sys/time.h>
#include <fcntl.h>
@@ -728,7 +729,7 @@ qemuDomainObjBeginJobInternal(struct qemud_driver *driver,
priv->jobs_queued++;
- if (virTimeMs(&now) < 0)
+ if (virTimeMillisNow(&now) < 0)
return -1;
then = now + QEMU_JOB_WAIT_TIME;
...
This (and other similar ones in this patch) simple replacement won't work
since virTimeMs used to report libvirt error while virTimeMillisNow doesn't do
that. Thus in case of error (although it's not a frequent one)
qemuDomainObjBeginJobInternal would return -1 without reporting any error.
Jirka
7:43 a.m.
New subject: [libvirt] [PATCH 3/3] Remove time APIs from src/util/util.h
On Tue, Nov 29, 2011 at 02:10:20PM +0100, Jiri Denemark wrote:
On Tue, Nov 29, 2011 at 12:38:24 +0000, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange(a)redhat.com>
>
> The virTimestamp and virTimeMs functions in src/util/util.h
> duplicate functionality from virtime.h, in a non-async signal
> safe manner. Remove them, and convert all code over to the new
> APIs.
>
> * src/util/util.c, src/util/util.h: Delete virTimeMs and virTimestamp
> * src/lxc/lxc_driver.c, src/qemu/qemu_domain.c,
> src/qemu/qemu_driver.c, src/qemu/qemu_migration.c,
> src/qemu/qemu_process.c, src/util/event_poll.c: Convert to use
> virtime APIs
> ---
> src/libvirt_private.syms | 2 -
> src/lxc/lxc_driver.c | 3 +-
> src/qemu/qemu_domain.c | 5 ++-
> src/qemu/qemu_driver.c | 7 +++--
> src/qemu/qemu_migration.c | 5 ++-
> src/qemu/qemu_process.c | 9 ++++---
> src/util/event_poll.c | 9 ++++---
> src/util/util.c | 53 ---------------------------------------------
> src/util/util.h | 4 ---
> 9 files changed, 22 insertions(+), 75 deletions(-)
>
...
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index d33d1d9..b28c734 100644> > --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -36,6 +36,7 @@
> #include "uuid.h"
> #include "virfile.h"
> #include "domain_event.h"
> +#include "virtime.h"
>
> #include <sys/time.h>
> #include <fcntl.h>
> @@ -728,7 +729,7 @@ qemuDomainObjBeginJobInternal(struct qemud_driver *driver,
>
> priv->jobs_queued++;
>
> - if (virTimeMs(&now) < 0)
> + if (virTimeMillisNow(&now) < 0)
> return -1;
> then = now + QEMU_JOB_WAIT_TIME;
>
...
This (and other similar ones in this patch) simple replacement won't work
since virTimeMs used to report libvirt error while virTimeMillisNow doesn't do
that. Thus in case of error (although it's not a frequent one)
qemuDomainObjBeginJobInternal would return -1 without reporting any error.
Oh whoops. I guess I'll create a second variant (which isn't async
signal safe) which raises real errors to use in this scenario
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
4742
days inactive
4742
days old
5 comments
2 participants
participants (2)
-
Daniel P. Berrange -
Jiri Denemark