[Qemu-devel] [PATCH 0/1] util/path: Do not cache all filenames at startup

[Qemu-devel] [PATCH 0/1] util/path: Do not cache all filenames at startup

[Richard Henderson]

This is a third variant attempting to fix the problem of
the -L interp_prefix handling not coping well pointing to
a full chroot.

Previous versions include:
https://lists.gnu.org/archive/html/qemu-devel/2018-05/msg06592.html
https://lists.gnu.org/archive/html/qemu-devel/2018-05/msg07304.html

Both of the other versions keep an open file descriptor to
the directory that is interp_prefix.  While this seems to be
efficient, they also leak a file descriptor into the guest.
This has follow-on effects on the emulation, causing failures
e.g. within the LTP testsuite.

This version uses only string manipulation and access(2).

A plausible criticism of this is the mutex and the cache.
Are they really needed?  I have no idea.  I can imagine
removing that is actually more efficient, not needing a
lock around a shared data structure.

Comments?


r~


Richard Henderson (1):
  util/path: Do not cache all filenames at startup

 util/path.c | 211 ++++++++++++++--------------------------------------
 1 file changed, 57 insertions(+), 154 deletions(-)

-- 
2.17.1

[Qemu-devel] [PATCH 0/1] util/path: Do not cache all filenames at startup

[Richard Henderson]

This is a third variant attempting to fix the problem of
the -L interp_prefix handling not coping well pointing to
a full chroot.

Previous versions include:
https://lists.gnu.org/archive/html/qemu-devel/2018-05/msg06592.html
https://lists.gnu.org/archive/html/qemu-devel/2018-05/msg07304.html

Both of the other versions keep an open file descriptor to
the directory that is interp_prefix.  While this seems to be
efficient, they also leak a file descriptor into the guest.
This has follow-on effects on the emulation, causing failures
e.g. within the LTP testsuite.

This version uses only string manipulation and access(2).

A plausible criticism of this is the mutex and the cache.
Are they really needed?  I have no idea.  I can imagine
removing that is actually more efficient, not needing a
lock around a shared data structure.

Comments?


r~


Richard Henderson (1):
  util/path: Do not cache all filenames at startup

 util/path.c | 211 ++++++++++++++--------------------------------------
 1 file changed, 57 insertions(+), 154 deletions(-)

-- 
2.17.1

[Qemu-devel] [PATCH 1/1] util/path: Do not cache all filenames at startup

[Richard Henderson]

If one uses -L $PATH to point to a full chroot, the startup time
is significant.  In addition, the existing probing algorithm fails
to handle symlink loops.

Instead, probe individual paths on demand.  Cache both positive
and negative results within $PATH, so that any one filename is
probed only once.

Use glib filename functions for clarity.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 util/path.c | 211 ++++++++++++++--------------------------------------
 1 file changed, 57 insertions(+), 154 deletions(-)

diff --git a/util/path.c b/util/path.c
index 7f9fc272fb..09f75f100a 100644
--- a/util/path.c
+++ b/util/path.c
@@ -8,170 +8,73 @@
 #include <dirent.h>
 #include "qemu/cutils.h"
 #include "qemu/path.h"
+#include "qemu/thread.h"
 
