Re: [libvirt] [PATCH 3/3] Remove time APIs from src/util/util.h

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; 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 c36baf8..5430434 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 c02fe70..2459c06 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -57,6 +57,7 @@ #include "network/bridge_driver.h" #include "virnetdev.h" #include "virnodesuspend.h" +#include "virtime.h" #define VIR_FROM_THIS VIR_FROM_LXC @@ -1847,7 +1848,7 @@ static int lxcVmStart(virConnectPtr conn, virCommandSetErrorFD(cmd, &logfd); /* Log timestamp */ - if ((timestamp = virTimestamp()) == NULL) { + if ((timestamp = virTimeStringNow()) == 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 3be97f3..b570193 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -89,6 +89,7 @@ #include "locking/domain_lock.h" #include "virkeycode.h" #include "virnodesuspend.h" +#include "virtime.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -2201,12 +2202,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; } @@ -8677,7 +8678,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..fec3aa8 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 = virTimeStringNow()) == 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 = virTimeStringNow()) == 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 = virTimeStringNow()) == 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 34541ec..278736a 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(&timestamp, "%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 eda60d2..2ba7d9b 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,