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