On Mon, Sep 09, 2019 at 12:38:25PM +0200, Pavel Hrdina wrote:
> 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.
Yes, of course. Dunno how I missed that :-(
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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
