[Qemu-devel] [PATCH v2 0/3] glib: update the min required version

[Qemu-devel] [PATCH v2 0/3] glib: update the min required version

[Daniel P. Berrangé]

The previous patch to bump glib to 2.42 hit problems with Peter's build
environment for testing merge:

  https://lists.gnu.org/archive/html/qemu-devel/2018-05/msg02557.html

This posting drops back to 2.40, which allows Ubuntu 14.04 from GLibC
compile farm to be supported.

It does NOT try to go back to 2.34, because it is hoped that the mxe.cc
Debian packages will be suitable for Peter to test Windows
cross-compile. Alternatively the docker environments provided in tree
can be used for mingw build testing on any host able to run docker.

I also dropped some more GLIB_CHECK_VERSION checks that are redundant
given the new min version.

Daniel P. Berrangé (3):
  glib: bump min required glib library version to 2.40
  glib: enforce the minimum required version and warn about old APIs
  util: remove redundant include of glib.h

 configure                                |   6 +-
 crypto/hash-glib.c                       |   4 -
 crypto/hmac-glib.c                       |  36 ---
 include/glib-compat.h                    | 372 ++++-------------------
 qga/commands.c                           |  11 +-
 tests/docker/dockerfiles/centos6.docker  |  30 --
 tests/docker/dockerfiles/min-glib.docker |   8 -
 tests/ivshmem-test.c                     |   6 -
 tests/test-qga.c                         |   2 -
 tests/test-qmp-event.c                   |   8 +-
 tests/tpm-emu.h                          |   4 +-
 tests/vhost-user-test.c                  |  26 +-
 trace/simple.c                           |   6 +-
 util/iova-tree.c                         |   1 -
 util/osdep.c                             |  14 -
 15 files changed, 71 insertions(+), 463 deletions(-)
 delete mode 100644 tests/docker/dockerfiles/centos6.docker
 delete mode 100644 tests/docker/dockerfiles/min-glib.docker

-- 
2.17.0

[Qemu-devel] [PATCH v2 1/3] glib: bump min required glib library version to 2.40

[Daniel P. Berrangé]

Per supported platforms doc, the various min glib on relevant distros is:

  RHEL-7: 2.50.3
  Debian (Stretch): 2.50.3
  Debian (Jessie): 2.42.1
  OpenBSD (Ports): 2.54.3
  FreeBSD (Ports): 2.50.3
  OpenSUSE Leap 15: 2.54.3
  Ubuntu (Xenial): 2.48.0
  macOS (Homebrew): 2.56.0

This suggests that a minimum glib of 2.42 is a reasonable target.

The GLibC compile farm, however, uses Ubuntu 14.04 (Trusty) which only
has glib 2.40.0, and this is needed for testing during merge. Thus an
exception is made to the documented platform support policy to allow for
all three current LTS releases to be supported.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 configure               |   6 +-
 crypto/hash-glib.c      |   4 -
 crypto/hmac-glib.c      |  36 -----
 include/glib-compat.h   | 319 ----------------------------------------
 qga/commands.c          |  11 +-
 tests/ivshmem-test.c    |   6 -
 tests/test-qmp-event.c  |   8 +-
 tests/tpm-emu.h         |   4 +-
 tests/vhost-user-test.c |  26 +---
 trace/simple.c          |   6 +-
 util/osdep.c            |  14 --
 11 files changed, 11 insertions(+), 429 deletions(-)

diff --git a/configure b/configure
index 14b11130a7..3f001dc849 100755
--- a/configure
+++ b/configure
@@ -3387,11 +3387,7 @@ fi
 ##########################################
 # glib support probe
 
-if test "$mingw32" = yes; then
-    glib_req_ver=2.30
-else
-    glib_req_ver=2.22
-fi
+glib_req_ver=2.40
 glib_modules=gthread-2.0
 if test "$modules" = yes; then
     glib_modules="$glib_modules gmodule-export-2.0"
diff --git a/crypto/hash-glib.c b/crypto/hash-glib.c
index a5871cc72f..a0096c7c47 100644
--- a/crypto/hash-glib.c
+++ b/crypto/hash-glib.c
@@ -30,11 +30,7 @@ static int qcrypto_hash_alg_map[QCRYPTO_HASH_ALG__MAX] = {
     [QCRYPTO_HASH_ALG_SHA224] = -1,
     [QCRYPTO_HASH_ALG_SHA256] = G_CHECKSUM_SHA256,
     [QCRYPTO_HASH_ALG_SHA384] = -1,
-#if GLIB_CHECK_VERSION(2, 36, 0)
     [QCRYPTO_HASH_ALG_SHA512] = G_CHECKSUM_SHA512,
-#else
-    [QCRYPTO_HASH_ALG_SHA512] = -1,
-#endif
     [QCRYPTO_HASH_ALG_RIPEMD160] = -1,
 };
 
diff --git a/crypto/hmac-glib.c b/crypto/hmac-glib.c
index a6c1730291..7df627329d 100644
--- a/crypto/hmac-glib.c
+++ b/crypto/hmac-glib.c
@@ -17,9 +17,6 @@
 #include "crypto/hmac.h"
 #include "hmacpriv.h"
 
