[PATCH 0/4] RFC: remove qemu_gettimeofday()

[PATCH 0/4] RFC: 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.

Marc-André Lureau (4):
  m68k/nios2-semi: fix gettimeofday() result check
  qtest: replace gettimeofday with GTimer
  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           | 12 +++++-------
 qemu-img.c                 |  8 ++++----
 qga/commands-posix.c       | 11 +----------
 softmmu/qtest.c            | 39 ++++++++++----------------------------
 target/m68k/m68k-semi.c    | 22 ++++++++++-----------
 target/nios2/nios2-semi.c  | 24 +++++++++++------------
 util/oslib-win32.c         | 20 -------------------
 12 files changed, 49 insertions(+), 119 deletions(-)

-- 
2.35.1.273.ge6ebfd0e8cbb

[PATCH 1/4] 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>
---
 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 2/4] 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>
---
 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 3/4] 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().

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>
---
 blockdev.c                 |  8 ++++----
 hw/rtc/m41t80.c            |  6 +++---
 hw/virtio/virtio-balloon.c |  9 +--------
 qapi/qmp-event.c           | 12 +++++-------
 qemu-img.c                 |  8 ++++----
 qga/commands-posix.c       | 11 +----------
 target/m68k/m68k-semi.c    | 22 ++++++++++------------
 target/nios2/nios2-semi.c  | 24 +++++++++++-------------
 8 files changed, 39 insertions(+), 61 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 42e098b458b1..4b07dbfbdefc 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1235,7 +1235,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;
@@ -1305,9 +1305,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..025716b3ec37 100644
--- a/qapi/qmp-event.c
+++ b/qapi/qmp-event.c
@@ -20,15 +20,13 @@
 
 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);
+    ts = qdict_from_jsonf_nofail("{ 'seconds': %" G_GINT64_FORMAT
+                                 ", 'microseconds': %" G_GINT64_FORMAT "}",
+                                 rt / G_USEC_PER_SEC,
+                                 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/qga/commands-posix.c b/qga/commands-posix.c
index 75dbaab68ea9..3311a4ca0167 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -138,16 +138,7 @@ void qmp_guest_shutdown(bool has_mode, const char *mode, Error **errp)
 
 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;
+    return g_get_real_time();
 }
 
 void qmp_guest_set_time(bool has_time, int64_t time_ns, Error **errp)
