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 <address@hidden>
>>> ---
>>>  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é <address@hidden>
> 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 <address@hidden>
>     Signed-off-by: Daniel P. Berrangé <address@hidden>
> 
> 
>>
>> 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