-/* Support for HMAC Algos has been added in GLib 2.30 */
-#if GLIB_CHECK_VERSION(2, 30, 0)
-
 static int qcrypto_hmac_alg_map[QCRYPTO_HASH_ALG__MAX] = {
     [QCRYPTO_HASH_ALG_MD5] = G_CHECKSUM_MD5,
     [QCRYPTO_HASH_ALG_SHA1] = G_CHECKSUM_SHA1,
@@ -126,39 +123,6 @@ qcrypto_glib_hmac_bytesv(QCryptoHmac *hmac,
     return 0;
 }
 
-#else
-
-bool qcrypto_hmac_supports(QCryptoHashAlgorithm alg)
-{
-    return false;
-}
-
-void *qcrypto_hmac_ctx_new(QCryptoHashAlgorithm alg,
-                           const uint8_t *key, size_t nkey,
-                           Error **errp)
-{
-    return NULL;
-}
-
-static void
-qcrypto_glib_hmac_ctx_free(QCryptoHmac *hmac)
-{
-    return;
-}
-
-static int
-qcrypto_glib_hmac_bytesv(QCryptoHmac *hmac,
-                         const struct iovec *iov,
-                         size_t niov,
-                         uint8_t **result,
-                         size_t *resultlen,
-                         Error **errp)
-{
-    return -1;
-}
-
-#endif
-
 QCryptoHmacDriver qcrypto_hmac_lib_driver = {
     .hmac_bytesv = qcrypto_glib_hmac_bytesv,
     .hmac_free = qcrypto_glib_hmac_ctx_free,
diff --git a/include/glib-compat.h b/include/glib-compat.h
index c49cf87196..3b340ab33c 100644
--- a/include/glib-compat.h
+++ b/include/glib-compat.h
@@ -18,27 +18,6 @@
 
 #include <glib.h>
 
-/* GLIB version compatibility flags */
-#if !GLIB_CHECK_VERSION(2, 26, 0)
-#define G_TIME_SPAN_SECOND              (G_GINT64_CONSTANT(1000000))
-#endif
-
-#if !GLIB_CHECK_VERSION(2, 28, 0)
-static inline gint64 qemu_g_get_monotonic_time(void)
-{
-    /* g_get_monotonic_time() is best-effort so we can use the wall clock as a
-     * fallback.
-     */
-
-    GTimeVal time;
-    g_get_current_time(&time);
-
-    return time.tv_sec * G_TIME_SPAN_SECOND + time.tv_usec;
-}
-/* work around distro backports of this interface */
-#define g_get_monotonic_time() qemu_g_get_monotonic_time()
-#endif
-
 #if defined(_WIN32) && !GLIB_CHECK_VERSION(2, 50, 0)
 /*
  * g_poll has a problem on Windows when using
@@ -48,228 +27,6 @@ static inline gint64 qemu_g_get_monotonic_time(void)
 gint g_poll_fixed(GPollFD *fds, guint nfds, gint timeout);
 #endif
 
-#if !GLIB_CHECK_VERSION(2, 30, 0)
-/* Not a 100% compatible implementation, but good enough for most
- * cases. Placeholders are only supported at the end of the
- * template. */
-static inline gchar *qemu_g_dir_make_tmp(gchar const *tmpl, GError **error)
-{
-    gchar *path = g_build_filename(g_get_tmp_dir(), tmpl ?: ".XXXXXX", NULL);
-
-    if (mkdtemp(path) != NULL) {
-        return path;
-    }
-    /* Error occurred, clean up. */
-    g_set_error(error, G_FILE_ERROR, g_file_error_from_errno(errno),
-                "mkdtemp() failed");
-    g_free(path);
-    return NULL;
-}
-#define g_dir_make_tmp(tmpl, error) qemu_g_dir_make_tmp(tmpl, error)
-#endif /* glib 2.30 */
-
-#if !GLIB_CHECK_VERSION(2, 31, 0)
-/* before glib-2.31, GMutex and GCond was dynamic-only (there was a separate
- * GStaticMutex, but it didn't work with condition variables).
- *
- * Our implementation uses GOnce to fake a static implementation that does
- * not require separate initialization.
- * We need to rename the types to avoid passing our CompatGMutex/CompatGCond
- * by mistake to a function that expects GMutex/GCond.  However, for ease
- * of use we keep the GLib function names.  GLib uses macros for the
- * implementation, we use inline functions instead and undefine the macros.
- */
-
-typedef struct CompatGMutex {
-    GOnce once;
-} CompatGMutex;
-
-typedef struct CompatGCond {
-    GOnce once;
-} CompatGCond;
-
-static inline gpointer do_g_mutex_new(gpointer unused)
-{
-    return (gpointer) g_mutex_new();
-}
-
-static inline void g_mutex_init(CompatGMutex *mutex)
-{
-    mutex->once = (GOnce) G_ONCE_INIT;
-}
-
-static inline void g_mutex_clear(CompatGMutex *mutex)
-{
-    g_assert(mutex->once.status != G_ONCE_STATUS_PROGRESS);
-    if (mutex->once.retval) {
-        g_mutex_free((GMutex *) mutex->once.retval);
-    }
-    mutex->once = (GOnce) G_ONCE_INIT;
-}
-
-static inline void (g_mutex_lock)(CompatGMutex *mutex)
-{
-    g_once(&mutex->once, do_g_mutex_new, NULL);
-    g_mutex_lock((GMutex *) mutex->once.retval);
-}
-#undef g_mutex_lock
-
-static inline gboolean (g_mutex_trylock)(CompatGMutex *mutex)
-{
-    g_once(&mutex->once, do_g_mutex_new, NULL);
-    return g_mutex_trylock((GMutex *) mutex->once.retval);
-}
-#undef g_mutex_trylock
-
-
-static inline void (g_mutex_unlock)(CompatGMutex *mutex)
-{
-    g_mutex_unlock((GMutex *) mutex->once.retval);
-}
-#undef g_mutex_unlock
-
-static inline gpointer do_g_cond_new(gpointer unused)
-{
-    return (gpointer) g_cond_new();
-}
-
-static inline void g_cond_init(CompatGCond *cond)
-{
-    cond->once = (GOnce) G_ONCE_INIT;
-}
-
-static inline void g_cond_clear(CompatGCond *cond)
-{
-    g_assert(cond->once.status != G_ONCE_STATUS_PROGRESS);
-    if (cond->once.retval) {
-        g_cond_free((GCond *) cond->once.retval);
-    }
-    cond->once = (GOnce) G_ONCE_INIT;
-}
-
-static inline void (g_cond_wait)(CompatGCond *cond, CompatGMutex *mutex)
-{
-    g_assert(mutex->once.status != G_ONCE_STATUS_PROGRESS);
-    g_once(&cond->once, do_g_cond_new, NULL);
-    g_cond_wait((GCond *) cond->once.retval, (GMutex *) mutex->once.retval);
-}
-#undef g_cond_wait
-
-static inline void (g_cond_broadcast)(CompatGCond *cond)
-{
-    g_once(&cond->once, do_g_cond_new, NULL);
-    g_cond_broadcast((GCond *) cond->once.retval);
-}
-#undef g_cond_broadcast
-
-static inline void (g_cond_signal)(CompatGCond *cond)
-{
-    g_once(&cond->once, do_g_cond_new, NULL);
-    g_cond_signal((GCond *) cond->once.retval);
-}
-#undef g_cond_signal
-
-static inline gboolean (g_cond_timed_wait)(CompatGCond *cond,
-                                           CompatGMutex *mutex,
-                                           GTimeVal *time)
-{
-    g_assert(mutex->once.status != G_ONCE_STATUS_PROGRESS);
-    g_once(&cond->once, do_g_cond_new, NULL);
-    return g_cond_timed_wait((GCond *) cond->once.retval,
-                             (GMutex *) mutex->once.retval, time);
-}
-#undef g_cond_timed_wait
-
-/* This is not a macro, because it didn't exist until 2.32.  */
-static inline gboolean g_cond_wait_until(CompatGCond *cond, CompatGMutex *mutex,
-                                         gint64 end_time)
-{
-    GTimeVal time;
-
-    /* Convert from monotonic to CLOCK_REALTIME.  */
-    end_time -= g_get_monotonic_time();
-    g_get_current_time(&time);
-    end_time += time.tv_sec * G_TIME_SPAN_SECOND + time.tv_usec;
-
-    time.tv_sec = end_time / G_TIME_SPAN_SECOND;
-    time.tv_usec = end_time % G_TIME_SPAN_SECOND;
-    return g_cond_timed_wait(cond, mutex, &time);
-}
-
-/* before 2.31 there was no g_thread_new() */
-static inline GThread *g_thread_new(const char *name,
-                                    GThreadFunc func, gpointer data)
-{
-    GThread *thread = g_thread_create(func, data, TRUE, NULL);
-    if (!thread) {
-        g_error("creating thread");
-    }
-    return thread;
-}
-#else
-#define CompatGMutex GMutex
-#define CompatGCond GCond
-#endif /* glib 2.31 */
-
-#if !GLIB_CHECK_VERSION(2, 32, 0)
-/* Beware, function returns gboolean since 2.39.2, see GLib commit 9101915 */
-static inline void g_hash_table_add(GHashTable *hash_table, gpointer key)
-{
-    g_hash_table_replace(hash_table, key, key);
-}
-
-static inline gboolean g_hash_table_contains(GHashTable *hash_table,
-                                             gpointer key)
-{
-    return g_hash_table_lookup_extended(hash_table, key, NULL, NULL);
-}
-#define G_SOURCE_CONTINUE TRUE
-#define G_SOURCE_REMOVE FALSE
-#endif
-
-#ifndef g_assert_true
-#define g_assert_true(expr)                                                    \
-    do {                                                                       \
-        if (G_LIKELY(expr)) {                                                  \
-        } else {                                                               \
-            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
-                                "'" #expr "' should be TRUE");                 \
-        }                                                                      \
-    } while (0)
-#endif
-
-#ifndef g_assert_false
-#define g_assert_false(expr)                                                   \
-    do {                                                                       \
-        if (G_LIKELY(!(expr))) {                                               \
-        } else {                                                               \
-            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
-                                "'" #expr "' should be FALSE");                \
-        }                                                                      \
-    } while (0)
-#endif
-
-#ifndef g_assert_null
-#define g_assert_null(expr)                                                    \
-    do {                                                                       \
-        if (G_LIKELY((expr) == NULL)) {                                        \
-        } else {                                                               \
-            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
-                                "'" #expr "' should be NULL");                 \
-        }                                                                      \
-    } while (0)
-#endif
-
-#ifndef g_assert_nonnull
-#define g_assert_nonnull(expr)                                                 \
-    do {                                                                       \
-        if (G_LIKELY((expr) != NULL)) {                                        \
-        } else {                                                               \
-            g_assertion_message(G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC,   \
-                                "'" #expr "' should not be NULL");             \
-        }                                                                      \
-    } while (0)
-#endif
 
 #ifndef g_assert_cmpmem
 #define g_assert_cmpmem(m1, l1, m2, l2)                                        \
@@ -288,80 +45,4 @@ static inline gboolean g_hash_table_contains(GHashTable *hash_table,
     } while (0)
 #endif
 
-#if !GLIB_CHECK_VERSION(2, 28, 0)
-static inline void g_list_free_full(GList *list, GDestroyNotify free_func)
-{
-    GList *l;
-
-    for (l = list; l; l = l->next) {
-        free_func(l->data);
-    }
-
-    g_list_free(list);
-}
-
-static inline void g_slist_free_full(GSList *list, GDestroyNotify free_func)
-{
-    GSList *l;
-
-    for (l = list; l; l = l->next) {
-        free_func(l->data);
-    }
-
-    g_slist_free(list);
-}
-#endif
-
-#if !GLIB_CHECK_VERSION(2, 26, 0)
-static inline void g_source_set_name(GSource *source, const char *name)
-{
-    /* This is just a debugging aid, so leaving it a no-op */
-}
-static inline void g_source_set_name_by_id(guint tag, const char *name)
-{
-    /* This is just a debugging aid, so leaving it a no-op */
-}
-#endif
-
-#if !GLIB_CHECK_VERSION(2, 36, 0)
-/* Always fail.  This will not include error_report output in the test log,
- * sending it instead to stderr.
- */
-#define g_test_initialized() (0)
-#endif
-#if !GLIB_CHECK_VERSION(2, 38, 0)
-#ifdef CONFIG_HAS_GLIB_SUBPROCESS_TESTS
-#error schizophrenic detection of glib subprocess testing
-#endif
-#define g_test_subprocess() (0)
-#endif
-
-
-#if !GLIB_CHECK_VERSION(2, 34, 0)
-static inline void
-g_test_add_data_func_full(const char *path,
-                          gpointer data,
-                          gpointer fn,
-                          gpointer data_free_func)
-{
-#if GLIB_CHECK_VERSION(2, 26, 0)
-    /* back-compat casts, remove this once we can require new-enough glib */
-    g_test_add_vtable(path, 0, data, NULL,
-                      (GTestFixtureFunc)fn, (GTestFixtureFunc) data_free_func);
-#else
-    /* back-compat casts, remove this once we can require new-enough glib */
-    g_test_add_vtable(path, 0, data, NULL,
-                      (void (*)(void)) fn, (void (*)(void)) data_free_func);
-#endif
-}
-#endif
-
-/* Small compat shim from glib 2.32 */
-#ifndef G_SOURCE_CONTINUE
-#define G_SOURCE_CONTINUE TRUE
-#endif
-#ifndef G_SOURCE_REMOVE
-#define G_SOURCE_REMOVE FALSE
-#endif
-
 #endif
diff --git a/qga/commands.c b/qga/commands.c
index cce3010f0f..0c7d1385c2 100644
--- a/qga/commands.c
+++ b/qga/commands.c
@@ -414,10 +414,8 @@ GuestExec *qmp_guest_exec(const char *path,
     argv = guest_exec_get_args(&arglist, true);
     envp = has_env ? guest_exec_get_args(env, false) : NULL;
 
-    flags = G_SPAWN_SEARCH_PATH | G_SPAWN_DO_NOT_REAP_CHILD;
-#if GLIB_CHECK_VERSION(2, 33, 2)
-    flags |= G_SPAWN_SEARCH_PATH_FROM_ENVP;
-#endif
+    flags = G_SPAWN_SEARCH_PATH | G_SPAWN_DO_NOT_REAP_CHILD |
+        G_SPAWN_SEARCH_PATH_FROM_ENVP;
     if (!has_output) {
         flags |= G_SPAWN_STDOUT_TO_DEV_NULL | G_SPAWN_STDERR_TO_DEV_NULL;
     }
@@ -514,7 +512,6 @@ GuestHostName *qmp_guest_get_host_name(Error **err)
 
 GuestTimezone *qmp_guest_get_timezone(Error **errp)
 {
-#if GLIB_CHECK_VERSION(2, 28, 0)
     GuestTimezone *info = NULL;
     GTimeZone *tz = NULL;
     gint64 now = 0;
@@ -544,8 +541,4 @@ GuestTimezone *qmp_guest_get_timezone(Error **errp)
 error:
     g_free(info);
     return NULL;
-#else
-    error_setg(errp, QERR_UNSUPPORTED);
-    return NULL;
-#endif
 }
diff --git a/tests/ivshmem-test.c b/tests/ivshmem-test.c
index 8af16ee79a..9b407a3e42 100644
--- a/tests/ivshmem-test.c
+++ b/tests/ivshmem-test.c
@@ -504,12 +504,6 @@ int main(int argc, char **argv)
     const char *arch = qtest_get_arch();
     gchar dir[] = "/tmp/ivshmem-test.XXXXXX";
 
-#if !GLIB_CHECK_VERSION(2, 31, 0)
-    if (!g_thread_supported()) {
-        g_thread_init(NULL);
-    }
-#endif
-
     g_test_init(&argc, &argv, NULL);
 
     qtest_add_abrt_handler(abrt_handler, NULL);
diff --git a/tests/test-qmp-event.c b/tests/test-qmp-event.c
index 3a7c227a1d..8677094ad1 100644
--- a/tests/test-qmp-event.c
+++ b/tests/test-qmp-event.c
@@ -32,7 +32,7 @@ typedef struct QDictCmpData {
 } QDictCmpData;
 
 TestEventData *test_event_data;
-static CompatGMutex test_event_lock;
+static GMutex test_event_lock;
 
 /* Only compares bool, int, string */
 static
@@ -242,12 +242,6 @@ static void test_event_d(TestEventData *data,
 
 int main(int argc, char **argv)
 {
-#if !GLIB_CHECK_VERSION(2, 31, 0)
-    if (!g_thread_supported()) {
-       g_thread_init(NULL);
-    }
-#endif
-
     qmp_event_set_func_emit(event_test_emit);
 
     g_test_init(&argc, &argv, NULL);
diff --git a/tests/tpm-emu.h b/tests/tpm-emu.h
index ef4bfa8800..08f902485e 100644
--- a/tests/tpm-emu.h
+++ b/tests/tpm-emu.h
@@ -24,8 +24,8 @@ struct tpm_hdr {
 } QEMU_PACKED;
 
 typedef struct TestState {
-    CompatGMutex data_mutex;
-    CompatGCond data_cond;
+    GMutex data_mutex;
+    GCond data_cond;
     SocketAddress *addr;
     QIOChannel *tpm_ioc;
     GThread *emu_tpm_thread;
diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
index bbc8091286..8ff2106d32 100644
--- a/tests/vhost-user-test.c
+++ b/tests/vhost-user-test.c
@@ -32,14 +32,6 @@
 #include <linux/virtio_net.h>
 #include <sys/vfs.h>
 
-/* GLIB version compatibility flags */
-#if !GLIB_CHECK_VERSION(2, 26, 0)
-#define G_TIME_SPAN_SECOND              (G_GINT64_CONSTANT(1000000))
-#endif
-
-#if GLIB_CHECK_VERSION(2, 28, 0)
-#define HAVE_MONOTONIC_TIME
-#endif
 
 #define QEMU_CMD_MEM    " -m %d -object memory-backend-file,id=mem,size=%dM," \
                         "mem-path=%s,share=on -numa node,memdev=mem"
@@ -150,8 +142,8 @@ typedef struct TestServer {
     int fds_num;
     int fds[VHOST_MEMORY_MAX_NREGIONS];
     VhostUserMemory memory;
-    CompatGMutex data_mutex;
-    CompatGCond data_cond;
+    GMutex data_mutex;
+    GCond data_cond;
     int log_fd;
     uint64_t rings;
     bool test_fail;
@@ -642,21 +634,7 @@ test_migrate_source_check(GSource *source)
     return FALSE;
 }
 
-#if !GLIB_CHECK_VERSION(2,36,0)
-/* this callback is unnecessary with glib >2.36, the default
- * prepare for the source does the same */
-static gboolean
-test_migrate_source_prepare(GSource *source, gint *timeout)
-{
-    *timeout = -1;
-    return FALSE;
-}
-#endif
-
 GSourceFuncs test_migrate_source_funcs = {
-#if !GLIB_CHECK_VERSION(2,36,0)
-    .prepare = test_migrate_source_prepare,
-#endif
     .check = test_migrate_source_check,
 };
 
diff --git a/trace/simple.c b/trace/simple.c
index e82018d923..701dec639c 100644
--- a/trace/simple.c
+++ b/trace/simple.c
@@ -36,9 +36,9 @@
  * Trace records are written out by a dedicated thread.  The thread waits for
  * records to become available, writes them out, and then waits again.
  */
-static CompatGMutex trace_lock;
-static CompatGCond trace_available_cond;
-static CompatGCond trace_empty_cond;
+static GMutex trace_lock;
+static GCond trace_available_cond;
+static GCond trace_empty_cond;
 
 static bool trace_available;
 static bool trace_writeout_enabled;
diff --git a/util/osdep.c b/util/osdep.c
index a73de0e1ba..8bfa022673 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -503,20 +503,6 @@ int socket_init(void)
     return 0;
 }
 
-#if !GLIB_CHECK_VERSION(2, 31, 0)
-/* Ensure that glib is running in multi-threaded mode
- * Old versions of glib require explicit initialization.  Failure to do
- * this results in the single-threaded code paths being taken inside
- * glib.  For example, the g_slice allocator will not be thread-safe
- * and cause crashes.
- */
-static void __attribute__((constructor)) thread_init(void)
-{
-    if (!g_thread_supported()) {
-       g_thread_init(NULL);
-    }
-}
-#endif
 
 #ifndef CONFIG_IOVEC
 /* helper function for iov_send_recv() */
-- 
2.17.0

Re: [Qemu-devel] [PATCH v2 1/3] glib: bump min required glib library version to 2.40

[John Snow]

On 06/06/2018 01:32 PM, Daniel P. Berrangé wrote:
> Per supported platforms doc, the various min glib on relevant distros is:

Which doc?

> 
>   RHEL-7: 2.50.3
>   Debian (Stretch): 2.50.3
>   Debian (Jessie): 2.42.1
>   OpenBSD (Ports): 2.54.3
>   FreeBSD (Ports): 2.50.3
>   OpenSUSE Leap 15: 2.54.3
>   Ubuntu (Xenial): 2.48.0
>   macOS (Homebrew): 2.56.0
> 
> This suggests that a minimum glib of 2.42 is a reasonable target.

Looks like useful info to check in somewhere -- though I suppose the
commit message counts.

--js

Re: [Qemu-devel] [PATCH v2 1/3] glib: bump min required glib library version to 2.40

[Daniel P. Berrangé]

On Wed, Jun 06, 2018 at 02:12:33PM -0400, John Snow wrote:
> 
> 
> On 06/06/2018 01:32 PM, Daniel P. Berrangé wrote:
> > Per supported platforms doc, the various min glib on relevant distros is:
> 
> Which doc?

The main QEMU doc 

https://qemu.weilnetz.de/doc/qemu-doc.html#Supported-build-platforms

> 
> > 
> >   RHEL-7: 2.50.3
> >   Debian (Stretch): 2.50.3
> >   Debian (Jessie): 2.42.1
> >   OpenBSD (Ports): 2.54.3
> >   FreeBSD (Ports): 2.50.3
> >   OpenSUSE Leap 15: 2.54.3
> >   Ubuntu (Xenial): 2.48.0
> >   macOS (Homebrew): 2.56.0
> > 
> > This suggests that a minimum glib of 2.42 is a reasonable target.
> 
> Looks like useful info to check in somewhere -- though I suppose the
> commit message counts.

Yeah I think commit message is sufficient

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v2 1/3] glib: bump min required glib library version to 2.40

[Olaf Hering]

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

Am Thu, 7 Jun 2018 09:25:24 +0100
schrieb Daniel P. Berrangé <berrange@redhat.com>:

> https://qemu.weilnetz.de/doc/qemu-doc.html#Supported-build-platforms

SLE12-SP2: 2.48.2

Olaf

[-- Attachment #2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [Qemu-devel] [PATCH v2 1/3] glib: bump min required glib library version to 2.40

[Daniel P. Berrangé]

On Thu, Jun 07, 2018 at 10:28:39AM +0200, Olaf Hering wrote:
> Am Thu, 7 Jun 2018 09:25:24 +0100
> schrieb Daniel P. Berrangé <berrange@redhat.com>:
> 
> > https://qemu.weilnetz.de/doc/qemu-doc.html#Supported-build-platforms
> 
> SLE12-SP2: 2.48.2

Thanks I'm adding that info


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v2 1/3] glib: bump min required glib library version to 2.40

[Thomas Huth]

On 06.06.2018 19:32, Daniel P. Berrangé wrote:
> Per supported platforms doc, the various min glib on relevant distros is:
> 
>   RHEL-7: 2.50.3
>   Debian (Stretch): 2.50.3
>   Debian (Jessie): 2.42.1
>   OpenBSD (Ports): 2.54.3
>   FreeBSD (Ports): 2.50.3
>   OpenSUSE Leap 15: 2.54.3
>   Ubuntu (Xenial): 2.48.0
>   macOS (Homebrew): 2.56.0
> 
> This suggests that a minimum glib of 2.42 is a reasonable target.
> 
> The GLibC compile farm, however, uses Ubuntu 14.04 (Trusty) which only
> has glib 2.40.0, and this is needed for testing during merge. Thus an
> exception is made to the documented platform support policy to allow for
> all three current LTS releases to be supported.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  configure               |   6 +-
>  crypto/hash-glib.c      |   4 -
>  crypto/hmac-glib.c      |  36 -----
>  include/glib-compat.h   | 319 ----------------------------------------
>  qga/commands.c          |  11 +-
>  tests/ivshmem-test.c    |   6 -
>  tests/test-qmp-event.c  |   8 +-
>  tests/tpm-emu.h         |   4 +-
>  tests/vhost-user-test.c |  26 +---
>  trace/simple.c          |   6 +-
>  util/osdep.c            |  14 --
>  11 files changed, 11 insertions(+), 429 deletions(-)

Great diffstat!

And the changes look fine to me:

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

[Qemu-devel] [PATCH v2 2/3] glib: enforce the minimum required version and warn about old APIs

[Daniel P. Berrangé]

There are two useful macros that can be defined before including
glib.h that are related to the min required glib version

 - GLIB_VERSION_MIN_REQUIRED

   When this is defined, if code uses an API that was deprecated
   in this version, or older, a compiler warning will be emitted.
   This alerts maintainers to update their code to whatever new
   replacement API is now recommended best practice.

 - GLIB_VERSION_MAX_ALLOWED

   When this is defined, if code uses an API that was introduced
   in a version that is newer than the declared version, a compiler
   warning will be emitted. This alerts maintainers if new code
   accidentally uses functionality that won't be available on some
   supported platforms.

The GLIB_VERSION_MAX_ALLOWED constant makes it a bit harder to opt
in to using specific new APIs with a GLIB_CHECK_VERSION conditional.
To workaround this Pragmas can be used to temporarily turn off the
-Wdeprecated-declarations compiler warning, while a static inline
compat function is implemented. This workaround is illustrated with the
implementation of the g_strv_contains method to satisfy the test suite.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 include/glib-compat.h                    | 67 ++++++++++++++++++++++++
 tests/docker/dockerfiles/centos6.docker  | 30 -----------
 tests/docker/dockerfiles/min-glib.docker |  8 ---
 tests/test-qga.c                         |  2 -
 4 files changed, 67 insertions(+), 40 deletions(-)
 delete mode 100644 tests/docker/dockerfiles/centos6.docker
 delete mode 100644 tests/docker/dockerfiles/min-glib.docker

diff --git a/include/glib-compat.h b/include/glib-compat.h
index 3b340ab33c..cfc3cba751 100644
--- a/include/glib-compat.h
+++ b/include/glib-compat.h
@@ -16,8 +16,73 @@
 #ifndef QEMU_GLIB_COMPAT_H
 #define QEMU_GLIB_COMPAT_H
 
+/* Ask for warnings for anything that was marked deprecated in
+ * the defined version, or before. It is a candidate for rewrite.
+ */
+#define GLIB_VERSION_MIN_REQUIRED GLIB_VERSION_2_40
+
+/* Ask for warnings if code tries to use function that did not
+ * exist in the defined version. These risk breaking builds
+ */
+#define GLIB_VERSION_MAX_ALLOWED GLIB_VERSION_2_40
+
+_Pragma("GCC diagnostic push")
+_Pragma("GCC diagnostic ignored \"-Wdeprecated-declarations\"")
 #include <glib.h>
 
+/*
+ * Note that because of the GLIB_VERSION_MAX_ALLOWED constant above, allowing
+ * use of functions from newer GLib via this compat header needs a little
+ * trickery to prevent warnings being emitted.
+ *
+ * Consider a function from newer glib-X.Y that we want to use
+ *
+ *    int g_foo(const char *wibble)
+ *
+ * We must define a static inline function with the same signature that does
+ * what we need, but with a "_qemu" suffix e.g.
+ *
+ * static inline void g_foo_qemu(const char *wibble)
+ * {
+ *     #if GLIB_CHECK_VERSION(X, Y, 0)
+ *        g_foo(wibble)
+ *     #else
+ *        g_something_equivalent_in_older_glib(wibble);
+ *     #endif
+ * }
+ *
+ * The _Pragma at the top of this file turns off -Wdeprecated-declarations,
+ * ensuring this wrapper function impl doesn't trigger the compiler warning
+ * about using too new glib APIs. Finally we can do
+ *
+ *   #define g_foo(a) g_foo_qemu(a)
+ *
+ * So now the code elsewhere in QEMU, which *does* have the
+ * -Wdeprecated-declarations warning active, can call g_foo(...) as normal,
+ * without generating warnings.
+ */
+
+static inline gboolean g_strv_contains_qemu(const gchar *const *strv,
+                                            const gchar *str)
+{
+#if GLIB_CHECK_VERSION(2, 44, 0)
+    return g_strv_contains(strv, str);
+#else
+    g_return_val_if_fail(strv != NULL, FALSE);
+    g_return_val_if_fail(str != NULL, FALSE);
+
+    for (; *strv != NULL; strv++) {
+        if (g_str_equal(str, *strv)) {
+            return TRUE;
+        }
+    }
+
+    return FALSE;
+#endif
+}
+#define g_strv_contains(a, b) g_strv_contains_qemu(a, b)
+
+
 #if defined(_WIN32) && !GLIB_CHECK_VERSION(2, 50, 0)
 /*
  * g_poll has a problem on Windows when using
@@ -45,4 +110,6 @@ gint g_poll_fixed(GPollFD *fds, guint nfds, gint timeout);
     } while (0)
 #endif
 
+_Pragma("GCC diagnostic pop")
+
 #endif
diff --git a/tests/docker/dockerfiles/centos6.docker b/tests/docker/dockerfiles/centos6.docker
deleted file mode 100644
index ad24319582..0000000000
--- a/tests/docker/dockerfiles/centos6.docker
+++ /dev/null
@@ -1,30 +0,0 @@
-FROM centos:6
-RUN yum install -y epel-release centos-release-xen
-ENV PACKAGES \
-    bison \
-    bzip2-devel \
-    ccache \
-    csnappy-devel \
-    flex \
-    g++ \
-    gcc \
-    gettext \
-    git \
-    glib2-devel \
-    libepoxy-devel \
-    libfdt-devel \
-    librdmacm-devel \
-    lzo-devel \
-    make \
-    mesa-libEGL-devel \
-    mesa-libgbm-devel \
-    pixman-devel \
-    SDL-devel \
-    spice-glib-devel \
-    spice-server-devel \
-    tar \
-    vte-devel \
-    xen-devel \
-    zlib-devel
-RUN yum install -y $PACKAGES
-RUN rpm -q $PACKAGES | sort > /packages.txt
diff --git a/tests/docker/dockerfiles/min-glib.docker b/tests/docker/dockerfiles/min-glib.docker
deleted file mode 100644
index f2eed97d35..0000000000
--- a/tests/docker/dockerfiles/min-glib.docker
+++ /dev/null
@@ -1,8 +0,0 @@
-FROM centos:6
-RUN yum install -y \
-    tar gettext git make gcc g++ \
-    zlib-devel SDL-devel pixman-devel \
-    epel-release
-RUN yum install -y libfdt-devel ccache
-RUN yum downgrade -y http://vault.centos.org/6.0/os/x86_64/Packages/glib2-2.22.5-5.el6.x86_64.rpm
-RUN yum install -y http://vault.centos.org/6.0/os/x86_64/Packages/glib2-devel-2.22.5-5.el6.x86_64.rpm
diff --git a/tests/test-qga.c b/tests/test-qga.c
index 18e63cb533..30c9643257 100644
--- a/tests/test-qga.c
+++ b/tests/test-qga.c
@@ -744,12 +744,10 @@ static void test_qga_config(gconstpointer data)
 
     strv = g_key_file_get_string_list(kf, "general", "blacklist", &n, &error);
     g_assert_cmpint(n, ==, 2);
-#if GLIB_CHECK_VERSION(2, 44, 0)
     g_assert_true(g_strv_contains((const char * const *)strv,
                                   "guest-ping"));
     g_assert_true(g_strv_contains((const char * const *)strv,
                                   "guest-get-time"));
-#endif
     g_assert_no_error(error);
     g_strfreev(strv);
 
-- 
2.17.0

Re: [Qemu-devel] [PATCH v2 2/3] glib: enforce the minimum required version and warn about old APIs

[Thomas Huth]

On 06.06.2018 19:32, Daniel P. Berrangé wrote:
> There are two useful macros that can be defined before including
> glib.h that are related to the min required glib version
> 
>  - GLIB_VERSION_MIN_REQUIRED
> 
>    When this is defined, if code uses an API that was deprecated
>    in this version, or older, a compiler warning will be emitted.
>    This alerts maintainers to update their code to whatever new
>    replacement API is now recommended best practice.
> 
>  - GLIB_VERSION_MAX_ALLOWED
> 
>    When this is defined, if code uses an API that was introduced
>    in a version that is newer than the declared version, a compiler
>    warning will be emitted. This alerts maintainers if new code
>    accidentally uses functionality that won't be available on some
>    supported platforms.
> 
> The GLIB_VERSION_MAX_ALLOWED constant makes it a bit harder to opt
> in to using specific new APIs with a GLIB_CHECK_VERSION conditional.
> To workaround this Pragmas can be used to temporarily turn off the
> -Wdeprecated-declarations compiler warning, while a static inline
> compat function is implemented. This workaround is illustrated with the
> implementation of the g_strv_contains method to satisfy the test suite.

I wonder whether that's a little bit too much magic already. We could
also set GLIB_VERSION_MAX_ALLOWED to the version where we've already got
the work-arounds in place (i.e. to 2.44 here). If we introduce
additional code that uses other new functions from 2.41 - 2.44, this
should also be caught by the patchew CI builders?

> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  include/glib-compat.h                    | 67 ++++++++++++++++++++++++
>  tests/docker/dockerfiles/centos6.docker  | 30 -----------
>  tests/docker/dockerfiles/min-glib.docker |  8 ---

The docker files are not directly related to this patch, are they? If
they are, please mention it in the patch description. If not, please
move this to a separate patch (or maybe into patch 1).

>  tests/test-qga.c                         |  2 -
>  4 files changed, 67 insertions(+), 40 deletions(-)
>  delete mode 100644 tests/docker/dockerfiles/centos6.docker
>  delete mode 100644 tests/docker/dockerfiles/min-glib.docker
> 
> diff --git a/include/glib-compat.h b/include/glib-compat.h
> index 3b340ab33c..cfc3cba751 100644
> --- a/include/glib-compat.h
> +++ b/include/glib-compat.h
> @@ -16,8 +16,73 @@
>  #ifndef QEMU_GLIB_COMPAT_H
>  #define QEMU_GLIB_COMPAT_H
>  
> +/* Ask for warnings for anything that was marked deprecated in
> + * the defined version, or before. It is a candidate for rewrite.
> + */
> +#define GLIB_VERSION_MIN_REQUIRED GLIB_VERSION_2_40
> +
> +/* Ask for warnings if code tries to use function that did not
> + * exist in the defined version. These risk breaking builds
> + */
> +#define GLIB_VERSION_MAX_ALLOWED GLIB_VERSION_2_40
> +
> +_Pragma("GCC diagnostic push")
> +_Pragma("GCC diagnostic ignored \"-Wdeprecated-declarations\"")

Why not "#pragma" ? I think that's a little bit more common?

 Thomas

Re: [Qemu-devel] [PATCH v2 2/3] glib: enforce the minimum required version and warn about old APIs

[Daniel P. Berrangé]

On Thu, Jun 07, 2018 at 10:57:56AM +0200, Thomas Huth wrote:
> On 06.06.2018 19:32, Daniel P. Berrangé wrote:
> > There are two useful macros that can be defined before including
> > glib.h that are related to the min required glib version
> > 
> >  - GLIB_VERSION_MIN_REQUIRED
> > 
> >    When this is defined, if code uses an API that was deprecated
> >    in this version, or older, a compiler warning will be emitted.
> >    This alerts maintainers to update their code to whatever new
> >    replacement API is now recommended best practice.
> > 
> >  - GLIB_VERSION_MAX_ALLOWED
> > 
> >    When this is defined, if code uses an API that was introduced
> >    in a version that is newer than the declared version, a compiler
> >    warning will be emitted. This alerts maintainers if new code
> >    accidentally uses functionality that won't be available on some
> >    supported platforms.
> > 
> > The GLIB_VERSION_MAX_ALLOWED constant makes it a bit harder to opt
> > in to using specific new APIs with a GLIB_CHECK_VERSION conditional.
> > To workaround this Pragmas can be used to temporarily turn off the
> > -Wdeprecated-declarations compiler warning, while a static inline
> > compat function is implemented. This workaround is illustrated with the
> > implementation of the g_strv_contains method to satisfy the test suite.
> 
> I wonder whether that's a little bit too much magic already. We could
> also set GLIB_VERSION_MAX_ALLOWED to the version where we've already got
> the work-arounds in place (i.e. to 2.44 here). If we introduce
> additional code that uses other new functions from 2.41 - 2.44, this
> should also be caught by the patchew CI builders?

I don't think that's a good direction to take - if we follow that
through and someone backports a function from 2.56, we'll loose
100% of the benefit of the MAX_ALLOWED checking. Given we only have
a handful of backported functions, that is not a good tradeoff.


> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  include/glib-compat.h                    | 67 ++++++++++++++++++++++++
> >  tests/docker/dockerfiles/centos6.docker  | 30 -----------
> >  tests/docker/dockerfiles/min-glib.docker |  8 ---
> 
> The docker files are not directly related to this patch, are they? If
> they are, please mention it in the patch description. If not, please
> move this to a separate patch (or maybe into patch 1).

Yeah should go in the patch that bumps the min version really.


> > +/* Ask for warnings for anything that was marked deprecated in
> > + * the defined version, or before. It is a candidate for rewrite.
> > + */
> > +#define GLIB_VERSION_MIN_REQUIRED GLIB_VERSION_2_40
> > +
> > +/* Ask for warnings if code tries to use function that did not
> > + * exist in the defined version. These risk breaking builds
> > + */
> > +#define GLIB_VERSION_MAX_ALLOWED GLIB_VERSION_2_40
> > +
> > +_Pragma("GCC diagnostic push")
> > +_Pragma("GCC diagnostic ignored \"-Wdeprecated-declarations\"")
> 
> Why not "#pragma" ? I think that's a little bit more common?

I tend to always use _Pragma as that offers superset of functionality
of #pramga, because _Pragma can be used in macros.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

[Qemu-devel] [PATCH v2 3/3] util: remove redundant include of glib.h

[Daniel P. Berrangé]

Code must only ever include glib.h indirectly via the glib-compat.h
header file, because we will need some macros set before glib.h is
pulled in. Adding extra includes of glib.h will (soon) cause compile
failures such as:

In file included from /home/berrange/src/virt/qemu/include/qemu/osdep.h:107,
                 from /home/berrange/src/virt/qemu/include/qemu/iova-tree.h:26,
                 from util/iova-tree.c:13:
/home/berrange/src/virt/qemu/include/glib-compat.h:22: error: "GLIB_VERSION_MIN_REQUIRED" redefined [-Werror]
 #define GLIB_VERSION_MIN_REQUIRED GLIB_VERSION_2_40

In file included from /usr/include/glib-2.0/glib/gtypes.h:34,
                 from /usr/include/glib-2.0/glib/galloca.h:32,
                 from /usr/include/glib-2.0/glib.h:30,
                 from util/iova-tree.c:12:
/usr/include/glib-2.0/glib/gversionmacros.h:237: note: this is the location of the previous definition
 # define GLIB_VERSION_MIN_REQUIRED      (GLIB_VERSION_CUR_STABLE)

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 util/iova-tree.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/util/iova-tree.c b/util/iova-tree.c
index 2d9cebfc89..d39cd8bb29 100644
--- a/util/iova-tree.c
+++ b/util/iova-tree.c
@@ -9,7 +9,6 @@
  * This work is licensed under the terms of the GNU GPL, version 2 or later.
  */
 
-#include <glib.h>
 #include "qemu/iova-tree.h"
 
 struct IOVATree {
-- 
2.17.0

Re: [Qemu-devel] [PATCH v2 3/3] util: remove redundant include of glib.h

[Philippe Mathieu-Daudé]

On 06/06/2018 02:32 PM, Daniel P. Berrangé wrote:
> Code must only ever include glib.h indirectly via the glib-compat.h
> header file, because we will need some macros set before glib.h is
> pulled in. Adding extra includes of glib.h will (soon) cause compile
> failures such as:
> 
> In file included from /home/berrange/src/virt/qemu/include/qemu/osdep.h:107,
>                  from /home/berrange/src/virt/qemu/include/qemu/iova-tree.h:26,
>                  from util/iova-tree.c:13:
> /home/berrange/src/virt/qemu/include/glib-compat.h:22: error: "GLIB_VERSION_MIN_REQUIRED" redefined [-Werror]
>  #define GLIB_VERSION_MIN_REQUIRED GLIB_VERSION_2_40
> 
> In file included from /usr/include/glib-2.0/glib/gtypes.h:34,
>                  from /usr/include/glib-2.0/glib/galloca.h:32,
>                  from /usr/include/glib-2.0/glib.h:30,
>                  from util/iova-tree.c:12:
> /usr/include/glib-2.0/glib/gversionmacros.h:237: note: this is the location of the previous definition
>  # define GLIB_VERSION_MIN_REQUIRED      (GLIB_VERSION_CUR_STABLE)
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  util/iova-tree.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/util/iova-tree.c b/util/iova-tree.c
> index 2d9cebfc89..d39cd8bb29 100644
> --- a/util/iova-tree.c
> +++ b/util/iova-tree.c
> @@ -9,7 +9,6 @@
>   * This work is licensed under the terms of the GNU GPL, version 2 or later.
>   */
>  
> -#include <glib.h>
>  #include "qemu/iova-tree.h"
>  
>  struct IOVATree {
> 

Re: [Qemu-devel] [PATCH v2 3/3] util: remove redundant include of glib.h

[Peter Maydell]

On 6 June 2018 at 18:32, Daniel P. Berrangé <berrange@redhat.com> wrote:
> Code must only ever include glib.h indirectly via the glib-compat.h
> header file, because we will need some macros set before glib.h is
> pulled in. Adding extra includes of glib.h will (soon) cause compile
> failures such as:
>
> In file included from /home/berrange/src/virt/qemu/include/qemu/osdep.h:107,
>                  from /home/berrange/src/virt/qemu/include/qemu/iova-tree.h:26,
>                  from util/iova-tree.c:13:
> /home/berrange/src/virt/qemu/include/glib-compat.h:22: error: "GLIB_VERSION_MIN_REQUIRED" redefined [-Werror]
>  #define GLIB_VERSION_MIN_REQUIRED GLIB_VERSION_2_40
>
> In file included from /usr/include/glib-2.0/glib/gtypes.h:34,
>                  from /usr/include/glib-2.0/glib/galloca.h:32,
>                  from /usr/include/glib-2.0/glib.h:30,
>                  from util/iova-tree.c:12:
> /usr/include/glib-2.0/glib/gversionmacros.h:237: note: this is the location of the previous definition
>  # define GLIB_VERSION_MIN_REQUIRED      (GLIB_VERSION_CUR_STABLE)
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  util/iova-tree.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/util/iova-tree.c b/util/iova-tree.c
> index 2d9cebfc89..d39cd8bb29 100644
> --- a/util/iova-tree.c
> +++ b/util/iova-tree.c
> @@ -9,7 +9,6 @@
>   * This work is licensed under the terms of the GNU GPL, version 2 or later.
>   */
>
> -#include <glib.h>
>  #include "qemu/iova-tree.h"

While we're fixing up the headers in this file:
it should start with an include of qemu/osdep.h,
and qemu/iova-tree.h should not include osdep.h...

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 3/3] util: remove redundant include of glib.h

[Peter Xu]

On Wed, Jun 06, 2018 at 07:31:53PM +0100, Peter Maydell wrote:
> On 6 June 2018 at 18:32, Daniel P. Berrangé <berrange@redhat.com> wrote:
> > Code must only ever include glib.h indirectly via the glib-compat.h
> > header file, because we will need some macros set before glib.h is
> > pulled in. Adding extra includes of glib.h will (soon) cause compile
> > failures such as:
> >
> > In file included from /home/berrange/src/virt/qemu/include/qemu/osdep.h:107,
> >                  from /home/berrange/src/virt/qemu/include/qemu/iova-tree.h:26,
> >                  from util/iova-tree.c:13:
> > /home/berrange/src/virt/qemu/include/glib-compat.h:22: error: "GLIB_VERSION_MIN_REQUIRED" redefined [-Werror]
> >  #define GLIB_VERSION_MIN_REQUIRED GLIB_VERSION_2_40
> >
> > In file included from /usr/include/glib-2.0/glib/gtypes.h:34,
> >                  from /usr/include/glib-2.0/glib/galloca.h:32,
> >                  from /usr/include/glib-2.0/glib.h:30,
> >                  from util/iova-tree.c:12:
> > /usr/include/glib-2.0/glib/gversionmacros.h:237: note: this is the location of the previous definition
> >  # define GLIB_VERSION_MIN_REQUIRED      (GLIB_VERSION_CUR_STABLE)
> >
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  util/iova-tree.c | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/util/iova-tree.c b/util/iova-tree.c
> > index 2d9cebfc89..d39cd8bb29 100644
> > --- a/util/iova-tree.c
> > +++ b/util/iova-tree.c
> > @@ -9,7 +9,6 @@
> >   * This work is licensed under the terms of the GNU GPL, version 2 or later.
> >   */
> >
> > -#include <glib.h>
> >  #include "qemu/iova-tree.h"
> 
> While we're fixing up the headers in this file:
> it should start with an include of qemu/osdep.h,
> and qemu/iova-tree.h should not include osdep.h...

Sorry to messed this up.  It was used for hwaddr definition.

Maybe we can just replace hwaddr usage in iova-tree.[ch] with
something like uint64_t?  Then I think we can drop the osdep.h.

Regards,

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v2 3/3] util: remove redundant include of glib.h

[Markus Armbruster]

Peter Xu <peterx@redhat.com> writes:

> On Wed, Jun 06, 2018 at 07:31:53PM +0100, Peter Maydell wrote:
>> On 6 June 2018 at 18:32, Daniel P. Berrangé <berrange@redhat.com> wrote:
>> > Code must only ever include glib.h indirectly via the glib-compat.h
>> > header file, because we will need some macros set before glib.h is
>> > pulled in. Adding extra includes of glib.h will (soon) cause compile
>> > failures such as:
>> >
>> > In file included from /home/berrange/src/virt/qemu/include/qemu/osdep.h:107,
>> >                  from /home/berrange/src/virt/qemu/include/qemu/iova-tree.h:26,
>> >                  from util/iova-tree.c:13:
>> > /home/berrange/src/virt/qemu/include/glib-compat.h:22: error: "GLIB_VERSION_MIN_REQUIRED" redefined [-Werror]
>> >  #define GLIB_VERSION_MIN_REQUIRED GLIB_VERSION_2_40
>> >
>> > In file included from /usr/include/glib-2.0/glib/gtypes.h:34,
>> >                  from /usr/include/glib-2.0/glib/galloca.h:32,
>> >                  from /usr/include/glib-2.0/glib.h:30,
>> >                  from util/iova-tree.c:12:
>> > /usr/include/glib-2.0/glib/gversionmacros.h:237: note: this is the location of the previous definition
>> >  # define GLIB_VERSION_MIN_REQUIRED      (GLIB_VERSION_CUR_STABLE)
>> >
>> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>> > ---
>> >  util/iova-tree.c | 1 -
>> >  1 file changed, 1 deletion(-)
>> >
>> > diff --git a/util/iova-tree.c b/util/iova-tree.c
>> > index 2d9cebfc89..d39cd8bb29 100644
>> > --- a/util/iova-tree.c
>> > +++ b/util/iova-tree.c
>> > @@ -9,7 +9,6 @@
>> >   * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> >   */
>> >
>> > -#include <glib.h>
>> >  #include "qemu/iova-tree.h"
>> 
>> While we're fixing up the headers in this file:
>> it should start with an include of qemu/osdep.h,
>> and qemu/iova-tree.h should not include osdep.h...
>
> Sorry to messed this up.  It was used for hwaddr definition.
>
> Maybe we can just replace hwaddr usage in iova-tree.[ch] with
> something like uint64_t?  Then I think we can drop the osdep.h.

Every compilation unit must include "osdep.h" first.  Its file comment
explains why.  If it's insufficiently convincing, we should fix it :)

Re: [Qemu-devel] [PATCH v2 3/3] util: remove redundant include of glib.h

[Peter Xu]

On Thu, Jun 07, 2018 at 09:05:28AM +0200, Markus Armbruster wrote:
> Peter Xu <peterx@redhat.com> writes:
> 
> > On Wed, Jun 06, 2018 at 07:31:53PM +0100, Peter Maydell wrote:
> >> On 6 June 2018 at 18:32, Daniel P. Berrangé <berrange@redhat.com> wrote:
> >> > Code must only ever include glib.h indirectly via the glib-compat.h
> >> > header file, because we will need some macros set before glib.h is
> >> > pulled in. Adding extra includes of glib.h will (soon) cause compile
> >> > failures such as:
> >> >
> >> > In file included from /home/berrange/src/virt/qemu/include/qemu/osdep.h:107,
> >> >                  from /home/berrange/src/virt/qemu/include/qemu/iova-tree.h:26,
> >> >                  from util/iova-tree.c:13:
> >> > /home/berrange/src/virt/qemu/include/glib-compat.h:22: error: "GLIB_VERSION_MIN_REQUIRED" redefined [-Werror]
> >> >  #define GLIB_VERSION_MIN_REQUIRED GLIB_VERSION_2_40
> >> >
> >> > In file included from /usr/include/glib-2.0/glib/gtypes.h:34,
> >> >                  from /usr/include/glib-2.0/glib/galloca.h:32,
> >> >                  from /usr/include/glib-2.0/glib.h:30,
> >> >                  from util/iova-tree.c:12:
> >> > /usr/include/glib-2.0/glib/gversionmacros.h:237: note: this is the location of the previous definition
> >> >  # define GLIB_VERSION_MIN_REQUIRED      (GLIB_VERSION_CUR_STABLE)
> >> >
> >> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> >> > ---
> >> >  util/iova-tree.c | 1 -
> >> >  1 file changed, 1 deletion(-)
> >> >
> >> > diff --git a/util/iova-tree.c b/util/iova-tree.c
> >> > index 2d9cebfc89..d39cd8bb29 100644
> >> > --- a/util/iova-tree.c
> >> > +++ b/util/iova-tree.c
> >> > @@ -9,7 +9,6 @@
> >> >   * This work is licensed under the terms of the GNU GPL, version 2 or later.
> >> >   */
> >> >
> >> > -#include <glib.h>
> >> >  #include "qemu/iova-tree.h"
> >> 
> >> While we're fixing up the headers in this file:
> >> it should start with an include of qemu/osdep.h,
> >> and qemu/iova-tree.h should not include osdep.h...
> >
> > Sorry to messed this up.  It was used for hwaddr definition.
> >
> > Maybe we can just replace hwaddr usage in iova-tree.[ch] with
> > something like uint64_t?  Then I think we can drop the osdep.h.
> 
> Every compilation unit must include "osdep.h" first.  Its file comment
> explains why.  If it's insufficiently convincing, we should fix it :)

Ah... :)

Then maybe also we can let iova-tree.c to include osdep.h (instead of
glib.h), and remove that line in iova-tree.h

Thanks,

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v2 3/3] util: remove redundant include of glib.h

[Daniel P. Berrangé]

On Thu, Jun 07, 2018 at 03:44:56PM +0800, Peter Xu wrote:
> On Thu, Jun 07, 2018 at 09:05:28AM +0200, Markus Armbruster wrote:
> > Peter Xu <peterx@redhat.com> writes:
> > 
> > > On Wed, Jun 06, 2018 at 07:31:53PM +0100, Peter Maydell wrote:
> > >> On 6 June 2018 at 18:32, Daniel P. Berrangé <berrange@redhat.com> wrote:
> > >> > Code must only ever include glib.h indirectly via the glib-compat.h
> > >> > header file, because we will need some macros set before glib.h is
> > >> > pulled in. Adding extra includes of glib.h will (soon) cause compile
> > >> > failures such as:
> > >> >
> > >> > In file included from /home/berrange/src/virt/qemu/include/qemu/osdep.h:107,
> > >> >                  from /home/berrange/src/virt/qemu/include/qemu/iova-tree.h:26,
> > >> >                  from util/iova-tree.c:13:
> > >> > /home/berrange/src/virt/qemu/include/glib-compat.h:22: error: "GLIB_VERSION_MIN_REQUIRED" redefined [-Werror]
> > >> >  #define GLIB_VERSION_MIN_REQUIRED GLIB_VERSION_2_40
> > >> >
> > >> > In file included from /usr/include/glib-2.0/glib/gtypes.h:34,
> > >> >                  from /usr/include/glib-2.0/glib/galloca.h:32,
> > >> >                  from /usr/include/glib-2.0/glib.h:30,
> > >> >                  from util/iova-tree.c:12:
> > >> > /usr/include/glib-2.0/glib/gversionmacros.h:237: note: this is the location of the previous definition
> > >> >  # define GLIB_VERSION_MIN_REQUIRED      (GLIB_VERSION_CUR_STABLE)
> > >> >
> > >> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > >> > ---
> > >> >  util/iova-tree.c | 1 -
> > >> >  1 file changed, 1 deletion(-)
> > >> >
> > >> > diff --git a/util/iova-tree.c b/util/iova-tree.c
> > >> > index 2d9cebfc89..d39cd8bb29 100644
> > >> > --- a/util/iova-tree.c
> > >> > +++ b/util/iova-tree.c
> > >> > @@ -9,7 +9,6 @@
> > >> >   * This work is licensed under the terms of the GNU GPL, version 2 or later.
> > >> >   */
> > >> >
> > >> > -#include <glib.h>
> > >> >  #include "qemu/iova-tree.h"
> > >> 
> > >> While we're fixing up the headers in this file:
> > >> it should start with an include of qemu/osdep.h,
> > >> and qemu/iova-tree.h should not include osdep.h...
> > >
> > > Sorry to messed this up.  It was used for hwaddr definition.
> > >
> > > Maybe we can just replace hwaddr usage in iova-tree.[ch] with
> > > something like uint64_t?  Then I think we can drop the osdep.h.
> > 
> > Every compilation unit must include "osdep.h" first.  Its file comment
> > explains why.  If it's insufficiently convincing, we should fix it :)
> 
> Ah... :)
> 
> Then maybe also we can let iova-tree.c to include osdep.h (instead of
> glib.h), and remove that line in iova-tree.h

Yeah, I'll repost with that changed

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v2 3/3] util: remove redundant include of glib.h

[Peter Xu]

On Thu, Jun 07, 2018 at 09:26:51AM +0100, Daniel P. Berrangé wrote:
> On Thu, Jun 07, 2018 at 03:44:56PM +0800, Peter Xu wrote:
> > On Thu, Jun 07, 2018 at 09:05:28AM +0200, Markus Armbruster wrote:
> > > Peter Xu <peterx@redhat.com> writes:
> > > 
> > > > On Wed, Jun 06, 2018 at 07:31:53PM +0100, Peter Maydell wrote:
> > > >> On 6 June 2018 at 18:32, Daniel P. Berrangé <berrange@redhat.com> wrote:
> > > >> > Code must only ever include glib.h indirectly via the glib-compat.h
> > > >> > header file, because we will need some macros set before glib.h is
> > > >> > pulled in. Adding extra includes of glib.h will (soon) cause compile
> > > >> > failures such as:
> > > >> >
> > > >> > In file included from /home/berrange/src/virt/qemu/include/qemu/osdep.h:107,
> > > >> >                  from /home/berrange/src/virt/qemu/include/qemu/iova-tree.h:26,
> > > >> >                  from util/iova-tree.c:13:
> > > >> > /home/berrange/src/virt/qemu/include/glib-compat.h:22: error: "GLIB_VERSION_MIN_REQUIRED" redefined [-Werror]
> > > >> >  #define GLIB_VERSION_MIN_REQUIRED GLIB_VERSION_2_40
> > > >> >
> > > >> > In file included from /usr/include/glib-2.0/glib/gtypes.h:34,
> > > >> >                  from /usr/include/glib-2.0/glib/galloca.h:32,
> > > >> >                  from /usr/include/glib-2.0/glib.h:30,
> > > >> >                  from util/iova-tree.c:12:
> > > >> > /usr/include/glib-2.0/glib/gversionmacros.h:237: note: this is the location of the previous definition
> > > >> >  # define GLIB_VERSION_MIN_REQUIRED      (GLIB_VERSION_CUR_STABLE)
> > > >> >
> > > >> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > > >> > ---
> > > >> >  util/iova-tree.c | 1 -
> > > >> >  1 file changed, 1 deletion(-)
> > > >> >
> > > >> > diff --git a/util/iova-tree.c b/util/iova-tree.c
> > > >> > index 2d9cebfc89..d39cd8bb29 100644
> > > >> > --- a/util/iova-tree.c
> > > >> > +++ b/util/iova-tree.c
> > > >> > @@ -9,7 +9,6 @@
> > > >> >   * This work is licensed under the terms of the GNU GPL, version 2 or later.
> > > >> >   */
> > > >> >
> > > >> > -#include <glib.h>
> > > >> >  #include "qemu/iova-tree.h"
> > > >> 
> > > >> While we're fixing up the headers in this file:
> > > >> it should start with an include of qemu/osdep.h,
> > > >> and qemu/iova-tree.h should not include osdep.h...
> > > >
> > > > Sorry to messed this up.  It was used for hwaddr definition.
> > > >
> > > > Maybe we can just replace hwaddr usage in iova-tree.[ch] with
> > > > something like uint64_t?  Then I think we can drop the osdep.h.
> > > 
> > > Every compilation unit must include "osdep.h" first.  Its file comment
> > > explains why.  If it's insufficiently convincing, we should fix it :)
> > 
> > Ah... :)
> > 
> > Then maybe also we can let iova-tree.c to include osdep.h (instead of
> > glib.h), and remove that line in iova-tree.h
> 
> Yeah, I'll repost with that changed

