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

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

 



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




[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