Re: [PATCH v2] util: activate directory override when used from library

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Sep 05, 2019 at 11:52:09AM +0100, Daniel P. Berrangé wrote:
> The Perl bindings for libvirt use the test driver for unit tests. This
> tries to load the cpu_map/index.xml file, and when run from an
> uninstalled build will fail.
> 
> The problem is that virFileActivateDirOverride is called by our various
> binaries like libvirtd, virsh, but is not called when a 3rd party app
> uses libvirt.so
> 
> To deal with this we allow the LIBVIRT_DIR_OVERRIDE=1 env variable to be
> set and make virInitialize look for this. The 'run' script will set it,
> so now build using this script to run against an uninstalled tree we
> will correctly resolve files to the source tree.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx>
> ---
> Changed in v2:
> 
>  - Use a separate function for env var override
> 
>  run.in                        |  7 ++++++
>  src/libvirt.c                 |  2 ++
>  src/libvirt_private.syms      |  3 ++-
>  src/locking/lock_daemon.c     |  2 +-
>  src/logging/log_daemon.c      |  2 +-
>  src/remote/remote_daemon.c    |  2 +-
>  src/security/virt-aa-helper.c |  2 +-
>  src/util/virfile.c            | 40 ++++++++++++++++++++++++++---------
>  src/util/virfile.h            |  3 ++-
>  tests/testutils.c             |  2 +-
>  tools/virsh.c                 |  2 +-
>  tools/virt-admin.c            |  2 +-
>  12 files changed, 50 insertions(+), 19 deletions(-)
> 
> diff --git a/run.in b/run.in
> index 9a1c6ea11a..ed946a0899 100644
> --- a/run.in
> +++ b/run.in
> @@ -60,6 +60,13 @@ else
>  fi
>  export PKG_CONFIG_PATH
>  
> +# Ensure that any 3rd party apps using libvirt.so from
> +# the build tree get files resolved to the build/source
> +# tree too. Typically useful for language bindings
> +# running tests against non-installed libvirt.
> +LIBVIRT_DIR_OVERRIDE=1
> +export LIBVIRT_DIR_OVERRIDE
> +

This line wrapping looks a bit to aggressive :).

>  # This is a cheap way to find some use-after-free and uninitialized
>  # read problems when using glibc.
>  random_val="$(awk 'BEGIN{srand(); print 1+int(255*rand())}' < /dev/null)"
> diff --git a/src/libvirt.c b/src/libvirt.c
> index 9650aaa453..a1ecd8bacc 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c
> @@ -247,6 +247,8 @@ virGlobalInit(void)
>          virErrorInitialize() < 0)
>          goto error;
>  
> +    virFileActivateDirOverrideForLib();
> +
>      if (getuid() != geteuid() ||
>          getgid() != getegid()) {
>          virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index a34d92f5ef..21cd6a8ef7 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1917,7 +1917,8 @@ virDirOpenQuiet;
>  virDirRead;
>  virFileAbsPath;
>  virFileAccessibleAs;
> -virFileActivateDirOverride;
> +virFileActivateDirOverrideForLib;
> +virFileActivateDirOverrideForProg;
>  virFileBindMountDevice;
>  virFileBuildPath;
>  virFileCanonicalizePath;
> diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
> index ac242bf65c..edb61d74c0 100644
> --- a/src/locking/lock_daemon.c
> +++ b/src/locking/lock_daemon.c
> @@ -1188,7 +1188,7 @@ int main(int argc, char **argv) {
>          }
>      }
>  
> -    virFileActivateDirOverride(argv[0]);
> +    virFileActivateDirOverrideForProg(argv[0]);
>  
>      if (!(config = virLockDaemonConfigNew(privileged))) {
>          VIR_ERROR(_("Can't create initial configuration"));
> diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c
> index f74e900aea..85c58aaa7b 100644
> --- a/src/logging/log_daemon.c
> +++ b/src/logging/log_daemon.c
> @@ -960,7 +960,7 @@ int main(int argc, char **argv) {
>          }
>      }
>  
> -    virFileActivateDirOverride(argv[0]);
> +    virFileActivateDirOverrideForProg(argv[0]);
>  
>      if (!(config = virLogDaemonConfigNew(privileged))) {
>          VIR_ERROR(_("Can't create initial configuration"));
> diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c
> index 546328b24d..8cf9412128 100644
> --- a/src/remote/remote_daemon.c
> +++ b/src/remote/remote_daemon.c
> @@ -1034,7 +1034,7 @@ int main(int argc, char **argv) {
>  
>      virUpdateSelfLastChanged(argv[0]);
>  
> -    virFileActivateDirOverride(argv[0]);
> +    virFileActivateDirOverrideForProg(argv[0]);
>  
>      while (1) {
>          int optidx = 0;
> diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
> index 3f9d775acc..326cfaf52a 100644
> --- a/src/security/virt-aa-helper.c
> +++ b/src/security/virt-aa-helper.c
> @@ -1427,7 +1427,7 @@ main(int argc, char **argv)
>          exit(EXIT_FAILURE);
>      }
>  
> -    virFileActivateDirOverride(argv[0]);
> +    virFileActivateDirOverrideForProg(argv[0]);
>  
>      /* Initialize the log system */
>      virLogSetFromEnv();
> diff --git a/src/util/virfile.c b/src/util/virfile.c
> index 81a3c096eb..d9ac482435 100644
> --- a/src/util/virfile.c
> +++ b/src/util/virfile.c
> @@ -1766,7 +1766,7 @@ virFileFindResource(const char *filename,
>  
>  
>  /**
> - * virFileActivateDirOverride:
> + * virFileActivateDirOverrideForProg:
>   * @argv0: argv[0] of the calling program
>   *
>   * Look at @argv0 and try to detect if running from
> @@ -1774,20 +1774,40 @@ virFileFindResource(const char *filename,
>   * on the binary name, or '/.libs/' in the path
>   */
>  void
> -virFileActivateDirOverride(const char *argv0)
> +virFileActivateDirOverrideForProg(const char *argv0)
>  {
> -    char *file = strrchr(argv0, '/');
> -    if (!file || file[1] == '\0')
> -        return;
> -    file++;
> -    if (STRPREFIX(file, "lt-") ||
> -        strstr(argv0, "/.libs/")) {
> -        useDirOverride = true;
> -        VIR_DEBUG("Activating build dir override for %s", argv0);
> +    if (argv0 == NULL) {
> +        if (getenv("LIBVIRT_DIR_OVERRIDE") != NULL)
> +            useDirOverride = true;
> +    } else {
> +        char *file = strrchr(argv0, '/');
> +        if (!file || file[1] == '\0')
> +            return;
> +        file++;
> +        if (STRPREFIX(file, "lt-") ||
> +            strstr(argv0, "/.libs/")) {
> +            useDirOverride = true;
> +            VIR_DEBUG("Activating build dir override for %s", argv0);
> +        }

This doesn't look right, probably leftover from V1 as this function
should not be modified now with a new separate function for Lib.

>      }
>  }
>  
>  
> +/**
> + * virFileActivateDirOverrideForEnv:

s/ForEnv/ForLib/

> + *
> + * Look for LIBVIRT_DIR_OVERRIDE env var to see if
> + * we should find files from the build/src tree
> + * instead of install tree
> + */

Wrapping looks weird to me again and there is a missing dot at the end
of the sentence.

Reviewed-by: Pavel Hrdina <phrdina@xxxxxxxxxx>

Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux