Skip to contentSkip to conversation

glib: update the min required version

PATCH Superseded Daniel P. Berrangé 8 participants 22 messages Jun 6, 2018 – Jun 8, 2018 Crypto API

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Daniel P. Berrangé

author
#0 lore raw

v2 cover glib: update the min required version

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
Signature · 1 line
2.17.0
author
#1 lore raw

PATCH v2 1/3 glib: bump min required glib library version to 2.40

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é <[email protected]>
---
 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(-)
configure +1 -5
index 14b11130a7..3f001dc849 100755
@@ -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"
crypto/hash-glib.c +0 -4
index a5871cc72f..a0096c7c47 100644
@@ -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,
 };
crypto/hmac-glib.c +0 -36
index a6c1730291..7df627329d 100644
@@ -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,
include/glib-compat.h +0 -319

335 lines not on this page · raw message

qga/commands.c +2 -9
index cce3010f0f..0c7d1385c2 100644
@@ -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
 }
tests/ivshmem-test.c +0 -6
index 8af16ee79a..9b407a3e42 100644
@@ -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);
tests/test-qmp-event.c +1 -7
index 3a7c227a1d..8677094ad1 100644
@@ -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);
tests/tpm-emu.h +2 -2
index ef4bfa8800..08f902485e 100644
@@ -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;
tests/vhost-user-test.c +2 -24
index bbc8091286..8ff2106d32 100644
@@ -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,
 };
trace/simple.c +3 -3
index e82018d923..701dec639c 100644
@@ -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;
util/osdep.c +0 -14
index a73de0e1ba..8bfa022673 100644
@@ -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() */
Signature · 1 line
2.17.0
#2 lore raw

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

Quoted 1 line · Daniel P. Berrangé

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

Which doc?

Quoted 11 lines

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

author
#3 lore raw

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

Quoted 6 lines · John Snow
Quoted 1 line · Daniel P. Berrangé

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

Quoted 11 lines
Quoted 11 lines

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

Quoted 4 lines
Quoted 1 line

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

Signature · 3 lines
|: 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 :|
author
#5 lore raw

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

Quoted 6 lines · Olaf Hering

Am Thu, 7 Jun 2018 09:25:24 +0100
schrieb Daniel P. Berrangé <[email protected]>:

Quoted 1 line

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

SLE12-SP2: 2.48.2

Thanks I'm adding that info

Regards,
Daniel

Signature · 3 lines
|: 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 :|
Reviewed-by on v2 1/3
#6 lore raw

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

In reply to Daniel P. Berrangé
Quoted 19 lines · 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é <[email protected]>
Quoted 13 lines
---
 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 <[email protected]>
author
#7 lore raw

PATCH v2 2/3 glib: enforce the minimum required version and warn about old APIs

In reply to 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é <[email protected]>
---
 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
include/glib-compat.h +67 -0
index 3b340ab33c..cfc3cba751 100644
@@ -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
tests/docker/dockerfiles/centos6.docker deleted +0 -30
deleted file mode 100644
index ad24319582..0000000000
@@ -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
tests/docker/dockerfiles/min-glib.docker deleted +0 -8
deleted file mode 100644
index f2eed97d35..0000000000
@@ -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
tests/test-qga.c +0 -2
index 18e63cb533..30c9643257 100644
@@ -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);
Signature · 1 line
2.17.0
#8 lore raw

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

Quoted 18 lines · 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.

Quoted 6 lines

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?

Quoted 5 lines
Signed-off-by Daniel P. Berrangé <[email protected]>
---
 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).

Quoted 15 lines
 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
include/glib-compat.h +2 -0
index 3b340ab33c..cfc3cba751 100644
@@ -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.

include/glib-compat.h line 30

Quoted 10 lines
include/glib-compat.h +10 -0
+ */
+#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

author
#9 lore raw

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

Quoted 26 lines · Thomas Huth
Quoted 18 lines · 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.

Quoted 6 lines

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.

Quoted 5 lines

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.

Quoted 9 lines
Quoted 5 lines
Signed-off-by Daniel P. Berrangé <[email protected]>
---
 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.

Quoted 6 lines
Quoted 6 lines
include/glib-compat.h +6 -0
+/* 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
Quoted 8 lines

include/glib-compat.h line 30

Quoted 6 lines

+ * exist in the defined version. These risk breaking builds
+ */

include/glib-compat.h +4 -0
+#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

Signature · 3 lines
|: 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 :|
author
#10 lore raw

PATCH v2 3/3 util: remove redundant include of glib.h

In reply to 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é <[email protected]>
---
 util/iova-tree.c | 1 -
 1 file changed, 1 deletion(-)