-struct pathelem
-{
-    /* Name of this, eg. lib */
-    char *name;
-    /* Full path name, eg. /usr/gnemul/x86-linux/lib. */
-    char *pathname;
-    struct pathelem *parent;
-    /* Children */
-    unsigned int num_entries;
-    struct pathelem *entries[0];
-};
-
-static struct pathelem *base;
-
-/* First N chars of S1 match S2, and S2 is N chars long. */
-static int strneq(const char *s1, unsigned int n, const char *s2)
-{
-    unsigned int i;
-
-    for (i = 0; i < n; i++)
-        if (s1[i] != s2[i])
-            return 0;
-    return s2[i] == 0;
-}
-
-static struct pathelem *add_entry(struct pathelem *root, const char *name,
-                                  unsigned type);
-
-static struct pathelem *new_entry(const char *root,
-                                  struct pathelem *parent,
-                                  const char *name)
-{
-    struct pathelem *new = g_malloc(sizeof(*new));
-    new->name = g_strdup(name);
-    new->pathname = g_strdup_printf("%s/%s", root, name);
-    new->num_entries = 0;
-    return new;
-}
-
-#define streq(a,b) (strcmp((a), (b)) == 0)
-
-/* Not all systems provide this feature */
-#if defined(DT_DIR) && defined(DT_UNKNOWN) && defined(DT_LNK)
-# define dirent_type(dirent) ((dirent)->d_type)
-# define is_dir_maybe(type) \
-    ((type) == DT_DIR || (type) == DT_UNKNOWN || (type) == DT_LNK)
-#else
-# define dirent_type(dirent) (1)
-# define is_dir_maybe(type)  (type)
-#endif
-
-static struct pathelem *add_dir_maybe(struct pathelem *path)
-{
-    DIR *dir;
-
-    if ((dir = opendir(path->pathname)) != NULL) {
-        struct dirent *dirent;
-
-        while ((dirent = readdir(dir)) != NULL) {
-            if (!streq(dirent->d_name,".") && !streq(dirent->d_name,"..")){
-                path = add_entry(path, dirent->d_name, dirent_type(dirent));
-            }
-        }
-        closedir(dir);
-    }
-    return path;
-}
-
-static struct pathelem *add_entry(struct pathelem *root, const char *name,
-                                  unsigned type)
-{
-    struct pathelem **e;
-
-    root->num_entries++;
-
-    root = g_realloc(root, sizeof(*root)
-                   + sizeof(root->entries[0])*root->num_entries);
-    e = &root->entries[root->num_entries-1];
-
-    *e = new_entry(root->pathname, root, name);
-    if (is_dir_maybe(type)) {
-        *e = add_dir_maybe(*e);
-    }
-
-    return root;
-}
-
-/* This needs to be done after tree is stabilized (ie. no more reallocs!). */
-static void set_parents(struct pathelem *child, struct pathelem *parent)
-{
-    unsigned int i;
-
-    child->parent = parent;
-    for (i = 0; i < child->num_entries; i++)
-        set_parents(child->entries[i], child);
-}
-
-/* FIXME: Doesn't handle DIR/.. where DIR is not in emulated dir. */
-static const char *
-follow_path(const struct pathelem *cursor, const char *name)
-{
-    unsigned int i, namelen;
-
-    name += strspn(name, "/");
-    namelen = strcspn(name, "/");
-
-    if (namelen == 0)
-        return cursor->pathname;
-
-    if (strneq(name, namelen, ".."))
-        return follow_path(cursor->parent, name + namelen);
-
-    if (strneq(name, namelen, "."))
-        return follow_path(cursor, name + namelen);
-
-    for (i = 0; i < cursor->num_entries; i++)
-        if (strneq(name, namelen, cursor->entries[i]->name))
-            return follow_path(cursor->entries[i], name + namelen);
-
-    /* Not found */
-    return NULL;
-}
+static const char *base;
+static GHashTable *hash;
+static QemuMutex lock;
 
 void init_paths(const char *prefix)
 {
-    char pref_buf[PATH_MAX];
-
-    if (prefix[0] == '\0' ||
-        !strcmp(prefix, "/"))
+    if (prefix[0] == '\0' || !strcmp(prefix, "/")) {
         return;
-
-    if (prefix[0] != '/') {
-        char *cwd = getcwd(NULL, 0);
-        size_t pref_buf_len = sizeof(pref_buf);
-
-        if (!cwd)
-            abort();
-        pstrcpy(pref_buf, sizeof(pref_buf), cwd);
-        pstrcat(pref_buf, pref_buf_len, "/");
-        pstrcat(pref_buf, pref_buf_len, prefix);
-        free(cwd);
-    } else
-        pstrcpy(pref_buf, sizeof(pref_buf), prefix + 1);
-
-    base = new_entry("", NULL, pref_buf);
-    base = add_dir_maybe(base);
-    if (base->num_entries == 0) {
-        g_free(base->pathname);
-        g_free(base->name);
-        g_free(base);
-        base = NULL;
-    } else {
-        set_parents(base, base);
     }
+
+#if GLIB_CHECK_VERSION(2, 58, 0)
+    base = g_canonicalize_filename(prefix, NULL);
+#else
+    if (prefix[0] != '/') {
+        char *cwd = g_get_current_dir();
+        base = g_build_filename(cwd, prefix, NULL);
+        g_free(cwd);
+    } else {
+        base = g_strdup(prefix);
+    }
+#endif
+
+    hash = g_hash_table_new(g_str_hash, g_str_equal);
+    qemu_mutex_init(&lock);
 }
 
 /* Look for path in emulation dir, otherwise return name. */
 const char *path(const char *name)
 {
-    /* Only do absolute paths: quick and dirty, but should mostly be OK.
-       Could do relative by tracking cwd. */
-    if (!base || !name || name[0] != '/')
-        return name;
+    gpointer key, value;
+    char *ret;
 
-    return follow_path(base, name) ?: name;
+    /* Only do absolute paths: quick and dirty, but should mostly be OK.  */
+    if (!base || !name || name[0] != '/') {
+        return name;
+    }
+
+    qemu_mutex_lock(&lock);
+
+    /* Have we looked up this file before?  */
+    if (g_hash_table_lookup_extended(hash, name, &key, &value)) {
+        ret = value ? value : name;
+    } else {
+        char *full_name, *save_name;
+
+        save_name = g_strdup(name);
+#if GLIB_CHECK_VERSION(2, 58, 0)
+        full_name = g_canonicalize_filename(g_path_skip_root(name), base);
+#else
+        full_name = g_build_filename(base, name, NULL);
+#endif
+
+        /* Look for the path; record the result, pass or fail.  */
+        if (access(full_name, F_OK) == 0) {
+            /* Exists.  */
+            g_hash_table_insert(hash, save_name, full_name);
+            ret = full_name;
+        } else {
+            /* Does not exist.  */
+            g_free(full_name);
+            g_hash_table_insert(hash, save_name, NULL);
+            ret = name;
+        }
+    }
+
+    qemu_mutex_unlock(&lock);
+    return ret;
 }
-- 
2.17.1

[Qemu-devel] [PATCH 1/1] util/path: Do not cache all filenames at startup

[Richard Henderson]

If one uses -L $PATH to point to a full chroot, the startup time
is significant.  In addition, the existing probing algorithm fails
to handle symlink loops.

Instead, probe individual paths on demand.  Cache both positive
and negative results within $PATH, so that any one filename is
probed only once.

Use glib filename functions for clarity.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 util/path.c | 211 ++++++++++++++--------------------------------------
 1 file changed, 57 insertions(+), 154 deletions(-)

diff --git a/util/path.c b/util/path.c
index 7f9fc272fb..09f75f100a 100644
--- a/util/path.c
+++ b/util/path.c
@@ -8,170 +8,73 @@
 #include <dirent.h>
 #include "qemu/cutils.h"
 #include "qemu/path.h"
+#include "qemu/thread.h"
 
-struct pathelem
-{
-    /* Name of this, eg. lib */
-    char *name;
-    /* Full path name, eg. /usr/gnemul/x86-linux/lib. */
-    char *pathname;
-    struct pathelem *parent;
-    /* Children */
-    unsigned int num_entries;
-    struct pathelem *entries[0];
-};
-
-static struct pathelem *base;
-
-/* First N chars of S1 match S2, and S2 is N chars long. */
-static int strneq(const char *s1, unsigned int n, const char *s2)
-{
-    unsigned int i;
-
-    for (i = 0; i < n; i++)
-        if (s1[i] != s2[i])
-            return 0;
-    return s2[i] == 0;
-}
-
-static struct pathelem *add_entry(struct pathelem *root, const char *name,
-                                  unsigned type);
-
-static struct pathelem *new_entry(const char *root,
-                                  struct pathelem *parent,
-                                  const char *name)
-{
-    struct pathelem *new = g_malloc(sizeof(*new));
-    new->name = g_strdup(name);
-    new->pathname = g_strdup_printf("%s/%s", root, name);
-    new->num_entries = 0;
-    return new;
-}
-
-#define streq(a,b) (strcmp((a), (b)) == 0)
-
-/* Not all systems provide this feature */
-#if defined(DT_DIR) && defined(DT_UNKNOWN) && defined(DT_LNK)
-# define dirent_type(dirent) ((dirent)->d_type)
-# define is_dir_maybe(type) \
-    ((type) == DT_DIR || (type) == DT_UNKNOWN || (type) == DT_LNK)
-#else
-# define dirent_type(dirent) (1)
-# define is_dir_maybe(type)  (type)
-#endif
-
-static struct pathelem *add_dir_maybe(struct pathelem *path)
-{
-    DIR *dir;
-
-    if ((dir = opendir(path->pathname)) != NULL) {
-        struct dirent *dirent;
-
-        while ((dirent = readdir(dir)) != NULL) {
-            if (!streq(dirent->d_name,".") && !streq(dirent->d_name,"..")){
-                path = add_entry(path, dirent->d_name, dirent_type(dirent));
-            }
-        }
-        closedir(dir);
-    }
-    return path;
-}
-
-static struct pathelem *add_entry(struct pathelem *root, const char *name,
-                                  unsigned type)
-{
-    struct pathelem **e;
-
-    root->num_entries++;
-
-    root = g_realloc(root, sizeof(*root)
-                   + sizeof(root->entries[0])*root->num_entries);
-    e = &root->entries[root->num_entries-1];
-
-    *e = new_entry(root->pathname, root, name);
-    if (is_dir_maybe(type)) {
-        *e = add_dir_maybe(*e);
-    }
-
-    return root;
-}
-
-/* This needs to be done after tree is stabilized (ie. no more reallocs!). */
-static void set_parents(struct pathelem *child, struct pathelem *parent)
-{
-    unsigned int i;
-
-    child->parent = parent;
-    for (i = 0; i < child->num_entries; i++)
-        set_parents(child->entries[i], child);
-}
-
-/* FIXME: Doesn't handle DIR/.. where DIR is not in emulated dir. */
-static const char *
-follow_path(const struct pathelem *cursor, const char *name)
-{
-    unsigned int i, namelen;
-
-    name += strspn(name, "/");
-    namelen = strcspn(name, "/");
-
-    if (namelen == 0)
-        return cursor->pathname;
-
-    if (strneq(name, namelen, ".."))
-        return follow_path(cursor->parent, name + namelen);
-
-    if (strneq(name, namelen, "."))
-        return follow_path(cursor, name + namelen);
-
-    for (i = 0; i < cursor->num_entries; i++)
-        if (strneq(name, namelen, cursor->entries[i]->name))
-            return follow_path(cursor->entries[i], name + namelen);
-
-    /* Not found */
-    return NULL;
-}
+static const char *base;
+static GHashTable *hash;
+static QemuMutex lock;
 
 void init_paths(const char *prefix)
 {
-    char pref_buf[PATH_MAX];
-
-    if (prefix[0] == '\0' ||
-        !strcmp(prefix, "/"))
+    if (prefix[0] == '\0' || !strcmp(prefix, "/")) {
         return;
-
-    if (prefix[0] != '/') {
-        char *cwd = getcwd(NULL, 0);
-        size_t pref_buf_len = sizeof(pref_buf);
-
-        if (!cwd)
-            abort();
-        pstrcpy(pref_buf, sizeof(pref_buf), cwd);
-        pstrcat(pref_buf, pref_buf_len, "/");
-        pstrcat(pref_buf, pref_buf_len, prefix);
-        free(cwd);
-    } else
-        pstrcpy(pref_buf, sizeof(pref_buf), prefix + 1);
-
-    base = new_entry("", NULL, pref_buf);
-    base = add_dir_maybe(base);
-    if (base->num_entries == 0) {
-        g_free(base->pathname);
-        g_free(base->name);
-        g_free(base);
-        base = NULL;
-    } else {
-        set_parents(base, base);
     }
+
+#if GLIB_CHECK_VERSION(2, 58, 0)
+    base = g_canonicalize_filename(prefix, NULL);
+#else
+    if (prefix[0] != '/') {
+        char *cwd = g_get_current_dir();
+        base = g_build_filename(cwd, prefix, NULL);
+        g_free(cwd);
+    } else {
+        base = g_strdup(prefix);
+    }
+#endif
+
+    hash = g_hash_table_new(g_str_hash, g_str_equal);
+    qemu_mutex_init(&lock);
 }
 
 /* Look for path in emulation dir, otherwise return name. */
 const char *path(const char *name)
 {
-    /* Only do absolute paths: quick and dirty, but should mostly be OK.
-       Could do relative by tracking cwd. */
-    if (!base || !name || name[0] != '/')
-        return name;
+    gpointer key, value;
+    char *ret;
 
-    return follow_path(base, name) ?: name;
+    /* Only do absolute paths: quick and dirty, but should mostly be OK.  */
+    if (!base || !name || name[0] != '/') {
+        return name;
+    }
+
+    qemu_mutex_lock(&lock);
+
+    /* Have we looked up this file before?  */
+    if (g_hash_table_lookup_extended(hash, name, &key, &value)) {
+        ret = value ? value : name;
+    } else {
+        char *full_name, *save_name;
+
+        save_name = g_strdup(name);
+#if GLIB_CHECK_VERSION(2, 58, 0)
+        full_name = g_canonicalize_filename(g_path_skip_root(name), base);
+#else
+        full_name = g_build_filename(base, name, NULL);
+#endif
+
+        /* Look for the path; record the result, pass or fail.  */
+        if (access(full_name, F_OK) == 0) {
+            /* Exists.  */
+            g_hash_table_insert(hash, save_name, full_name);
+            ret = full_name;
+        } else {
+            /* Does not exist.  */
+            g_free(full_name);
+            g_hash_table_insert(hash, save_name, NULL);
+            ret = name;
+        }
+    }
+
+    qemu_mutex_unlock(&lock);
+    return ret;
 }
-- 
2.17.1

Re: [Qemu-devel] [PATCH 0/1] util/path: Do not cache all filenames at startup

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190417053225.27505-1-richard.henderson@linaro.org/



Hi,

This series failed the asan build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC      stubs/blockdev-close-all-bdrv-states.o
  CC      stubs/clock-warp.o
  CC      stubs/cpu-get-clock.o
/tmp/qemu-test/src/util/path.c:24:12: error: 'g_canonicalize_filename' is deprecated [-Werror,-Wdeprecated-declarations]
    base = g_canonicalize_filename(prefix, NULL);
           ^
/usr/include/glib-2.0/glib/gfileutils.h:175:1: note: 'g_canonicalize_filename' has been explicitly marked deprecated here
---
/usr/include/glib-2.0/glib/gmacros.h:432:37: note: expanded from macro 'G_DEPRECATED'
#define G_DEPRECATED __attribute__((__deprecated__))
                                    ^
/tmp/qemu-test/src/util/path.c:54:13: error: assigning to 'char *' from 'const void *' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
        ret = value ? value : name;
            ^ ~~~~~~~~~~~~~~~~~~~~
/tmp/qemu-test/src/util/path.c:60:21: error: 'g_canonicalize_filename' is deprecated [-Werror,-Wdeprecated-declarations]
        full_name = g_canonicalize_filename(g_path_skip_root(name), base);
                    ^
/usr/include/glib-2.0/glib/gfileutils.h:175:1: note: 'g_canonicalize_filename' has been explicitly marked deprecated here
---
/usr/include/glib-2.0/glib/gmacros.h:432:37: note: expanded from macro 'G_DEPRECATED'
#define G_DEPRECATED __attribute__((__deprecated__))
                                    ^
/tmp/qemu-test/src/util/path.c:74:17: error: assigning to 'char *' from 'const char *' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
            ret = name;
                ^ ~~~~
4 errors generated.


The full log is available at
http://patchew.org/logs/20190417053225.27505-1-richard.henderson@linaro.org/testing.asan/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [Qemu-devel] [PATCH 0/1] util/path: Do not cache all filenames at startup

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190417053225.27505-1-richard.henderson@linaro.org/



Hi,

This series failed the asan build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC      stubs/blockdev-close-all-bdrv-states.o
  CC      stubs/clock-warp.o
  CC      stubs/cpu-get-clock.o
/tmp/qemu-test/src/util/path.c:24:12: error: 'g_canonicalize_filename' is deprecated [-Werror,-Wdeprecated-declarations]
    base = g_canonicalize_filename(prefix, NULL);
           ^
/usr/include/glib-2.0/glib/gfileutils.h:175:1: note: 'g_canonicalize_filename' has been explicitly marked deprecated here
---
/usr/include/glib-2.0/glib/gmacros.h:432:37: note: expanded from macro 'G_DEPRECATED'
#define G_DEPRECATED __attribute__((__deprecated__))
                                    ^
/tmp/qemu-test/src/util/path.c:54:13: error: assigning to 'char *' from 'const void *' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
        ret = value ? value : name;
            ^ ~~~~~~~~~~~~~~~~~~~~
/tmp/qemu-test/src/util/path.c:60:21: error: 'g_canonicalize_filename' is deprecated [-Werror,-Wdeprecated-declarations]
        full_name = g_canonicalize_filename(g_path_skip_root(name), base);
                    ^
/usr/include/glib-2.0/glib/gfileutils.h:175:1: note: 'g_canonicalize_filename' has been explicitly marked deprecated here
---
/usr/include/glib-2.0/glib/gmacros.h:432:37: note: expanded from macro 'G_DEPRECATED'
#define G_DEPRECATED __attribute__((__deprecated__))
                                    ^
/tmp/qemu-test/src/util/path.c:74:17: error: assigning to 'char *' from 'const char *' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
            ret = name;
                ^ ~~~~
4 errors generated.


The full log is available at
http://patchew.org/logs/20190417053225.27505-1-richard.henderson@linaro.org/testing.asan/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [Qemu-devel] [PATCH 0/1] util/path: Do not cache all filenames at startup

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190417053225.27505-1-richard.henderson@linaro.org/



Hi,

This series failed the docker-mingw@fedora build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
time make docker-test-mingw@fedora SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC      replication.o
  CC      block/raw-format.o
/tmp/qemu-test/src/util/path.c: In function 'init_paths':
/tmp/qemu-test/src/util/path.c:24:5: error: 'g_canonicalize_filename' is deprecated: Not available before 2.58 [-Werror=deprecated-declarations]
     base = g_canonicalize_filename(prefix, NULL);
     ^~~~
In file included from /usr/x86_64-w64-mingw32/sys-root/mingw/include/glib-2.0/glib.h:48,
---
 gchar *g_canonicalize_filename (const gchar *filename,
        ^~~~~~~~~~~~~~~~~~~~~~~
/tmp/qemu-test/src/util/path.c: In function 'path':
/tmp/qemu-test/src/util/path.c:54:13: error: assignment discards 'const' qualifier from pointer target type [-Werror=discarded-qualifiers]
         ret = value ? value : name;
             ^
/tmp/qemu-test/src/util/path.c:60:9: error: 'g_canonicalize_filename' is deprecated: Not available before 2.58 [-Werror=deprecated-declarations]
         full_name = g_canonicalize_filename(g_path_skip_root(name), base);
         ^~~~~~~~~
In file included from /usr/x86_64-w64-mingw32/sys-root/mingw/include/glib-2.0/glib.h:48,
---
/usr/x86_64-w64-mingw32/sys-root/mingw/include/glib-2.0/glib/gfileutils.h:176:8: note: declared here
 gchar *g_canonicalize_filename (const gchar *filename,
        ^~~~~~~~~~~~~~~~~~~~~~~
/tmp/qemu-test/src/util/path.c:74:17: error: assignment discards 'const' qualifier from pointer target type [-Werror=discarded-qualifiers]
             ret = name;
                 ^
cc1: all warnings being treated as errors


The full log is available at
http://patchew.org/logs/20190417053225.27505-1-richard.henderson@linaro.org/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [Qemu-devel] [PATCH 0/1] util/path: Do not cache all filenames at startup

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190417053225.27505-1-richard.henderson@linaro.org/



Hi,

This series failed the docker-mingw@fedora build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
time make docker-test-mingw@fedora SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC      replication.o
  CC      block/raw-format.o
/tmp/qemu-test/src/util/path.c: In function 'init_paths':
/tmp/qemu-test/src/util/path.c:24:5: error: 'g_canonicalize_filename' is deprecated: Not available before 2.58 [-Werror=deprecated-declarations]
     base = g_canonicalize_filename(prefix, NULL);
     ^~~~
In file included from /usr/x86_64-w64-mingw32/sys-root/mingw/include/glib-2.0/glib.h:48,
---
 gchar *g_canonicalize_filename (const gchar *filename,
        ^~~~~~~~~~~~~~~~~~~~~~~
/tmp/qemu-test/src/util/path.c: In function 'path':
/tmp/qemu-test/src/util/path.c:54:13: error: assignment discards 'const' qualifier from pointer target type [-Werror=discarded-qualifiers]
         ret = value ? value : name;
             ^
/tmp/qemu-test/src/util/path.c:60:9: error: 'g_canonicalize_filename' is deprecated: Not available before 2.58 [-Werror=deprecated-declarations]
         full_name = g_canonicalize_filename(g_path_skip_root(name), base);
         ^~~~~~~~~
In file included from /usr/x86_64-w64-mingw32/sys-root/mingw/include/glib-2.0/glib.h:48,
---
/usr/x86_64-w64-mingw32/sys-root/mingw/include/glib-2.0/glib/gfileutils.h:176:8: note: declared here
 gchar *g_canonicalize_filename (const gchar *filename,
        ^~~~~~~~~~~~~~~~~~~~~~~
/tmp/qemu-test/src/util/path.c:74:17: error: assignment discards 'const' qualifier from pointer target type [-Werror=discarded-qualifiers]
             ret = name;
                 ^
cc1: all warnings being treated as errors


The full log is available at
http://patchew.org/logs/20190417053225.27505-1-richard.henderson@linaro.org/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [Qemu-devel] [PATCH 1/1] util/path: Do not cache all filenames at startup

[Philippe Mathieu-Daudé]

Hi Richard, Daniel,

On 4/17/19 7:32 AM, Richard Henderson wrote:
> If one uses -L $PATH to point to a full chroot, the startup time
> is significant.  In addition, the existing probing algorithm fails
> to handle symlink loops.
> 
> Instead, probe individual paths on demand.  Cache both positive
> and negative results within $PATH, so that any one filename is
> probed only once.
> 
> Use glib filename functions for clarity.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  util/path.c | 211 ++++++++++++++--------------------------------------
>  1 file changed, 57 insertions(+), 154 deletions(-)
> 
> diff --git a/util/path.c b/util/path.c
> index 7f9fc272fb..09f75f100a 100644
> --- a/util/path.c
> +++ b/util/path.c
> @@ -8,170 +8,73 @@
>  #include <dirent.h>
>  #include "qemu/cutils.h"
>  #include "qemu/path.h"
> +#include "qemu/thread.h"
>  
> -struct pathelem
> -{
> -    /* Name of this, eg. lib */
> -    char *name;
> -    /* Full path name, eg. /usr/gnemul/x86-linux/lib. */
> -    char *pathname;
> -    struct pathelem *parent;
> -    /* Children */
> -    unsigned int num_entries;
> -    struct pathelem *entries[0];
> -};
> -
> -static struct pathelem *base;
> -
> -/* First N chars of S1 match S2, and S2 is N chars long. */
> -static int strneq(const char *s1, unsigned int n, const char *s2)
> -{
> -    unsigned int i;
> -
> -    for (i = 0; i < n; i++)
> -        if (s1[i] != s2[i])
> -            return 0;
> -    return s2[i] == 0;
> -}
> -
> -static struct pathelem *add_entry(struct pathelem *root, const char *name,
> -                                  unsigned type);
> -
> -static struct pathelem *new_entry(const char *root,
> -                                  struct pathelem *parent,
> -                                  const char *name)
> -{
> -    struct pathelem *new = g_malloc(sizeof(*new));
> -    new->name = g_strdup(name);
> -    new->pathname = g_strdup_printf("%s/%s", root, name);
> -    new->num_entries = 0;
> -    return new;
> -}
> -
> -#define streq(a,b) (strcmp((a), (b)) == 0)
> -
> -/* Not all systems provide this feature */
> -#if defined(DT_DIR) && defined(DT_UNKNOWN) && defined(DT_LNK)
> -# define dirent_type(dirent) ((dirent)->d_type)
> -# define is_dir_maybe(type) \
> -    ((type) == DT_DIR || (type) == DT_UNKNOWN || (type) == DT_LNK)
> -#else
> -# define dirent_type(dirent) (1)
> -# define is_dir_maybe(type)  (type)
> -#endif
> -
> -static struct pathelem *add_dir_maybe(struct pathelem *path)
> -{
> -    DIR *dir;
> -
> -    if ((dir = opendir(path->pathname)) != NULL) {
> -        struct dirent *dirent;
> -
> -        while ((dirent = readdir(dir)) != NULL) {
> -            if (!streq(dirent->d_name,".") && !streq(dirent->d_name,"..")){
> -                path = add_entry(path, dirent->d_name, dirent_type(dirent));
> -            }
> -        }
> -        closedir(dir);
> -    }
> -    return path;
> -}
> -
> -static struct pathelem *add_entry(struct pathelem *root, const char *name,
> -                                  unsigned type)
> -{
> -    struct pathelem **e;
> -
> -    root->num_entries++;
> -
> -    root = g_realloc(root, sizeof(*root)
> -                   + sizeof(root->entries[0])*root->num_entries);
> -    e = &root->entries[root->num_entries-1];
> -
> -    *e = new_entry(root->pathname, root, name);
> -    if (is_dir_maybe(type)) {
> -        *e = add_dir_maybe(*e);
> -    }
> -
> -    return root;
> -}
> -
> -/* This needs to be done after tree is stabilized (ie. no more reallocs!). */
> -static void set_parents(struct pathelem *child, struct pathelem *parent)
> -{
> -    unsigned int i;
> -
> -    child->parent = parent;
> -    for (i = 0; i < child->num_entries; i++)
> -        set_parents(child->entries[i], child);
> -}
> -
> -/* FIXME: Doesn't handle DIR/.. where DIR is not in emulated dir. */
> -static const char *
> -follow_path(const struct pathelem *cursor, const char *name)
> -{
> -    unsigned int i, namelen;
> -
> -    name += strspn(name, "/");
> -    namelen = strcspn(name, "/");
> -
> -    if (namelen == 0)
> -        return cursor->pathname;
> -
> -    if (strneq(name, namelen, ".."))
> -        return follow_path(cursor->parent, name + namelen);
> -
> -    if (strneq(name, namelen, "."))
> -        return follow_path(cursor, name + namelen);
> -
> -    for (i = 0; i < cursor->num_entries; i++)
> -        if (strneq(name, namelen, cursor->entries[i]->name))
> -            return follow_path(cursor->entries[i], name + namelen);
> -
> -    /* Not found */
> -    return NULL;
> -}
> +static const char *base;
> +static GHashTable *hash;
> +static QemuMutex lock;
>  
>  void init_paths(const char *prefix)
>  {
> -    char pref_buf[PATH_MAX];
> -
> -    if (prefix[0] == '\0' ||
> -        !strcmp(prefix, "/"))
> +    if (prefix[0] == '\0' || !strcmp(prefix, "/")) {
>          return;
> -
> -    if (prefix[0] != '/') {
> -        char *cwd = getcwd(NULL, 0);
> -        size_t pref_buf_len = sizeof(pref_buf);
> -
> -        if (!cwd)
> -            abort();
> -        pstrcpy(pref_buf, sizeof(pref_buf), cwd);
> -        pstrcat(pref_buf, pref_buf_len, "/");
> -        pstrcat(pref_buf, pref_buf_len, prefix);
> -        free(cwd);
> -    } else
> -        pstrcpy(pref_buf, sizeof(pref_buf), prefix + 1);
> -
> -    base = new_entry("", NULL, pref_buf);
> -    base = add_dir_maybe(base);
> -    if (base->num_entries == 0) {
> -        g_free(base->pathname);
> -        g_free(base->name);
> -        g_free(base);
> -        base = NULL;
> -    } else {
> -        set_parents(base, base);
>      }
> +
> +#if GLIB_CHECK_VERSION(2, 58, 0)

Should we raise GLIB_VERSION_MAX_ALLOWED in "glib-compat.h"?

Currently it is:

  /* 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

>From commit e71e8cc035558eabd6b3e19f6d3254c754c027ef:

 glib: enforce the minimum required version and warn about old APIs

 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.

> +    base = g_canonicalize_filename(prefix, NULL);
> +#else
> +    if (prefix[0] != '/') {
> +        char *cwd = g_get_current_dir();
> +        base = g_build_filename(cwd, prefix, NULL);
> +        g_free(cwd);
> +    } else {
> +        base = g_strdup(prefix);
> +    }
> +#endif
> +
> +    hash = g_hash_table_new(g_str_hash, g_str_equal);
> +    qemu_mutex_init(&lock);
>  }
>  
>  /* Look for path in emulation dir, otherwise return name. */
>  const char *path(const char *name)
>  {
> -    /* Only do absolute paths: quick and dirty, but should mostly be OK.
> -       Could do relative by tracking cwd. */
> -    if (!base || !name || name[0] != '/')
> -        return name;
> +    gpointer key, value;
> +    char *ret;
>  
> -    return follow_path(base, name) ?: name;
> +    /* Only do absolute paths: quick and dirty, but should mostly be OK.  */
> +    if (!base || !name || name[0] != '/') {
> +        return name;
> +    }
> +
> +    qemu_mutex_lock(&lock);
> +
> +    /* Have we looked up this file before?  */
> +    if (g_hash_table_lookup_extended(hash, name, &key, &value)) {
> +        ret = value ? value : name;
> +    } else {
> +        char *full_name, *save_name;
> +
> +        save_name = g_strdup(name);
> +#if GLIB_CHECK_VERSION(2, 58, 0)
> +        full_name = g_canonicalize_filename(g_path_skip_root(name), base);
> +#else
> +        full_name = g_build_filename(base, name, NULL);
> +#endif
> +
> +        /* Look for the path; record the result, pass or fail.  */
> +        if (access(full_name, F_OK) == 0) {
> +            /* Exists.  */
> +            g_hash_table_insert(hash, save_name, full_name);
> +            ret = full_name;
> +        } else {
> +            /* Does not exist.  */
> +            g_free(full_name);
> +            g_hash_table_insert(hash, save_name, NULL);
> +            ret = name;
> +        }
> +    }
> +
> +    qemu_mutex_unlock(&lock);
> +    return ret;
>  }
> 

Re: [Qemu-devel] [PATCH 1/1] util/path: Do not cache all filenames at startup

[Philippe Mathieu-Daudé]

Hi Richard, Daniel,

On 4/17/19 7:32 AM, Richard Henderson wrote:
> If one uses -L $PATH to point to a full chroot, the startup time
> is significant.  In addition, the existing probing algorithm fails
> to handle symlink loops.
> 
> Instead, probe individual paths on demand.  Cache both positive
> and negative results within $PATH, so that any one filename is
> probed only once.
> 
> Use glib filename functions for clarity.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  util/path.c | 211 ++++++++++++++--------------------------------------
>  1 file changed, 57 insertions(+), 154 deletions(-)
> 
> diff --git a/util/path.c b/util/path.c
> index 7f9fc272fb..09f75f100a 100644
> --- a/util/path.c
> +++ b/util/path.c
> @@ -8,170 +8,73 @@
>  #include <dirent.h>
>  #include "qemu/cutils.h"
>  #include "qemu/path.h"
> +#include "qemu/thread.h"
>  
> -struct pathelem
> -{
> -    /* Name of this, eg. lib */
> -    char *name;
> -    /* Full path name, eg. /usr/gnemul/x86-linux/lib. */
> -    char *pathname;
> -    struct pathelem *parent;
> -    /* Children */
> -    unsigned int num_entries;
> -    struct pathelem *entries[0];
> -};
> -
> -static struct pathelem *base;
> -
> -/* First N chars of S1 match S2, and S2 is N chars long. */
> -static int strneq(const char *s1, unsigned int n, const char *s2)
> -{
> -    unsigned int i;
> -
> -    for (i = 0; i < n; i++)
> -        if (s1[i] != s2[i])
> -            return 0;
> -    return s2[i] == 0;
> -}
> -
> -static struct pathelem *add_entry(struct pathelem *root, const char *name,
> -                                  unsigned type);
> -
> -static struct pathelem *new_entry(const char *root,
> -                                  struct pathelem *parent,
> -                                  const char *name)
> -{
> -    struct pathelem *new = g_malloc(sizeof(*new));
> -    new->name = g_strdup(name);
> -    new->pathname = g_strdup_printf("%s/%s", root, name);
> -    new->num_entries = 0;
> -    return new;
> -}
> -
> -#define streq(a,b) (strcmp((a), (b)) == 0)
> -
> -/* Not all systems provide this feature */
> -#if defined(DT_DIR) && defined(DT_UNKNOWN) && defined(DT_LNK)
> -# define dirent_type(dirent) ((dirent)->d_type)
> -# define is_dir_maybe(type) \
> -    ((type) == DT_DIR || (type) == DT_UNKNOWN || (type) == DT_LNK)
> -#else
> -# define dirent_type(dirent) (1)
> -# define is_dir_maybe(type)  (type)
> -#endif
> -
> -static struct pathelem *add_dir_maybe(struct pathelem *path)
> -{
> -    DIR *dir;
> -
> -    if ((dir = opendir(path->pathname)) != NULL) {
> -        struct dirent *dirent;
> -
> -        while ((dirent = readdir(dir)) != NULL) {
> -            if (!streq(dirent->d_name,".") && !streq(dirent->d_name,"..")){
> -                path = add_entry(path, dirent->d_name, dirent_type(dirent));
> -            }
> -        }
> -        closedir(dir);
> -    }
> -    return path;
> -}
> -
> -static struct pathelem *add_entry(struct pathelem *root, const char *name,
> -                                  unsigned type)
> -{
> -    struct pathelem **e;
> -
> -    root->num_entries++;
> -
> -    root = g_realloc(root, sizeof(*root)
> -                   + sizeof(root->entries[0])*root->num_entries);
> -    e = &root->entries[root->num_entries-1];
> -
> -    *e = new_entry(root->pathname, root, name);
> -    if (is_dir_maybe(type)) {
> -        *e = add_dir_maybe(*e);
> -    }
> -
> -    return root;
> -}
> -
> -/* This needs to be done after tree is stabilized (ie. no more reallocs!). */
> -static void set_parents(struct pathelem *child, struct pathelem *parent)
> -{
> -    unsigned int i;
> -
> -    child->parent = parent;
> -    for (i = 0; i < child->num_entries; i++)
> -        set_parents(child->entries[i], child);
> -}
> -
> -/* FIXME: Doesn't handle DIR/.. where DIR is not in emulated dir. */
> -static const char *
> -follow_path(const struct pathelem *cursor, const char *name)
> -{
> -    unsigned int i, namelen;
> -
> -    name += strspn(name, "/");
> -    namelen = strcspn(name, "/");
> -
> -    if (namelen == 0)
> -        return cursor->pathname;
> -
> -    if (strneq(name, namelen, ".."))
> -        return follow_path(cursor->parent, name + namelen);
> -
> -    if (strneq(name, namelen, "."))
> -        return follow_path(cursor, name + namelen);
> -
> -    for (i = 0; i < cursor->num_entries; i++)
> -        if (strneq(name, namelen, cursor->entries[i]->name))
> -            return follow_path(cursor->entries[i], name + namelen);
> -
> -    /* Not found */
> -    return NULL;
> -}
> +static const char *base;
> +static GHashTable *hash;
> +static QemuMutex lock;
>  
>  void init_paths(const char *prefix)
>  {
> -    char pref_buf[PATH_MAX];
> -
> -    if (prefix[0] == '\0' ||
> -        !strcmp(prefix, "/"))
> +    if (prefix[0] == '\0' || !strcmp(prefix, "/")) {
>          return;
> -
> -    if (prefix[0] != '/') {
> -        char *cwd = getcwd(NULL, 0);
> -        size_t pref_buf_len = sizeof(pref_buf);
> -
> -        if (!cwd)
> -            abort();
> -        pstrcpy(pref_buf, sizeof(pref_buf), cwd);
> -        pstrcat(pref_buf, pref_buf_len, "/");
> -        pstrcat(pref_buf, pref_buf_len, prefix);
> -        free(cwd);
> -    } else
> -        pstrcpy(pref_buf, sizeof(pref_buf), prefix + 1);
> -
> -    base = new_entry("", NULL, pref_buf);
> -    base = add_dir_maybe(base);
> -    if (base->num_entries == 0) {
> -        g_free(base->pathname);
> -        g_free(base->name);
> -        g_free(base);
> -        base = NULL;
> -    } else {
> -        set_parents(base, base);
>      }
> +
> +#if GLIB_CHECK_VERSION(2, 58, 0)

Should we raise GLIB_VERSION_MAX_ALLOWED in "glib-compat.h"?

Currently it is:

  /* 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

From commit e71e8cc035558eabd6b3e19f6d3254c754c027ef:

 glib: enforce the minimum required version and warn about old APIs

 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.

> +    base = g_canonicalize_filename(prefix, NULL);
> +#else
> +    if (prefix[0] != '/') {
> +        char *cwd = g_get_current_dir();
> +        base = g_build_filename(cwd, prefix, NULL);
> +        g_free(cwd);
> +    } else {
> +        base = g_strdup(prefix);
> +    }
> +#endif
> +
> +    hash = g_hash_table_new(g_str_hash, g_str_equal);
> +    qemu_mutex_init(&lock);
>  }
>  
>  /* Look for path in emulation dir, otherwise return name. */
>  const char *path(const char *name)
>  {
> -    /* Only do absolute paths: quick and dirty, but should mostly be OK.
> -       Could do relative by tracking cwd. */
> -    if (!base || !name || name[0] != '/')
> -        return name;
> +    gpointer key, value;
> +    char *ret;
>  
> -    return follow_path(base, name) ?: name;
> +    /* Only do absolute paths: quick and dirty, but should mostly be OK.  */
> +    if (!base || !name || name[0] != '/') {
> +        return name;
> +    }
> +
> +    qemu_mutex_lock(&lock);
> +
> +    /* Have we looked up this file before?  */
> +    if (g_hash_table_lookup_extended(hash, name, &key, &value)) {
> +        ret = value ? value : name;
> +    } else {
> +        char *full_name, *save_name;
> +
> +        save_name = g_strdup(name);
> +#if GLIB_CHECK_VERSION(2, 58, 0)
> +        full_name = g_canonicalize_filename(g_path_skip_root(name), base);
> +#else
> +        full_name = g_build_filename(base, name, NULL);
> +#endif
> +
> +        /* Look for the path; record the result, pass or fail.  */
> +        if (access(full_name, F_OK) == 0) {
> +            /* Exists.  */
> +            g_hash_table_insert(hash, save_name, full_name);
> +            ret = full_name;
> +        } else {
> +            /* Does not exist.  */
> +            g_free(full_name);
> +            g_hash_table_insert(hash, save_name, NULL);
> +            ret = name;
> +        }
> +    }
> +
> +    qemu_mutex_unlock(&lock);
> +    return ret;
>  }
> 

Re: [Qemu-devel] [PATCH 1/1] util/path: Do not cache all filenames at startup

[Daniel P. Berrangé]

On Tue, Apr 23, 2019 at 11:54:53AM +0200, Philippe Mathieu-Daudé wrote:
> Hi Richard, Daniel,
> 
> On 4/17/19 7:32 AM, Richard Henderson wrote:
> > If one uses -L $PATH to point to a full chroot, the startup time
> > is significant.  In addition, the existing probing algorithm fails
> > to handle symlink loops.
> > 
> > Instead, probe individual paths on demand.  Cache both positive
> > and negative results within $PATH, so that any one filename is
> > probed only once.
> > 
> > Use glib filename functions for clarity.
> > 
> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> > ---
> >  util/path.c | 211 ++++++++++++++--------------------------------------
> >  1 file changed, 57 insertions(+), 154 deletions(-)


> > +#if GLIB_CHECK_VERSION(2, 58, 0)
> 
> Should we raise GLIB_VERSION_MAX_ALLOWED in "glib-compat.h"?
> 
> Currently it is:
> 
>   /* 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

Use of 2.40 is set in accordance with our targetted platform support
policy. If Peter has stopped using the Ubuntu 14.04 build host, we
could bump it to 2.42. Once our dev tree opens up we could in fact
drop Jessie since we'll be supporting Buster by the time next QEMU
is released. That would still only take us upto perhaps Glib 2.48

Glib 2.58 is waaaay to new to rely on.

commit e7b3af81597db1a6b55f2c15d030d703c6b2c6ac
Author: Daniel P. Berrangé <berrange@redhat.com>
Date:   Fri May 4 15:34:46 2018 +0100

    glib: bump min required glib library version to 2.40
    
    Per supported platforms doc[1], 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
      SLE12-SP2: 2.48.2
      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.
    
    Docker jobs that not longer satisfy this new min version are removed.
    
    [1] https://qemu.weilnetz.de/doc/qemu-doc.html#Supported-build-platforms
    
    Reviewed-by: Thomas Huth <thuth@redhat.com>
    Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>


> 
> From commit e71e8cc035558eabd6b3e19f6d3254c754c027ef:
> 
>  glib: enforce the minimum required version and warn about old APIs
> 
>  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.
> 
> > +    base = g_canonicalize_filename(prefix, NULL);
> > +#else
> > +    if (prefix[0] != '/') {
> > +        char *cwd = g_get_current_dir();
> > +        base = g_build_filename(cwd, prefix, NULL);
> > +        g_free(cwd);
> > +    } else {
> > +        base = g_strdup(prefix);
> > +    }
> > +#endif

To use functionality from newer glib releases we can't put the #ifdef
conditionals inline in the main source files.

They have to be put into glib-compat.h instead. There's a detailed
comment in that file illustrating how todo this without triggering
the compile warnings about deprecations.


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 1/1] util/path: Do not cache all filenames at startup

[Daniel P. Berrangé]

On Tue, Apr 23, 2019 at 11:54:53AM +0200, Philippe Mathieu-Daudé wrote:
> Hi Richard, Daniel,
> 
> On 4/17/19 7:32 AM, Richard Henderson wrote:
> > If one uses -L $PATH to point to a full chroot, the startup time
> > is significant.  In addition, the existing probing algorithm fails
> > to handle symlink loops.
> > 
> > Instead, probe individual paths on demand.  Cache both positive
> > and negative results within $PATH, so that any one filename is
> > probed only once.
> > 
> > Use glib filename functions for clarity.
> > 
> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> > ---
> >  util/path.c | 211 ++++++++++++++--------------------------------------
> >  1 file changed, 57 insertions(+), 154 deletions(-)


> > +#if GLIB_CHECK_VERSION(2, 58, 0)
> 
> Should we raise GLIB_VERSION_MAX_ALLOWED in "glib-compat.h"?
> 
> Currently it is:
> 
>   /* 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

Use of 2.40 is set in accordance with our targetted platform support
policy. If Peter has stopped using the Ubuntu 14.04 build host, we
could bump it to 2.42. Once our dev tree opens up we could in fact
drop Jessie since we'll be supporting Buster by the time next QEMU
is released. That would still only take us upto perhaps Glib 2.48

Glib 2.58 is waaaay to new to rely on.

commit e7b3af81597db1a6b55f2c15d030d703c6b2c6ac
Author: Daniel P. Berrangé <berrange@redhat.com>
Date:   Fri May 4 15:34:46 2018 +0100

    glib: bump min required glib library version to 2.40
    
    Per supported platforms doc[1], 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
      SLE12-SP2: 2.48.2
      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.
    
    Docker jobs that not longer satisfy this new min version are removed.
    
    [1] https://qemu.weilnetz.de/doc/qemu-doc.html#Supported-build-platforms
    
    Reviewed-by: Thomas Huth <thuth@redhat.com>
    Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>


> 
> From commit e71e8cc035558eabd6b3e19f6d3254c754c027ef:
> 
>  glib: enforce the minimum required version and warn about old APIs
> 
>  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.
> 
> > +    base = g_canonicalize_filename(prefix, NULL);
> > +#else
> > +    if (prefix[0] != '/') {
> > +        char *cwd = g_get_current_dir();
> > +        base = g_build_filename(cwd, prefix, NULL);
> > +        g_free(cwd);
> > +    } else {
> > +        base = g_strdup(prefix);
> > +    }
> > +#endif

To use functionality from newer glib releases we can't put the #ifdef
conditionals inline in the main source files.

They have to be put into glib-compat.h instead. There's a detailed
comment in that file illustrating how todo this without triggering
the compile warnings about deprecations.


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 1/1] util/path: Do not cache all filenames at startup

[David Hildenbrand]

On 23.04.19 12:01, Daniel P. Berrangé wrote:
> On Tue, Apr 23, 2019 at 11:54:53AM +0200, Philippe Mathieu-Daudé wrote:
>> Hi Richard, Daniel,
>>
>> On 4/17/19 7:32 AM, Richard Henderson wrote:
>>> If one uses -L $PATH to point to a full chroot, the startup time
>>> is significant.  In addition, the existing probing algorithm fails
>>> to handle symlink loops.
>>>
>>> Instead, probe individual paths on demand.  Cache both positive
>>> and negative results within $PATH, so that any one filename is
>>> probed only once.
>>>
>>> Use glib filename functions for clarity.
>>>
>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>>> ---
>>>  util/path.c | 211 ++++++++++++++--------------------------------------
>>>  1 file changed, 57 insertions(+), 154 deletions(-)
> 
> 
>>> +#if GLIB_CHECK_VERSION(2, 58, 0)
>>
>> Should we raise GLIB_VERSION_MAX_ALLOWED in "glib-compat.h"?
>>
>> Currently it is:
>>
>>   /* 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
> 
> Use of 2.40 is set in accordance with our targetted platform support
> policy. If Peter has stopped using the Ubuntu 14.04 build host, we
> could bump it to 2.42. Once our dev tree opens up we could in fact
> drop Jessie since we'll be supporting Buster by the time next QEMU
> is released. That would still only take us upto perhaps Glib 2.48
> 
> Glib 2.58 is waaaay to new to rely on.
> 
> commit e7b3af81597db1a6b55f2c15d030d703c6b2c6ac
> Author: Daniel P. Berrangé <berrange@redhat.com>
> Date:   Fri May 4 15:34:46 2018 +0100
> 
>     glib: bump min required glib library version to 2.40
>     
>     Per supported platforms doc[1], 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
>       SLE12-SP2: 2.48.2
>       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.
>     
>     Docker jobs that not longer satisfy this new min version are removed.
>     
>     [1] https://qemu.weilnetz.de/doc/qemu-doc.html#Supported-build-platforms
>     
>     Reviewed-by: Thomas Huth <thuth@redhat.com>
>     Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> 
> 
>>
>> From commit e71e8cc035558eabd6b3e19f6d3254c754c027ef:
>>
>>  glib: enforce the minimum required version and warn about old APIs
>>
>>  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.
>>
>>> +    base = g_canonicalize_filename(prefix, NULL);
>>> +#else
>>> +    if (prefix[0] != '/') {
>>> +        char *cwd = g_get_current_dir();
>>> +        base = g_build_filename(cwd, prefix, NULL);
>>> +        g_free(cwd);
>>> +    } else {
>>> +        base = g_strdup(prefix);
>>> +    }
>>> +#endif
> 
> To use functionality from newer glib releases we can't put the #ifdef
> conditionals inline in the main source files.
> 
> They have to be put into glib-compat.h instead. There's a detailed
> comment in that file illustrating how todo this without triggering
> the compile warnings about deprecations.
> 
> 
> Regards,
> Daniel
> 

Trying to build (Richard's tcg-vec-next branch) on Fedora29 I get

util/path.c: In function 'init_paths':
util/path.c:24:5: error: 'g_canonicalize_filename' is deprecated: Not
available before 2.58 [-Werror=deprecated-declarations]
     base = g_canonicalize_filename(prefix, NULL);
     ^~~~
In file included from /usr/include/glib-2.0/glib.h:48,
                 from /home/dhildenb/git/qemu/include/glib-compat.h:32,
                 from /home/dhildenb/git/qemu/include/qemu/osdep.h:140,
                 from util/path.c:6:
/usr/include/glib-2.0/glib/gfileutils.h:176:8: note: declared here
 gchar *g_canonicalize_filename (const gchar *filename,
        ^~~~~~~~~~~~~~~~~~~~~~~
util/path.c: In function 'path':
util/path.c:60:9: error: 'g_canonicalize_filename' is deprecated: Not
available before 2.58 [-Werror=deprecated-declarations]
         full_name = g_canonicalize_filename(g_path_skip_root(name), base);
         ^~~~~~~~~
In file included from /usr/include/glib-2.0/glib.h:48,
                 from /home/dhildenb/git/qemu/include/glib-compat.h:32,
                 from /home/dhildenb/git/qemu/include/qemu/osdep.h:140,
                 from util/path.c:6:
/usr/include/glib-2.0/glib/gfileutils.h:176:8: note: declared here
 gchar *g_canonicalize_filename (const gchar *filename,
        ^~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
  CC      backends/rng-egd.o
make: *** [/home/dhildenb/git/qemu/rules.mak:69: util/path.o] Error 1
make: *** Waiting for unfinished jobs....
  CC      backends/rng-random.o


-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH 1/1] util/path: Do not cache all filenames at startup

[Daniel P. Berrangé]

On Tue, Apr 23, 2019 at 08:30:55PM +0200, David Hildenbrand wrote:
> On 23.04.19 12:01, Daniel P. Berrangé wrote:
> > On Tue, Apr 23, 2019 at 11:54:53AM +0200, Philippe Mathieu-Daudé wrote:
> >> Hi Richard, Daniel,
> >>
> >> On 4/17/19 7:32 AM, Richard Henderson wrote:
> >>> If one uses -L $PATH to point to a full chroot, the startup time
> >>> is significant.  In addition, the existing probing algorithm fails
> >>> to handle symlink loops.
> >>>
> >>> Instead, probe individual paths on demand.  Cache both positive
> >>> and negative results within $PATH, so that any one filename is
> >>> probed only once.
> >>>
> >>> Use glib filename functions for clarity.
> >>>
> >>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> >>> ---
> >>>  util/path.c | 211 ++++++++++++++--------------------------------------
> >>>  1 file changed, 57 insertions(+), 154 deletions(-)
> > 
> > 
> >>> +#if GLIB_CHECK_VERSION(2, 58, 0)
> >>
> >> Should we raise GLIB_VERSION_MAX_ALLOWED in "glib-compat.h"?
> >>
> >> Currently it is:
> >>
> >>   /* 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
> > 
> > Use of 2.40 is set in accordance with our targetted platform support
> > policy. If Peter has stopped using the Ubuntu 14.04 build host, we
> > could bump it to 2.42. Once our dev tree opens up we could in fact
> > drop Jessie since we'll be supporting Buster by the time next QEMU
> > is released. That would still only take us upto perhaps Glib 2.48
> > 
> > Glib 2.58 is waaaay to new to rely on.
> > 
> > commit e7b3af81597db1a6b55f2c15d030d703c6b2c6ac
> > Author: Daniel P. Berrangé <berrange@redhat.com>
> > Date:   Fri May 4 15:34:46 2018 +0100
> > 
> >     glib: bump min required glib library version to 2.40
> >     
> >     Per supported platforms doc[1], 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
> >       SLE12-SP2: 2.48.2
> >       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.
> >     
> >     Docker jobs that not longer satisfy this new min version are removed.
> >     
> >     [1] https://qemu.weilnetz.de/doc/qemu-doc.html#Supported-build-platforms
> >     
> >     Reviewed-by: Thomas Huth <thuth@redhat.com>
> >     Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > 
> > 
> >>
> >> From commit e71e8cc035558eabd6b3e19f6d3254c754c027ef:
> >>
> >>  glib: enforce the minimum required version and warn about old APIs
> >>
> >>  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.
> >>
> >>> +    base = g_canonicalize_filename(prefix, NULL);
> >>> +#else
> >>> +    if (prefix[0] != '/') {
> >>> +        char *cwd = g_get_current_dir();
> >>> +        base = g_build_filename(cwd, prefix, NULL);
> >>> +        g_free(cwd);
> >>> +    } else {
> >>> +        base = g_strdup(prefix);
> >>> +    }
> >>> +#endif
> > 
> > To use functionality from newer glib releases we can't put the #ifdef
> > conditionals inline in the main source files.
> > 
> > They have to be put into glib-compat.h instead. There's a detailed
> > comment in that file illustrating how todo this without triggering
> > the compile warnings about deprecations.
> > 
> > 
> > Regards,
> > Daniel
> > 
> 
> Trying to build (Richard's tcg-vec-next branch) on Fedora29 I get
> 
> util/path.c: In function 'init_paths':
> util/path.c:24:5: error: 'g_canonicalize_filename' is deprecated: Not
> available before 2.58 [-Werror=deprecated-declarations]
>      base = g_canonicalize_filename(prefix, NULL);
>      ^~~~

[snip]

This is exactly why the compat code needs to go into glib-compat.h.
There is a special mechanism in that file for avoiding triggering
the deprecation warnings from glib.

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 1/1] util/path: Do not cache all filenames at startup

[Daniel P. Berrangé]

On Tue, Apr 23, 2019 at 08:30:55PM +0200, David Hildenbrand wrote:
> On 23.04.19 12:01, Daniel P. Berrangé wrote:
> > On Tue, Apr 23, 2019 at 11:54:53AM +0200, Philippe Mathieu-Daudé wrote:
> >> Hi Richard, Daniel,
> >>
> >> On 4/17/19 7:32 AM, Richard Henderson wrote:
> >>> If one uses -L $PATH to point to a full chroot, the startup time
> >>> is significant.  In addition, the existing probing algorithm fails
> >>> to handle symlink loops.
> >>>
> >>> Instead, probe individual paths on demand.  Cache both positive
> >>> and negative results within $PATH, so that any one filename is
> >>> probed only once.
> >>>
> >>> Use glib filename functions for clarity.
> >>>
> >>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> >>> ---
> >>>  util/path.c | 211 ++++++++++++++--------------------------------------
> >>>  1 file changed, 57 insertions(+), 154 deletions(-)
> > 
> > 
> >>> +#if GLIB_CHECK_VERSION(2, 58, 0)
> >>
> >> Should we raise GLIB_VERSION_MAX_ALLOWED in "glib-compat.h"?
> >>
> >> Currently it is:
> >>
> >>   /* 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
> > 
> > Use of 2.40 is set in accordance with our targetted platform support
> > policy. If Peter has stopped using the Ubuntu 14.04 build host, we
> > could bump it to 2.42. Once our dev tree opens up we could in fact
> > drop Jessie since we'll be supporting Buster by the time next QEMU
> > is released. That would still only take us upto perhaps Glib 2.48
> > 
> > Glib 2.58 is waaaay to new to rely on.
> > 
> > commit e7b3af81597db1a6b55f2c15d030d703c6b2c6ac
> > Author: Daniel P. Berrangé <berrange@redhat.com>
> > Date:   Fri May 4 15:34:46 2018 +0100
> > 
> >     glib: bump min required glib library version to 2.40
> >     
> >     Per supported platforms doc[1], 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
> >       SLE12-SP2: 2.48.2
> >       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.
> >     
> >     Docker jobs that not longer satisfy this new min version are removed.
> >     
> >     [1] https://qemu.weilnetz.de/doc/qemu-doc.html#Supported-build-platforms
> >     
> >     Reviewed-by: Thomas Huth <thuth@redhat.com>
> >     Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > 
> > 
> >>
> >> From commit e71e8cc035558eabd6b3e19f6d3254c754c027ef:
> >>
> >>  glib: enforce the minimum required version and warn about old APIs
> >>
> >>  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.
> >>
> >>> +    base = g_canonicalize_filename(prefix, NULL);
> >>> +#else
> >>> +    if (prefix[0] != '/') {
> >>> +        char *cwd = g_get_current_dir();
> >>> +        base = g_build_filename(cwd, prefix, NULL);
> >>> +        g_free(cwd);
> >>> +    } else {
> >>> +        base = g_strdup(prefix);
> >>> +    }
> >>> +#endif
> > 
> > To use functionality from newer glib releases we can't put the #ifdef
> > conditionals inline in the main source files.
> > 
> > They have to be put into glib-compat.h instead. There's a detailed
> > comment in that file illustrating how todo this without triggering
> > the compile warnings about deprecations.
> > 
> > 
> > Regards,
> > Daniel
> > 
> 
> Trying to build (Richard's tcg-vec-next branch) on Fedora29 I get
> 
> util/path.c: In function 'init_paths':
> util/path.c:24:5: error: 'g_canonicalize_filename' is deprecated: Not
> available before 2.58 [-Werror=deprecated-declarations]
>      base = g_canonicalize_filename(prefix, NULL);
>      ^~~~

[snip]

This is exactly why the compat code needs to go into glib-compat.h.
There is a special mechanism in that file for avoiding triggering
the deprecation warnings from glib.

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