[PATCH v3 0/5] Remove qemu_gettimeofday()

[PATCH v3 0/5] Remove qemu_gettimeofday()

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Hi,

Here is a few patches to replace qemu_gettimeofday() helper with functions
provided by GLib.

v3:
- fix wrong qdict_from_jsonf() format change

v2:
- fix return of get-time in nanoseconds
- qga: replace with a common get-time function for qga posix/win32
- split qga patch
- add r-b tags, drop RFC

Marc-André Lureau (5):
  m68k/nios2-semi: fix gettimeofday() result check
  qtest: replace gettimeofday with GTimer
  qga: replace qemu_gettimeofday() with g_get_real_time()
  Replace qemu_gettimeofday() with g_get_real_time()
  oslib: drop qemu_gettimeofday()

 include/sysemu/os-posix.h  |  3 ---
 include/sysemu/os-win32.h  |  6 ------
 blockdev.c                 |  8 ++++----
 hw/rtc/m41t80.c            |  6 +++---
 hw/virtio/virtio-balloon.c |  9 +--------
 qapi/qmp-event.c           |  9 +++------
 qemu-img.c                 |  8 ++++----
 qga/commands-posix.c       | 14 --------------
 qga/commands-win32.c       | 19 -------------------
 qga/commands.c             |  5 +++++
 softmmu/qtest.c            | 39 ++++++++++----------------------------
 target/m68k/m68k-semi.c    | 22 ++++++++++-----------
 target/nios2/nios2-semi.c  | 23 ++++++++++------------
 util/oslib-win32.c         | 20 -------------------
 14 files changed, 50 insertions(+), 141 deletions(-)

-- 
2.35.1.273.ge6ebfd0e8cbb

[PATCH v3 1/5] m68k/nios2-semi: fix gettimeofday() result check

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

gettimeofday() returns 0 for success.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/m68k/m68k-semi.c   | 2 +-
 target/nios2/nios2-semi.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/m68k/m68k-semi.c b/target/m68k/m68k-semi.c
index 44ec7e4612c6..c5c164e096c8 100644
--- a/target/m68k/m68k-semi.c
+++ b/target/m68k/m68k-semi.c
@@ -381,7 +381,7 @@ void do_m68k_semihosting(CPUM68KState *env, int nr)
             qemu_timeval tv;
             struct gdb_timeval *p;
             result = qemu_gettimeofday(&tv);
