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

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(a)redhat.com&gt; > --- > 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.