Thanks for fixing that!

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v2 3/3] util: remove redundant include of glib.h

[Thomas Huth]

On 06.06.2018 19:32, Daniel P. Berrangé wrote:
> Code must only ever include glib.h indirectly via the glib-compat.h
> header file, because we will need some macros set before glib.h is
> pulled in. Adding extra includes of glib.h will (soon) cause compile
> failures such as:
> 
> In file included from /home/berrange/src/virt/qemu/include/qemu/osdep.h:107,
>                  from /home/berrange/src/virt/qemu/include/qemu/iova-tree.h:26,
>                  from util/iova-tree.c:13:
> /home/berrange/src/virt/qemu/include/glib-compat.h:22: error: "GLIB_VERSION_MIN_REQUIRED" redefined [-Werror]
>  #define GLIB_VERSION_MIN_REQUIRED GLIB_VERSION_2_40
> 
> In file included from /usr/include/glib-2.0/glib/gtypes.h:34,
>                  from /usr/include/glib-2.0/glib/galloca.h:32,
>                  from /usr/include/glib-2.0/glib.h:30,
>                  from util/iova-tree.c:12:
> /usr/include/glib-2.0/glib/gversionmacros.h:237: note: this is the location of the previous definition
>  # define GLIB_VERSION_MIN_REQUIRED      (GLIB_VERSION_CUR_STABLE)

