[PATCH] Remove support for CLOCK_MONOTONIC not being defined

[PATCH] Remove support for CLOCK_MONOTONIC not being defined

[Peter Maydell]

Some older parts of QEMU's codebase assume that CLOCK_MONOTONIC
might not be defined by the host OS, and have workarounds to
deal with this. However, more recently (notably in commit
50290c002c045280f8d for qemu-img in mid-2019, but also much
earlier in 2011 in commit 22795174a37e0 for ui/spice-display.c)
we've written code that assumes CLOCK_MONOTONIC is always
defined. The only host OS anybody's ever noticed this on
is OSX 10.11 and earlier, which we don't support.

So we can assume that all our host OSes have the #define,
and we can remove some now-unnecessary ifdefs.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
I've left in the runtime-check in init_get_clock(), as it's
less easy to see if the fallback there is ever used.
---
 include/qemu/timer.h     |  5 +----
 util/qemu-timer-common.c | 11 ++++-------
 2 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/include/qemu/timer.h b/include/qemu/timer.h
index 85bc6eb00b2..6a8b48b5a9d 100644
--- a/include/qemu/timer.h
+++ b/include/qemu/timer.h
@@ -838,14 +838,11 @@ extern int use_rt_clock;
 
 static inline int64_t get_clock(void)
 {
-#ifdef CLOCK_MONOTONIC
     if (use_rt_clock) {
         struct timespec ts;
         clock_gettime(CLOCK_MONOTONIC, &ts);
         return ts.tv_sec * 1000000000LL + ts.tv_nsec;
-    } else
-#endif
-    {
+    } else {
         /* XXX: using gettimeofday leads to problems if the date
            changes, so it should be avoided. */
         return get_clock_realtime();
diff --git a/util/qemu-timer-common.c b/util/qemu-timer-common.c
index 06d084d3646..baf3317f745 100644
--- a/util/qemu-timer-common.c
+++ b/util/qemu-timer-common.c
@@ -49,14 +49,11 @@ int use_rt_clock;
 
 static void __attribute__((constructor)) init_get_clock(void)
 {
+    struct timespec ts;
+
     use_rt_clock = 0;
-#ifdef CLOCK_MONOTONIC
-    {
-        struct timespec ts;
-        if (clock_gettime(CLOCK_MONOTONIC, &ts) == 0) {
-            use_rt_clock = 1;
-        }
+    if (clock_gettime(CLOCK_MONOTONIC, &ts) == 0) {
+        use_rt_clock = 1;
     }
-#endif
 }
 #endif
-- 
2.20.1

Re: [PATCH] Remove support for CLOCK_MONOTONIC not being defined

[Paolo Bonzini]

On 01/02/20 18:22, Peter Maydell wrote:
> Some older parts of QEMU's codebase assume that CLOCK_MONOTONIC
> might not be defined by the host OS, and have workarounds to
> deal with this. However, more recently (notably in commit
> 50290c002c045280f8d for qemu-img in mid-2019, but also much
> earlier in 2011 in commit 22795174a37e0 for ui/spice-display.c)
> we've written code that assumes CLOCK_MONOTONIC is always
> defined. The only host OS anybody's ever noticed this on
> is OSX 10.11 and earlier, which we don't support.
> 
> So we can assume that all our host OSes have the #define,
> and we can remove some now-unnecessary ifdefs.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> I've left in the runtime-check in init_get_clock(), as it's
> less easy to see if the fallback there is ever used.
> ---
>  include/qemu/timer.h     |  5 +----
>  util/qemu-timer-common.c | 11 ++++-------
>  2 files changed, 5 insertions(+), 11 deletions(-)
> 
> diff --git a/include/qemu/timer.h b/include/qemu/timer.h
> index 85bc6eb00b2..6a8b48b5a9d 100644
> --- a/include/qemu/timer.h
> +++ b/include/qemu/timer.h
> @@ -838,14 +838,11 @@ extern int use_rt_clock;
>  
>  static inline int64_t get_clock(void)
>  {
> -#ifdef CLOCK_MONOTONIC
>      if (use_rt_clock) {
>          struct timespec ts;
>          clock_gettime(CLOCK_MONOTONIC, &ts);
>          return ts.tv_sec * 1000000000LL + ts.tv_nsec;
> -    } else
> -#endif
> -    {
> +    } else {
>          /* XXX: using gettimeofday leads to problems if the date
>             changes, so it should be avoided. */
>          return get_clock_realtime();
> diff --git a/util/qemu-timer-common.c b/util/qemu-timer-common.c
> index 06d084d3646..baf3317f745 100644
> --- a/util/qemu-timer-common.c
> +++ b/util/qemu-timer-common.c
> @@ -49,14 +49,11 @@ int use_rt_clock;
>  
>  static void __attribute__((constructor)) init_get_clock(void)
>  {
> +    struct timespec ts;
> +
>      use_rt_clock = 0;
> -#ifdef CLOCK_MONOTONIC
> -    {
> -        struct timespec ts;
> -        if (clock_gettime(CLOCK_MONOTONIC, &ts) == 0) {
> -            use_rt_clock = 1;
> -        }
> +    if (clock_gettime(CLOCK_MONOTONIC, &ts) == 0) {
> +        use_rt_clock = 1;
>      }
> -#endif
>  }
>  #endif
> 

Queued, thanks.

Paolo

Re: [PATCH] Remove support for CLOCK_MONOTONIC not being defined

[Peter Maydell]

On Sat, 1 Feb 2020 at 17:22, Peter Maydell <peter.maydell@linaro.org> wrote:
> I've left in the runtime-check in init_get_clock(), as it's
> less easy to see if the fallback there is ever used.

That code was first added in 1dce7c3c2244 in 2006, so
it seems at least plausible that we could remove the
runtime-check too at some point. Somebody would need
to go through all our supported hosts and check that
CLOCK_MONOTONIC really did always work rather
than returning 0...

thanks
-- PMM