-            if (result != 0) {
+            if (result == 0) {
                 if (!(p = lock_user(VERIFY_WRITE,
                                     arg0, sizeof(struct gdb_timeval), 0))) {
                     /* FIXME - check error code? */
diff --git a/target/nios2/nios2-semi.c b/target/nios2/nios2-semi.c
index fe5598bae4d7..5a7ad0c7108d 100644
--- a/target/nios2/nios2-semi.c
+++ b/target/nios2/nios2-semi.c
@@ -403,7 +403,7 @@ void do_nios2_semihosting(CPUNios2State *env)
             qemu_timeval tv;
             struct gdb_timeval *p;
             result = qemu_gettimeofday(&tv);
-            if (result != 0) {
+            if (result == 0) {
                 p = lock_user(VERIFY_WRITE, arg0, sizeof(struct gdb_timeval),
                               0);
                 if (!p) {
-- 
2.35.1.273.ge6ebfd0e8cbb

[PATCH v3 2/5] qtest: replace gettimeofday with GTimer

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

glib provides a convenience helper to measure elapsed time. It isn't
subject to wall-clock time changes.

Note that this changes the initial OPENED time, which used to print the
current time.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 softmmu/qtest.c | 39 ++++++++++-----------------------------
 1 file changed, 10 insertions(+), 29 deletions(-)

diff --git a/softmmu/qtest.c b/softmmu/qtest.c
index 8b7cb6aa8e46..b2bb7777d17d 100644
--- a/softmmu/qtest.c
+++ b/softmmu/qtest.c
@@ -58,12 +58,12 @@ static FILE *qtest_log_fp;
 static QTest *qtest;
 static GString *inbuf;
 static int irq_levels[MAX_IRQ];
-static qemu_timeval start_time;
+static GTimer *timer;
 static bool qtest_opened;
 static void (*qtest_server_send)(void*, const char*);
 static void *qtest_server_send_opaque;
 
-#define FMT_timeval "%ld.%06ld"
+#define FMT_timeval "%.06f"
 
 /**
  * DOC: QTest Protocol
@@ -264,28 +264,13 @@ static int hex2nib(char ch)
     }
 }
 
-static void qtest_get_time(qemu_timeval *tv)
-{
-    qemu_gettimeofday(tv);
-    tv->tv_sec -= start_time.tv_sec;
-    tv->tv_usec -= start_time.tv_usec;
-    if (tv->tv_usec < 0) {
-        tv->tv_usec += 1000000;
-        tv->tv_sec -= 1;
-    }
-}
-
 static void qtest_send_prefix(CharBackend *chr)
 {
-    qemu_timeval tv;
-
     if (!qtest_log_fp || !qtest_opened) {
         return;
     }
 
-    qtest_get_time(&tv);
-    fprintf(qtest_log_fp, "[S +" FMT_timeval "] ",
-            (long) tv.tv_sec, (long) tv.tv_usec);
+    fprintf(qtest_log_fp, "[S +" FMT_timeval "] ", g_timer_elapsed(timer, NULL));
 }
 
 static void GCC_FMT_ATTR(1, 2) qtest_log_send(const char *fmt, ...)
@@ -386,12 +371,9 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
     command = words[0];
 
     if (qtest_log_fp) {
-        qemu_timeval tv;
         int i;
 
-        qtest_get_time(&tv);
-        fprintf(qtest_log_fp, "[R +" FMT_timeval "]",
-                (long) tv.tv_sec, (long) tv.tv_usec);
+        fprintf(qtest_log_fp, "[R +" FMT_timeval "]", g_timer_elapsed(timer, NULL));
         for (i = 0; words[i]; i++) {
             fprintf(qtest_log_fp, " %s", words[i]);
         }
@@ -846,21 +828,20 @@ static void qtest_event(void *opaque, QEMUChrEvent event)
         for (i = 0; i < ARRAY_SIZE(irq_levels); i++) {
             irq_levels[i] = 0;
         }
-        qemu_gettimeofday(&start_time);
+
+        g_clear_pointer(&timer, g_timer_destroy);
+        timer = g_timer_new();
         qtest_opened = true;
         if (qtest_log_fp) {
-            fprintf(qtest_log_fp, "[I " FMT_timeval "] OPENED\n",
-                    (long) start_time.tv_sec, (long) start_time.tv_usec);
+            fprintf(qtest_log_fp, "[I " FMT_timeval "] OPENED\n", g_timer_elapsed(timer, NULL));
         }
         break;
     case CHR_EVENT_CLOSED:
         qtest_opened = false;
         if (qtest_log_fp) {
-            qemu_timeval tv;
-            qtest_get_time(&tv);
-            fprintf(qtest_log_fp, "[I +" FMT_timeval "] CLOSED\n",
-                    (long) tv.tv_sec, (long) tv.tv_usec);
+            fprintf(qtest_log_fp, "[I +" FMT_timeval "] CLOSED\n", g_timer_elapsed(timer, NULL));
         }
+        g_clear_pointer(&timer, g_timer_destroy);
         break;
     default:
         break;
-- 
2.35.1.273.ge6ebfd0e8cbb

[PATCH v3 3/5] qga: replace qemu_gettimeofday() with g_get_real_time()

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

GLib g_get_real_time() is an alternative to gettimeofday() which allows
to simplify our code.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 qga/commands-posix.c | 14 --------------
 qga/commands-win32.c | 19 -------------------
 qga/commands.c       |  5 +++++
 3 files changed, 5 insertions(+), 33 deletions(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 75dbaab68ea9..1e7b4656edd1 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -136,20 +136,6 @@ void qmp_guest_shutdown(bool has_mode, const char *mode, Error **errp)
     /* succeeded */
 }
 
-int64_t qmp_guest_get_time(Error **errp)
-{
-   int ret;
-   qemu_timeval tq;
-
-   ret = qemu_gettimeofday(&tq);
-   if (ret < 0) {
-       error_setg_errno(errp, errno, "Failed to get time");
-       return -1;
-   }
-
-   return tq.tv_sec * 1000000000LL + tq.tv_usec * 1000;
-}
-
 void qmp_guest_set_time(bool has_time, int64_t time_ns, Error **errp)
 {
     int ret;
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 4fbbad793f2e..ce0af5ba45fc 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -1751,25 +1751,6 @@ static int64_t filetime_to_ns(const FILETIME *tf)
             - W32_FT_OFFSET) * 100;
 }
 
-int64_t qmp_guest_get_time(Error **errp)
-{
-    SYSTEMTIME ts = {0};
-    FILETIME tf;
-
-    GetSystemTime(&ts);
-    if (ts.wYear < 1601 || ts.wYear > 30827) {
-        error_setg(errp, "Failed to get time");
-        return -1;
-    }
-
-    if (!SystemTimeToFileTime(&ts, &tf)) {
-        error_setg(errp, "Failed to convert system time: %d", (int)GetLastError());
-        return -1;
-    }
-
-    return filetime_to_ns(&tf);
-}
-
 void qmp_guest_set_time(bool has_time, int64_t time_ns, Error **errp)
 {
     Error *local_err = NULL;
diff --git a/qga/commands.c b/qga/commands.c
index 80501e4a737c..653ba3061e24 100644
--- a/qga/commands.c
+++ b/qga/commands.c
@@ -585,3 +585,8 @@ GuestFileRead *qmp_guest_file_read(int64_t handle, bool has_count,
 
     return read_data;
 }
+
+int64_t qmp_guest_get_time(Error **errp)
+{
+    return g_get_real_time() * 1000;
+}
-- 
2.35.1.273.ge6ebfd0e8cbb

[PATCH v3 4/5] Replace qemu_gettimeofday() with g_get_real_time()

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

GLib g_get_real_time() is an alternative to gettimeofday() which allows
to simplify our code.

For semihosting, a few bits are lost on POSIX host, but this shouldn't
be a big concern.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 blockdev.c                 |  8 ++++----
 hw/rtc/m41t80.c            |  6 +++---
 hw/virtio/virtio-balloon.c |  9 +--------
 qapi/qmp-event.c           |  9 +++------
 qemu-img.c                 |  8 ++++----
 target/m68k/m68k-semi.c    | 22 ++++++++++------------
 target/nios2/nios2-semi.c  | 23 ++++++++++-------------
 7 files changed, 35 insertions(+), 50 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index e46e8312121e..9230888e349f 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1258,7 +1258,7 @@ static void internal_snapshot_prepare(BlkActionState *common,
     BlockDriverState *bs;
     QEMUSnapshotInfo old_sn, *sn;
     bool ret;
-    qemu_timeval tv;
+    int64_t rt;
     BlockdevSnapshotInternal *internal;
     InternalSnapshotState *state;
     AioContext *aio_context;
@@ -1328,9 +1328,9 @@ static void internal_snapshot_prepare(BlkActionState *common,
     /* 3. take the snapshot */
     sn = &state->sn;
     pstrcpy(sn->name, sizeof(sn->name), name);
-    qemu_gettimeofday(&tv);
-    sn->date_sec = tv.tv_sec;
-    sn->date_nsec = tv.tv_usec * 1000;
+    rt = g_get_real_time();
+    sn->date_sec = rt / G_USEC_PER_SEC;
+    sn->date_nsec = (rt % G_USEC_PER_SEC) * 1000;
     sn->vm_clock_nsec = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
     if (replay_mode != REPLAY_MODE_NONE) {
         sn->icount = replay_get_current_icount();
diff --git a/hw/rtc/m41t80.c b/hw/rtc/m41t80.c
index a00971a67e1c..e045c864bb44 100644
--- a/hw/rtc/m41t80.c
+++ b/hw/rtc/m41t80.c
@@ -47,7 +47,7 @@ static uint8_t m41t80_recv(I2CSlave *i2c)
 {
     M41t80State *s = M41T80(i2c);
     struct tm now;
-    qemu_timeval tv;
+    int64_t rt;
 
     if (s->addr < 0) {
         s->addr = 0;
@@ -57,8 +57,8 @@ static uint8_t m41t80_recv(I2CSlave *i2c)
     }
     switch (s->addr++) {
     case 0:
-        qemu_gettimeofday(&tv);
-        return to_bcd(tv.tv_usec / 10000);
+        rt = g_get_real_time();
+        return to_bcd((rt % G_USEC_PER_SEC) / 10000);
     case 1:
         return to_bcd(now.tm_sec);
     case 2:
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index e6c1b0aa46fe..b1bada84cecc 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -452,7 +452,6 @@ static void virtio_balloon_receive_stats(VirtIODevice *vdev, VirtQueue *vq)
     VirtQueueElement *elem;
     VirtIOBalloonStat stat;
     size_t offset = 0;
-    qemu_timeval tv;
 
     elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
     if (!elem) {
@@ -484,13 +483,7 @@ static void virtio_balloon_receive_stats(VirtIODevice *vdev, VirtQueue *vq)
             s->stats[tag] = val;
     }
     s->stats_vq_offset = offset;
-
-    if (qemu_gettimeofday(&tv) < 0) {
-        warn_report("%s: failed to get time of day", __func__);
-        goto out;
-    }
-
-    s->stats_last_update = tv.tv_sec;
+    s->stats_last_update = g_get_real_time() / G_USEC_PER_SEC;
 
 out:
     if (balloon_stats_enabled(s)) {
diff --git a/qapi/qmp-event.c b/qapi/qmp-event.c
index 19d3cd003833..0fe0d0a5a6e5 100644
--- a/qapi/qmp-event.c
+++ b/qapi/qmp-event.c
@@ -20,15 +20,12 @@
 
 static void timestamp_put(QDict *qdict)
 {
-    int err;
     QDict *ts;
-    qemu_timeval tv;
+    int64_t rt = g_get_real_time();
 
-    err = qemu_gettimeofday(&tv);
-    /* Put -1 to indicate failure of getting host time */
     ts = qdict_from_jsonf_nofail("{ 'seconds': %lld, 'microseconds': %lld }",
-                                 err < 0 ? -1LL : (long long)tv.tv_sec,
-                                 err < 0 ? -1LL : (long long)tv.tv_usec);
+                                 (long long)rt / G_USEC_PER_SEC,
+                                 (long long)rt % G_USEC_PER_SEC);
     qdict_put(qdict, "timestamp", ts);
 }
 
diff --git a/qemu-img.c b/qemu-img.c
index 6fe2466032f9..e26773909684 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -3304,11 +3304,11 @@ static int img_snapshot(int argc, char **argv)
     char *filename, *snapshot_name = NULL;
     int c, ret = 0, bdrv_oflags;
     int action = 0;
-    qemu_timeval tv;
     bool quiet = false;
     Error *err = NULL;
     bool image_opts = false;
     bool force_share = false;
+    int64_t rt;
 
     bdrv_oflags = BDRV_O_RDWR;
     /* Parse commandline parameters */
@@ -3405,9 +3405,9 @@ static int img_snapshot(int argc, char **argv)
         memset(&sn, 0, sizeof(sn));
         pstrcpy(sn.name, sizeof(sn.name), snapshot_name);
 
-        qemu_gettimeofday(&tv);
-        sn.date_sec = tv.tv_sec;
-        sn.date_nsec = tv.tv_usec * 1000;
+        rt = g_get_real_time();
+        sn.date_sec = rt / G_USEC_PER_SEC;
+        sn.date_nsec = (rt % G_USEC_PER_SEC) * 1000;
 
         ret = bdrv_snapshot_create(bs, &sn);
         if (ret) {
diff --git a/target/m68k/m68k-semi.c b/target/m68k/m68k-semi.c
index c5c164e096c8..37343d47e247 100644
--- a/target/m68k/m68k-semi.c
+++ b/target/m68k/m68k-semi.c
@@ -378,19 +378,17 @@ void do_m68k_semihosting(CPUM68KState *env, int nr)
                            arg0, arg1);
             return;
         } else {
-            qemu_timeval tv;
             struct gdb_timeval *p;
-            result = qemu_gettimeofday(&tv);
-            if (result == 0) {
-                if (!(p = lock_user(VERIFY_WRITE,
-                                    arg0, sizeof(struct gdb_timeval), 0))) {
-                    /* FIXME - check error code? */
-                    result = -1;
-                } else {
-                    p->tv_sec = cpu_to_be32(tv.tv_sec);
-                    p->tv_usec = cpu_to_be64(tv.tv_usec);
-                    unlock_user(p, arg0, sizeof(struct gdb_timeval));
-                }
+            int64_t rt = g_get_real_time();
+            p = lock_user(VERIFY_WRITE, arg0, sizeof(struct gdb_timeval), 0);
+            if (!p) {
+                /* FIXME - check error code? */
+                result = -1;
+            } else {
+                result = 0;
+                p->tv_sec = cpu_to_be32(rt / G_USEC_PER_SEC);
+                p->tv_usec = cpu_to_be64(rt % G_USEC_PER_SEC);
+                unlock_user(p, arg0, sizeof(struct gdb_timeval));
             }
         }
         break;
diff --git a/target/nios2/nios2-semi.c b/target/nios2/nios2-semi.c
index 5a7ad0c7108d..3decf6924c8f 100644
--- a/target/nios2/nios2-semi.c
+++ b/target/nios2/nios2-semi.c
@@ -400,20 +400,17 @@ void do_nios2_semihosting(CPUNios2State *env)
                            arg0, 0);
             return;
         } else {
-            qemu_timeval tv;
             struct gdb_timeval *p;
-            result = qemu_gettimeofday(&tv);
-            if (result == 0) {
-                p = lock_user(VERIFY_WRITE, arg0, sizeof(struct gdb_timeval),
-                              0);
-                if (!p) {
-                    result = -1;
-                    errno = EFAULT;
-                } else {
-                    p->tv_sec = cpu_to_be32(tv.tv_sec);
-                    p->tv_usec = cpu_to_be64(tv.tv_usec);
-                    unlock_user(p, arg0, sizeof(struct gdb_timeval));
-                }
+            int64_t rt = g_get_real_time();
+            p = lock_user(VERIFY_WRITE, arg0, sizeof(struct gdb_timeval), 0);
+            if (!p) {
+                result = -1;
+                errno = EFAULT;
+            } else {
+                result = 0;
+                p->tv_sec = cpu_to_be32(rt / G_USEC_PER_SEC);
+                p->tv_usec = cpu_to_be64(rt % G_USEC_PER_SEC);
+                unlock_user(p, arg0, sizeof(struct gdb_timeval));
             }
         }
         break;
-- 
2.35.1.273.ge6ebfd0e8cbb

[PATCH v3 5/5] oslib: drop qemu_gettimeofday()

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

No longer used after the previous patches.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Reviewed-by: Stefan Weil <sw@weilnetz.de>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/sysemu/os-posix.h |  3 ---
 include/sysemu/os-win32.h |  6 ------
 util/oslib-win32.c        | 20 --------------------
 3 files changed, 29 deletions(-)

diff --git a/include/sysemu/os-posix.h b/include/sysemu/os-posix.h
index dd64fb401dfb..23bd45457d71 100644
--- a/include/sysemu/os-posix.h
+++ b/include/sysemu/os-posix.h
@@ -52,9 +52,6 @@ int os_mlock(void);
 #define closesocket(s) close(s)
 #define ioctlsocket(s, r, v) ioctl(s, r, v)
 
-typedef struct timeval qemu_timeval;
-#define qemu_gettimeofday(tp) gettimeofday(tp, NULL)
-
 int os_set_daemonize(bool d);
 bool is_daemonized(void);
 
diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
index 770752222ae3..1351d1d29ece 100644
--- a/include/sysemu/os-win32.h
+++ b/include/sysemu/os-win32.h
@@ -71,12 +71,6 @@ int getpagesize(void);
 # define EPROTONOSUPPORT EINVAL
 #endif
 
-typedef struct {
-    long tv_sec;
-    long tv_usec;
-} qemu_timeval;
-int qemu_gettimeofday(qemu_timeval *tp);
-
 static inline int os_set_daemonize(bool d)
 {
     if (d) {
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index af559ef3398d..7faf59e9aaea 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -265,26 +265,6 @@ void qemu_set_cloexec(int fd)
 {
 }
 
-/* Offset between 1/1/1601 and 1/1/1970 in 100 nanosec units */
-#define _W32_FT_OFFSET (116444736000000000ULL)
-
-int qemu_gettimeofday(qemu_timeval *tp)
-{
-  union {
-    unsigned long long ns100; /*time since 1 Jan 1601 in 100ns units */
-    FILETIME ft;
-  }  _now;
-
-  if(tp) {
-      GetSystemTimeAsFileTime (&_now.ft);
-      tp->tv_usec=(long)((_now.ns100 / 10ULL) % 1000000ULL );
-      tp->tv_sec= (long)((_now.ns100 - _W32_FT_OFFSET) / 10000000ULL);
-  }
-  /* Always return 0 as per Open Group Base Specifications Issue 6.
-     Do not set errno on error.  */
-  return 0;
-}
-
 int qemu_get_thread_id(void)
 {
     return GetCurrentThreadId();
-- 
2.35.1.273.ge6ebfd0e8cbb

Re: [PATCH v3 2/5] qtest: replace gettimeofday with GTimer

[Thomas Huth]

On 07/03/2022 08.03, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> glib provides a convenience helper to measure elapsed time. It isn't
> subject to wall-clock time changes.
> 
> Note that this changes the initial OPENED time, which used to print the
> current time.
[...]
> @@ -846,21 +828,20 @@ static void qtest_event(void *opaque, QEMUChrEvent event)
>           for (i = 0; i < ARRAY_SIZE(irq_levels); i++) {
>               irq_levels[i] = 0;
>           }
> -        qemu_gettimeofday(&start_time);
> +
> +        g_clear_pointer(&timer, g_timer_destroy);
> +        timer = g_timer_new();
>           qtest_opened = true;
>           if (qtest_log_fp) {
> -            fprintf(qtest_log_fp, "[I " FMT_timeval "] OPENED\n",
> -                    (long) start_time.tv_sec, (long) start_time.tv_usec);
> +            fprintf(qtest_log_fp, "[I " FMT_timeval "] OPENED\n", g_timer_elapsed(timer, NULL));
>           }
>           break;

The new timestamp here is quite unuseful now, of course ... could you 
replace it with g_get_current_time()  instead?

  Thomas

Re: [PATCH v3 3/5] qga: replace qemu_gettimeofday() with g_get_real_time()

[Thomas Huth]

On 07/03/2022 08.03, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> GLib g_get_real_time() is an alternative to gettimeofday() which allows
> to simplify our code.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> ---
>   qga/commands-posix.c | 14 --------------
>   qga/commands-win32.c | 19 -------------------
>   qga/commands.c       |  5 +++++
>   3 files changed, 5 insertions(+), 33 deletions(-)


Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH v3 2/5] qtest: replace gettimeofday with GTimer

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 1411 bytes --]

Hi

On Mon, Mar 7, 2022 at 11:46 AM Thomas Huth <thuth@redhat.com> wrote:

> On 07/03/2022 08.03, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > glib provides a convenience helper to measure elapsed time. It isn't
> > subject to wall-clock time changes.
> >
> > Note that this changes the initial OPENED time, which used to print the
> > current time.
> [...]
> > @@ -846,21 +828,20 @@ static void qtest_event(void *opaque, QEMUChrEvent
> event)
> >           for (i = 0; i < ARRAY_SIZE(irq_levels); i++) {
> >               irq_levels[i] = 0;
> >           }
> > -        qemu_gettimeofday(&start_time);
> > +
> > +        g_clear_pointer(&timer, g_timer_destroy);
> > +        timer = g_timer_new();
> >           qtest_opened = true;
> >           if (qtest_log_fp) {
> > -            fprintf(qtest_log_fp, "[I " FMT_timeval "] OPENED\n",
> > -                    (long) start_time.tv_sec, (long)
> start_time.tv_usec);
> > +            fprintf(qtest_log_fp, "[I " FMT_timeval "] OPENED\n",
> g_timer_elapsed(timer, NULL));
> >           }
> >           break;
>
> The new timestamp here is quite unuseful now, of course ... could you
> replace it with g_get_current_time()  instead?
>

Eventually, but I wonder why this (and only this) particular timestamp
should be the current time.


-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 2179 bytes --]

Re: [PATCH v3 2/5] qtest: replace gettimeofday with GTimer

[Thomas Huth]

On 07/03/2022 09.05, Marc-André Lureau wrote:
> Hi
> 
> On Mon, Mar 7, 2022 at 11:46 AM Thomas Huth <thuth@redhat.com 
> <mailto:thuth@redhat.com>> wrote:
> 
>     On 07/03/2022 08.03, marcandre.lureau@redhat.com
>     <mailto:marcandre.lureau@redhat.com> wrote:
>      > From: Marc-André Lureau <marcandre.lureau@redhat.com
>     <mailto:marcandre.lureau@redhat.com>>
>      >
>      > glib provides a convenience helper to measure elapsed time. It isn't
>      > subject to wall-clock time changes.
>      >
>      > Note that this changes the initial OPENED time, which used to print the
>      > current time.
>     [...]
>      > @@ -846,21 +828,20 @@ static void qtest_event(void *opaque,
>     QEMUChrEvent event)
>      >           for (i = 0; i < ARRAY_SIZE(irq_levels); i++) {
>      >               irq_levels[i] = 0;
>      >           }
>      > -        qemu_gettimeofday(&start_time);
>      > +
>      > +        g_clear_pointer(&timer, g_timer_destroy);
>      > +        timer = g_timer_new();
>      >           qtest_opened = true;
>      >           if (qtest_log_fp) {
>      > -            fprintf(qtest_log_fp, "[I " FMT_timeval "] OPENED\n",
>      > -                    (long) start_time.tv_sec, (long)
>     start_time.tv_usec);
>      > +            fprintf(qtest_log_fp, "[I " FMT_timeval "] OPENED\n",
>     g_timer_elapsed(timer, NULL));
>      >           }
>      >           break;
> 
>     The new timestamp here is quite unuseful now, of course ... could you
>     replace it with g_get_current_time()  instead?
> 
> 
> Eventually, but I wonder why this (and only this) particular timestamp 
> should be the current time.

I assume it was meant as a possibility to sync the times in this log with 
other things that are going on on the host system in parallel. If you only 
have the relative time stamps in the log here, you cannot compare the events 
to other logs anymore.
(having said that, I wonder why it doesn't simply always use the current 
wall time and uses the relative time instead, but maybe there is also a 
reason for that...)

  Thomas