In case you respin this series, should the order of patch 2 and 3 be
swapped? ... so that we keep the tree bisectable?

 Thomas

Re: [Qemu-devel] [PATCH v2 3/3] util: remove redundant include of glib.h

[Daniel P. Berrangé]

On Thu, Jun 07, 2018 at 10:44:15AM +0200, Thomas Huth wrote:
> On 06.06.2018 19:32, Daniel P. Berrangé wrote:
> > Code must only ever include glib.h indirectly via the glib-compat.h
> > header file, because we will need some macros set before glib.h is
> > pulled in. Adding extra includes of glib.h will (soon) cause compile
> > failures such as:
> > 
> > In file included from /home/berrange/src/virt/qemu/include/qemu/osdep.h:107,
> >                  from /home/berrange/src/virt/qemu/include/qemu/iova-tree.h:26,
> >                  from util/iova-tree.c:13:
> > /home/berrange/src/virt/qemu/include/glib-compat.h:22: error: "GLIB_VERSION_MIN_REQUIRED" redefined [-Werror]
> >  #define GLIB_VERSION_MIN_REQUIRED GLIB_VERSION_2_40
> > 
> > In file included from /usr/include/glib-2.0/glib/gtypes.h:34,
> >                  from /usr/include/glib-2.0/glib/galloca.h:32,
> >                  from /usr/include/glib-2.0/glib.h:30,
> >                  from util/iova-tree.c:12:
> > /usr/include/glib-2.0/glib/gversionmacros.h:237: note: this is the location of the previous definition
> >  # define GLIB_VERSION_MIN_REQUIRED      (GLIB_VERSION_CUR_STABLE)
> 
> In case you respin this series, should the order of patch 2 and 3 be
> swapped? ... so that we keep the tree bisectable?

Yes it should be.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v2 0/3] glib: update the min required version

[Peter Maydell]

On 6 June 2018 at 18:32, Daniel P. Berrangé <berrange@redhat.com> wrote:
> The previous patch to bump glib to 2.42 hit problems with Peter's build
> environment for testing merge:
>
>   https://lists.gnu.org/archive/html/qemu-devel/2018-05/msg02557.html
>
> This posting drops back to 2.40, which allows Ubuntu 14.04 from GLibC
> compile farm to be supported.
>
> It does NOT try to go back to 2.34, because it is hoped that the mxe.cc
> Debian packages will be suitable for Peter to test Windows
> cross-compile. Alternatively the docker environments provided in tree
> can be used for mingw build testing on any host able to run docker.
>
> I also dropped some more GLIB_CHECK_VERSION checks that are redundant
> given the new min version.

Note that updating to MXE is still on my todo list; I'll let
you know when I get to it...

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 0/3] glib: update the min required version

[Peter Maydell]

On 8 June 2018 at 13:00, Peter Maydell <peter.maydell@linaro.org> wrote:
> Note that updating to MXE is still on my todo list; I'll let
> you know when I get to it...

...and now I've updated my windows crossbuild environment.

thanks
-- PMM