Re: [Qemu-devel] [PATCH v2] vhost-user-test: do not reinvent glib-compat.h

[Marc-André Lureau]

Reviewed-by: Marc-André Lureau <address@hidden>
Tested-by: Marc-André Lureau <address@hidden>


(It conflicts with my vhost-user series, but fixing it is quite
trivial, I pushed a rebased version on my devel tree
https://github.com/elmarco/qemu/tree/vhost-user)

On Tue, Sep 29, 2015 at 2:12 PM, Paolo Bonzini <address@hidden> wrote:
> glib-compat.h has the gunk to support both old-style and new-style
> gthread functions.  Use it instead of reinventing it.
>
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
>  tests/vhost-user-test.c | 113 
> +++++++-----------------------------------------
>  1 file changed, 16 insertions(+), 97 deletions(-)
>
> diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
> index e301db7..0e04f06 100644
> --- a/tests/vhost-user-test.c
> +++ b/tests/vhost-user-test.c
> @@ -8,7 +8,6 @@
>   *
>   */
>
> -#define QEMU_GLIB_COMPAT_H
>  #include <glib.h>
>
>  #include "libqtest.h"
> @@ -30,12 +29,6 @@
>  #define HAVE_MONOTONIC_TIME
>  #endif
>
> -#if GLIB_CHECK_VERSION(2, 32, 0)
> -#define HAVE_MUTEX_INIT
> -#define HAVE_COND_INIT
> -#define HAVE_THREAD_NEW
> -#endif
> -
>  #define QEMU_CMD_ACCEL  " -machine accel=tcg"
>  #define QEMU_CMD_MEM    " -m 512 -object 
> memory-backend-file,id=mem,size=512M,"\
>                          "mem-path=%s,share=on -numa node,memdev=mem"
> @@ -113,93 +106,21 @@ static VhostUserMsg m __attribute__ ((unused));
>
>  int fds_num = 0, fds[VHOST_MEMORY_MAX_NREGIONS];
>  static VhostUserMemory memory;
> -static GMutex *data_mutex;
> -static GCond *data_cond;
> -
> -static gint64 _get_time(void)
> -{
> -#ifdef HAVE_MONOTONIC_TIME
> -    return g_get_monotonic_time();
> -#else
> -    GTimeVal time;
> -    g_get_current_time(&time);
> -
> -    return time.tv_sec * G_TIME_SPAN_SECOND + time.tv_usec;
> -#endif
> -}
> -
> -static GMutex *_mutex_new(void)
> -{
> -    GMutex *mutex;
> -
> -#ifdef HAVE_MUTEX_INIT
> -    mutex = g_new(GMutex, 1);
> -    g_mutex_init(mutex);
> -#else
> -    mutex = g_mutex_new();
> -#endif
> -
> -    return mutex;
> -}
> +static CompatGMutex data_mutex;
> +static CompatGCond data_cond;
>
> -static void _mutex_free(GMutex *mutex)
> -{
> -#ifdef HAVE_MUTEX_INIT
> -    g_mutex_clear(mutex);
> -    g_free(mutex);
> -#else
> -    g_mutex_free(mutex);
> -#endif
> -}
> -
> -static GCond *_cond_new(void)
> -{
> -    GCond *cond;
> -
> -#ifdef HAVE_COND_INIT
> -    cond = g_new(GCond, 1);
> -    g_cond_init(cond);
> -#else
> -    cond = g_cond_new();
> -#endif
> -
> -    return cond;
> -}
> -
> -static gboolean _cond_wait_until(GCond *cond, GMutex *mutex, gint64 end_time)
> +#if !GLIB_CHECK_VERSION(2, 32, 0)
> +static gboolean g_cond_wait_until(CompatGCond cond, CompatGMutex mutex,
> +                                  gint64 end_time)
>  {
>      gboolean ret = FALSE;
> -#ifdef HAVE_COND_INIT
> -    ret = g_cond_wait_until(cond, mutex, end_time);
> -#else
> +    end_time -= g_get_monotonic_time();
>      GTimeVal time = { end_time / G_TIME_SPAN_SECOND,
>                        end_time % G_TIME_SPAN_SECOND };
>      ret = g_cond_timed_wait(cond, mutex, &time);
> -#endif
>      return ret;
>  }
> -
> -static void _cond_free(GCond *cond)
> -{
> -#ifdef HAVE_COND_INIT
> -    g_cond_clear(cond);
> -    g_free(cond);
> -#else
> -    g_cond_free(cond);
>  #endif
> -}
> -
> -static GThread *_thread_new(const gchar *name, GThreadFunc func, gpointer 
> data)
> -{
> -    GThread *thread = NULL;
> -    GError *error = NULL;
> -#ifdef HAVE_THREAD_NEW
> -    thread = g_thread_try_new(name, func, data, &error);
> -#else
> -    thread = g_thread_create(func, data, TRUE, &error);
> -#endif
> -    return thread;
> -}
>
>  static void read_guest_mem(void)
>  {
> @@ -208,11 +129,11 @@ static void read_guest_mem(void)
>      int i, j;
>      size_t size;
>
> -    g_mutex_lock(data_mutex);
> +    g_mutex_lock(&data_mutex);
>
> -    end_time = _get_time() + 5 * G_TIME_SPAN_SECOND;
> +    end_time = g_get_monotonic_time() + 5 * G_TIME_SPAN_SECOND;
>      while (!fds_num) {
> -        if (!_cond_wait_until(data_cond, data_mutex, end_time)) {
> +        if (!g_cond_wait_until(&data_cond, &data_mutex, end_time)) {
>              /* timeout has passed */
>              g_assert(fds_num);
>              break;
> @@ -252,7 +173,7 @@ static void read_guest_mem(void)
>      }
>
>      g_assert_cmpint(1, ==, 1);
> -    g_mutex_unlock(data_mutex);
> +    g_mutex_unlock(&data_mutex);
>  }
>
>  static void *thread_function(void *data)
> @@ -280,7 +201,7 @@ static void chr_read(void *opaque, const uint8_t *buf, 
> int size)
>          return;
>      }
>
> -    g_mutex_lock(data_mutex);
> +    g_mutex_lock(&data_mutex);
>      memcpy(p, buf, VHOST_USER_HDR_SIZE);
>
>      if (msg.size) {
> @@ -313,7 +234,7 @@ static void chr_read(void *opaque, const uint8_t *buf, 
> int size)
>          fds_num = qemu_chr_fe_get_msgfds(chr, fds, sizeof(fds) / 
> sizeof(int));
>
>          /* signal the test that it can continue */
> -        g_cond_signal(data_cond);
> +        g_cond_signal(&data_cond);
>          break;
>
>      case VHOST_USER_SET_VRING_KICK:
> @@ -330,7 +251,7 @@ static void chr_read(void *opaque, const uint8_t *buf, 
> int size)
>      default:
>          break;
>      }
> -    g_mutex_unlock(data_mutex);
> +    g_mutex_unlock(&data_mutex);
>  }
>
>  static const char *init_hugepagefs(void)
> @@ -395,9 +316,9 @@ int main(int argc, char **argv)
>      qemu_chr_add_handlers(chr, chr_can_read, chr_read, NULL, chr);
>
>      /* run the main loop thread so the chardev may operate */
> -    data_mutex = _mutex_new();
> -    data_cond = _cond_new();
> -    _thread_new(NULL, thread_function, NULL);
> +    g_mutex_init(&data_mutex);
> +    g_cond_init(&data_cond);
> +    g_thread_new(NULL, thread_function, NULL);
>
>      qemu_cmd = g_strdup_printf(QEMU_CMD, hugefs, socket_path);
>      s = qtest_start(qemu_cmd);
> @@ -414,8 +335,6 @@ int main(int argc, char **argv)
>      /* cleanup */
>      unlink(socket_path);
>      g_free(socket_path);
> -    _cond_free(data_cond);
> -    _mutex_free(data_mutex);
>
>      return ret;
>  }
> --
> 2.5.0
>
>



-- 
Marc-André Lureau