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