diff --git a/target/m68k/m68k-semi.c b/target/m68k/m68k-semi.c
index c5c164e096c8..54f735743165 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();
+            if (!(p = lock_user(VERIFY_WRITE,
+                                arg0, sizeof(struct gdb_timeval), 0))) {
+                /* 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..4600bf0a39d9 100644
--- a/target/nios2/nios2-semi.c
+++ b/target/nios2/nios2-semi.c
@@ -400,20 +400,18 @@ 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 4/4] 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>
---
 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 2edf33658a44..c9b1d63fedda 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)
-
 bool is_daemonized(void);
 
 /**
diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
index 43f569b5c216..4d4be826f48c 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 bool is_daemonized(void)
 {
     return false;
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 1/4] m68k/nios2-semi: fix gettimeofday() result check

[Laurent Vivier]

Le 04/03/2022 à 16:27, marcandre.lureau@redhat.com a écrit :
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> gettimeofday() returns 0 for success.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>   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) {

It seems to have never worked correctly...

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

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

[Laurent Vivier]

Le 04/03/2022 à 16:27, marcandre.lureau@redhat.com a écrit :
> 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.
> 

Time is printed using FMT_timeval ("%ld.%06ld"), but g_timer_elapsed() returns a gdouble.

Are you sure it works properly?

Laurent

> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>   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;

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

[Laurent Vivier]

Le 04/03/2022 à 16:54, Laurent Vivier a écrit :
> Le 04/03/2022 à 16:27, marcandre.lureau@redhat.com a écrit :
>> 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.
>>
> 
> Time is printed using FMT_timeval ("%ld.%06ld"), but g_timer_elapsed() returns a gdouble.
> 
> Are you sure it works properly?

Sorry, I missed this part:

>> -#define FMT_timeval "%ld.%06ld"
>> +#define FMT_timeval "%.06f"

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Re: [PATCH 3/4] Replace qemu_gettimeofday() with g_get_real_time()

[Laurent Vivier]

Le 04/03/2022 à 16:27, marcandre.lureau@redhat.com a écrit :
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> GLib g_get_real_time() is an alternative to gettimeofday().
> 
> 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>
> ---
>   blockdev.c                 |  8 ++++----
>   hw/rtc/m41t80.c            |  6 +++---
>   hw/virtio/virtio-balloon.c |  9 +--------
>   qapi/qmp-event.c           | 12 +++++-------
>   qemu-img.c                 |  8 ++++----
>   qga/commands-posix.c       | 11 +----------
>   target/m68k/m68k-semi.c    | 22 ++++++++++------------
>   target/nios2/nios2-semi.c  | 24 +++++++++++-------------
>   8 files changed, 39 insertions(+), 61 deletions(-)
> 
...
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index 75dbaab68ea9..3311a4ca0167 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -138,16 +138,7 @@ void qmp_guest_shutdown(bool has_mode, const char *mode, Error **errp)
>   
>   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;
> +    return g_get_real_time();

now you return microseconds, before it was nanoseconds.

qga/qapi-schema.json:

##
# @guest-get-time:
#
# Get the information about guest's System Time relative to
# the Epoch of 1970-01-01 in UTC.
#
# Returns: Time in nanoseconds.
#
# Since: 1.5
##
{ 'command': 'guest-get-time',
   'returns': 'int' }

Except this problem:

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Re: [PATCH 4/4] oslib: drop qemu_gettimeofday()

[Laurent Vivier]

Le 04/03/2022 à 16:27, marcandre.lureau@redhat.com a écrit :
> 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>
> ---
>   include/sysemu/os-posix.h |  3 ---
>   include/sysemu/os-win32.h |  6 ------
>   util/oslib-win32.c        | 20 --------------------
>   3 files changed, 29 deletions(-)
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Re: [PATCH 3/4] Replace qemu_gettimeofday() with g_get_real_time()

[Laurent Vivier]

Le 04/03/2022 à 17:08, Laurent Vivier a écrit :
> Le 04/03/2022 à 16:27, marcandre.lureau@redhat.com a écrit :
>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>
>> GLib g_get_real_time() is an alternative to gettimeofday().
>>
>> 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>
>> ---
>>   blockdev.c                 |  8 ++++----
>>   hw/rtc/m41t80.c            |  6 +++---
>>   hw/virtio/virtio-balloon.c |  9 +--------
>>   qapi/qmp-event.c           | 12 +++++-------
>>   qemu-img.c                 |  8 ++++----
>>   qga/commands-posix.c       | 11 +----------
>>   target/m68k/m68k-semi.c    | 22 ++++++++++------------
>>   target/nios2/nios2-semi.c  | 24 +++++++++++-------------
>>   8 files changed, 39 insertions(+), 61 deletions(-)
>>
> ...
>> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
>> index 75dbaab68ea9..3311a4ca0167 100644
>> --- a/qga/commands-posix.c
>> +++ b/qga/commands-posix.c
>> @@ -138,16 +138,7 @@ void qmp_guest_shutdown(bool has_mode, const char *mode, Error **errp)
>>   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;
>> +    return g_get_real_time();
> 
> now you return microseconds, before it was nanoseconds.
> 
> qga/qapi-schema.json:
> 
> ##
> # @guest-get-time:
> #
> # Get the information about guest's System Time relative to
> # the Epoch of 1970-01-01 in UTC.
> #
> # Returns: Time in nanoseconds.
> #
> # Since: 1.5
> ##
> { 'command': 'guest-get-time',
>    'returns': 'int' }
> 
> Except this problem:
> 
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Perhaps you can also remove the windows version from qga/commands-win32.c and move the function to 
qga/commands.c ?

Thanks,
Laurent

Re: [PATCH 4/4] oslib: drop qemu_gettimeofday()

[Stefan Weil]

[-- Attachment #1.1.1: Type: text/plain, Size: 2395 bytes --]

Am 04.03.22 um 16:27 schrieb marcandre.lureau@redhat.com:

> 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>
> ---
>   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 2edf33658a44..c9b1d63fedda 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)
> -
>   bool is_daemonized(void);
>   
>   /**
> diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
> index 43f569b5c216..4d4be826f48c 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 bool is_daemonized(void)
>   {
>       return false;
> 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();


Reviewed-by: Stefan Weil <sw@weilnetz.de>



[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 15123 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH 3/4] Replace qemu_gettimeofday() with g_get_real_time()

[Marc-André Lureau]

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

Hi

On Fri, Mar 4, 2022 at 8:10 PM Laurent Vivier <laurent@vivier.eu> wrote:

> Le 04/03/2022 à 16:27, marcandre.lureau@redhat.com a écrit :
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > GLib g_get_real_time() is an alternative to gettimeofday().
> >
> > 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>
> > ---
> >   blockdev.c                 |  8 ++++----
> >   hw/rtc/m41t80.c            |  6 +++---
> >   hw/virtio/virtio-balloon.c |  9 +--------
> >   qapi/qmp-event.c           | 12 +++++-------
> >   qemu-img.c                 |  8 ++++----
> >   qga/commands-posix.c       | 11 +----------
> >   target/m68k/m68k-semi.c    | 22 ++++++++++------------
> >   target/nios2/nios2-semi.c  | 24 +++++++++++-------------
> >   8 files changed, 39 insertions(+), 61 deletions(-)
> >
> ...
> > diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> > index 75dbaab68ea9..3311a4ca0167 100644
> > --- a/qga/commands-posix.c
> > +++ b/qga/commands-posix.c
> > @@ -138,16 +138,7 @@ void qmp_guest_shutdown(bool has_mode, const char
> *mode, Error **errp)
> >
> >   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;
> > +    return g_get_real_time();
>
> now you return microseconds, before it was nanoseconds.
>
>
my bad, will fix it in v2, thanks for noticing!


> qga/qapi-schema.json:
>
> ##
> # @guest-get-time:
> #
> # Get the information about guest's System Time relative to
> # the Epoch of 1970-01-01 in UTC.
> #
> # Returns: Time in nanoseconds.
> #
> # Since: 1.5
> ##
> { 'command': 'guest-get-time',
>    'returns': 'int' }
>
> Except this problem:
>
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
>
>

-- 
Marc-André Lureau

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

Re: [PATCH 3/4] Replace qemu_gettimeofday() with g_get_real_time()

[BALATON Zoltan]

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

On Fri, 4 Mar 2022, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> GLib g_get_real_time() is an alternative to gettimeofday().
>
> 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>
> ---
> blockdev.c                 |  8 ++++----
> hw/rtc/m41t80.c            |  6 +++---
> hw/virtio/virtio-balloon.c |  9 +--------
> qapi/qmp-event.c           | 12 +++++-------
> qemu-img.c                 |  8 ++++----
> qga/commands-posix.c       | 11 +----------
> target/m68k/m68k-semi.c    | 22 ++++++++++------------
> target/nios2/nios2-semi.c  | 24 +++++++++++-------------
> 8 files changed, 39 insertions(+), 61 deletions(-)
>
> diff --git a/blockdev.c b/blockdev.c
> index 42e098b458b1..4b07dbfbdefc 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -1235,7 +1235,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;
> @@ -1305,9 +1305,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

This part

Acked-by: BALATON Zoltan <balaton@eik.bme.hu>

but why don't you use g_get_current_time() instead which seems to be a 
more direct replacement for gettimeofday, then you don't have to do maths 
with G_USEC_PER_SEC.

Regards.
BALATON Zoltan

> @@ -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..025716b3ec37 100644
> --- a/qapi/qmp-event.c
> +++ b/qapi/qmp-event.c
> @@ -20,15 +20,13 @@
>
> 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);
> +    ts = qdict_from_jsonf_nofail("{ 'seconds': %" G_GINT64_FORMAT
> +                                 ", 'microseconds': %" G_GINT64_FORMAT "}",
> +                                 rt / G_USEC_PER_SEC,
> +                                 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/qga/commands-posix.c b/qga/commands-posix.c
> index 75dbaab68ea9..3311a4ca0167 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -138,16 +138,7 @@ void qmp_guest_shutdown(bool has_mode, const char *mode, Error **errp)
>
> 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;
> +    return g_get_real_time();
> }
>
> void qmp_guest_set_time(bool has_time, int64_t time_ns, Error **errp)
> diff --git a/target/m68k/m68k-semi.c b/target/m68k/m68k-semi.c
> index c5c164e096c8..54f735743165 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();
> +            if (!(p = lock_user(VERIFY_WRITE,
> +                                arg0, sizeof(struct gdb_timeval), 0))) {
> +                /* 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..4600bf0a39d9 100644
> --- a/target/nios2/nios2-semi.c
> +++ b/target/nios2/nios2-semi.c
> @@ -400,20 +400,18 @@ 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;
>

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

[Richard Henderson]

On 3/4/22 05:27, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau<marcandre.lureau@redhat.com>
> 
> gettimeofday() returns 0 for success.
> 
> Signed-off-by: Marc-André Lureau<marcandre.lureau@redhat.com>
> ---
>   target/m68k/m68k-semi.c   | 2 +-
>   target/nios2/nios2-semi.c | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 3/4] Replace qemu_gettimeofday() with g_get_real_time()

[Marc-André Lureau]

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

Hi

On Sat, Mar 5, 2022 at 12:40 AM BALATON Zoltan <balaton@eik.bme.hu> wrote:

> On Fri, 4 Mar 2022, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > GLib g_get_real_time() is an alternative to gettimeofday().
> >
> > 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>
> > ---
> > blockdev.c                 |  8 ++++----
> > hw/rtc/m41t80.c            |  6 +++---
> > hw/virtio/virtio-balloon.c |  9 +--------
> > qapi/qmp-event.c           | 12 +++++-------
> > qemu-img.c                 |  8 ++++----
> > qga/commands-posix.c       | 11 +----------
> > target/m68k/m68k-semi.c    | 22 ++++++++++------------
> > target/nios2/nios2-semi.c  | 24 +++++++++++-------------
> > 8 files changed, 39 insertions(+), 61 deletions(-)
> >
> > diff --git a/blockdev.c b/blockdev.c
> > index 42e098b458b1..4b07dbfbdefc 100644
> > --- a/blockdev.c
> > +++ b/blockdev.c
> > @@ -1235,7 +1235,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;
> > @@ -1305,9 +1305,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
>
> This part
>
> Acked-by: BALATON Zoltan <balaton@eik.bme.hu>
>
> but why don't you use g_get_current_time() instead which seems to be a
> more direct replacement for gettimeofday, then you don't have to do maths
> with G_USEC_PER_SEC.>
>
>
As per glib docs, "g_get_current_time has been deprecated since version
2.62 and should not be used in newly-written code. GTimeVal is not
year-2038-safe. Use g_get_real_time() instead."

thanks




> Regards.
> BALATON Zoltan
>
> > @@ -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..025716b3ec37 100644
> > --- a/qapi/qmp-event.c
> > +++ b/qapi/qmp-event.c
> > @@ -20,15 +20,13 @@
> >
> > 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);
> > +    ts = qdict_from_jsonf_nofail("{ 'seconds': %" G_GINT64_FORMAT
> > +                                 ", 'microseconds': %" G_GINT64_FORMAT
> "}",
> > +                                 rt / G_USEC_PER_SEC,
> > +                                 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/qga/commands-posix.c b/qga/commands-posix.c
> > index 75dbaab68ea9..3311a4ca0167 100644
> > --- a/qga/commands-posix.c
> > +++ b/qga/commands-posix.c
> > @@ -138,16 +138,7 @@ void qmp_guest_shutdown(bool has_mode, const char
> *mode, Error **errp)
> >
> > 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;
> > +    return g_get_real_time();
> > }
> >
> > void qmp_guest_set_time(bool has_time, int64_t time_ns, Error **errp)
> > diff --git a/target/m68k/m68k-semi.c b/target/m68k/m68k-semi.c
> > index c5c164e096c8..54f735743165 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();
> > +            if (!(p = lock_user(VERIFY_WRITE,
> > +                                arg0, sizeof(struct gdb_timeval), 0))) {
> > +                /* 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..4600bf0a39d9 100644
> > --- a/target/nios2/nios2-semi.c
> > +++ b/target/nios2/nios2-semi.c
> > @@ -400,20 +400,18 @@ 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;
> >



-- 
Marc-André Lureau

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

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

[Richard Henderson]

On 3/4/22 05:27, marcandre.lureau@redhat.com wrote:
> +        g_clear_pointer(&timer, g_timer_destroy);
> +        timer = g_timer_new();

Why not g_timer_{reset,start}, instead of destroying and recreating?


r~

Re: [PATCH 4/4] oslib: drop qemu_gettimeofday()

[Richard Henderson]

On 3/4/22 05:27, marcandre.lureau@redhat.com wrote:
> 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>
> ---
>   include/sysemu/os-posix.h |  3 ---
>   include/sysemu/os-win32.h |  6 ------
>   util/oslib-win32.c        | 20 --------------------
>   3 files changed, 29 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

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

[Marc-André Lureau]

Hi Richard

On Sat, Mar 5, 2022 at 12:50 AM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 3/4/22 05:27, marcandre.lureau@redhat.com wrote:
> > +        g_clear_pointer(&timer, g_timer_destroy);
> > +        timer = g_timer_new();
>
> Why not g_timer_{reset,start}, instead of destroying and recreating?

Well, that didn't seem much easier, as that opens the question where
to create/destroy. And we would potentially have a "running" timer at
creation time, which could be confusing.

(btw, just found that doc : "This function is useless; it's fine to
call g_timer_start() on an already-started timer to reset the start
time, so g_timer_reset() serves no purpose" ;)

thanks