util/iova-tree.c +0 -1
index 2d9cebfc89..d39cd8bb29 100644
@@ -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 {
Signature · 1 line
2.17.0
Reviewed-by on v2 3/3
#11 lore raw

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

Quoted 11 lines · 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

Quoted 8 lines

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é <[email protected]>
Reviewed-by Philippe Mathieu-Daudé <[email protected]>
Quoted 17 lines
---
 util/iova-tree.c | 1 -
 1 file changed, 1 deletion(-)
util/iova-tree.c +0 -1
index 2d9cebfc89..d39cd8bb29 100644
@@ -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 {
#12 lore raw

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

In reply to Daniel P. Berrangé
Quoted 28 lines · 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é <[email protected]>
---
 util/iova-tree.c | 1 -
 1 file changed, 1 deletion(-)
util/iova-tree.c +0 -0
index 2d9cebfc89..d39cd8bb29 100644

util/iova-tree.c line 12

Quoted 5 lines
util/iova-tree.c +0 -1
@@ -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

#13 lore raw

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

Quoted 32 lines · Peter Maydell
Quoted 31 lines · 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é <[email protected]>
---
 util/iova-tree.c | 1 -
 1 file changed, 1 deletion(-)
util/iova-tree.c +0 -0
index 2d9cebfc89..d39cd8bb29 100644
@@ -9,7 +9,6 @@
  * This work is licensed under the terms of the GNU GPL, version 2 or later.
  */
Quoted 6 lines

util/iova-tree.c line 12

Quoted 2 lines
util/iova-tree.c +0 -1
-#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,

Signature · 1 line
Peter Xu
#14 lore raw

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

Quoted 40 lines · Peter Xu
Quoted 32 lines · Peter Maydell
Quoted 31 lines · 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é <[email protected]>
---
 util/iova-tree.c | 1 -
 1 file changed, 1 deletion(-)
util/iova-tree.c +0 -0
index 2d9cebfc89..d39cd8bb29 100644
@@ -9,7 +9,6 @@
  * This work is licensed under the terms of the GNU GPL, version 2 or later.
  */
Quoted 6 lines

util/iova-tree.c line 12

Quoted 2 lines
util/iova-tree.c +0 -1
-#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...

Quoted 4 lines

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 :)

#15 lore raw

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

Quoted 42 lines · Markus Armbruster
Quoted 40 lines · Peter Xu
Quoted 38 lines · Peter Maydell
Quoted 28 lines · 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é <[email protected]>
---
 util/iova-tree.c | 1 -
 1 file changed, 1 deletion(-)
util/iova-tree.c +0 -0
index 2d9cebfc89..d39cd8bb29 100644

util/iova-tree.c line 12

Quoted 5 lines
util/iova-tree.c +0 -1
@@ -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...

Quoted 7 lines
Quoted 4 lines

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,

Signature · 1 line
Peter Xu
author
#16 lore raw

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

Quoted 48 lines · Peter Xu
Quoted 47 lines · Markus Armbruster
Quoted 44 lines · Peter Xu
Quoted 32 lines · Peter Maydell
Quoted 31 lines · 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é <[email protected]>
---
 util/iova-tree.c | 1 -
 1 file changed, 1 deletion(-)
util/iova-tree.c +0 -0
index 2d9cebfc89..d39cd8bb29 100644
@@ -9,7 +9,6 @@
  * This work is licensed under the terms of the GNU GPL, version 2 or later.
  */
Quoted 6 lines

util/iova-tree.c line 12

Quoted 2 lines
util/iova-tree.c +0 -1
-#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.

Quoted 7 lines
Quoted 2 lines

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

Signature · 3 lines
|: 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 :|
#17 lore raw

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

Quoted 52 lines · Daniel P. Berrangé
Quoted 51 lines · Peter Xu
Quoted 49 lines · Markus Armbruster
Quoted 40 lines · Peter Xu
Quoted 32 lines · Peter Maydell
Quoted 31 lines · 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é <[email protected]>
---
 util/iova-tree.c | 1 -
 1 file changed, 1 deletion(-)
util/iova-tree.c +0 -0
index 2d9cebfc89..d39cd8bb29 100644
@@ -9,7 +9,6 @@
  * This work is licensed under the terms of the GNU GPL, version 2 or later.
  */
Quoted 6 lines

util/iova-tree.c line 12

Quoted 2 lines
util/iova-tree.c +0 -1
-#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...

Quoted 4 lines

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 :)

Quoted 6 lines
Quoted 4 lines

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!

Signature · 1 line
Peter Xu
#18 lore raw

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

In reply to Daniel P. Berrangé
Quoted 11 lines · 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

Quoted 6 lines

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

author
#19 lore raw

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

Quoted 19 lines · Thomas Huth
Quoted 11 lines · 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

Quoted 6 lines

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)
Quoted 2 lines

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

Signature · 3 lines
|: 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 :|
#20 lore raw
In reply to Daniel P. Berrangé
Quoted 8 lines · 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.

Quoted 7 lines

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

#21 lore raw
Quoted 2 lines · Peter Maydell

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