[libvirt] [PATCH 0/2] Fix virFileFindResource in VPATH
Jiri Denemark (2): virFileFindResource: Add support for looking in source tree Search for schemas and cpu_map.xml in source tree src/Makefile.am | 2 ++ src/conf/domain_conf.c | 6 ++--- src/cpu/cpu_map.c | 6 ++--- src/driver.c | 1 + src/libvirt_private.syms | 1 + src/locking/lock_driver_lockd.c | 1 + src/locking/lock_manager.c | 1 + src/remote/remote_driver.c | 1 + src/util/virfile.c | 49 +++++++++++++++++++++++++++++++++-------- src/util/virfile.h | 12 +++++++++- 10 files changed, 64 insertions(+), 16 deletions(-) -- 2.3.0
Not all file we want to find using virFileFindResource{,Full} are generated when libvirt is built, some of them (such as RNG schemas) are distributed with sources. The current API was not able to find source files if libvirt was built in VPATH. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/Makefile.am | 2 ++ src/driver.c | 1 + src/locking/lock_driver_lockd.c | 1 + src/locking/lock_manager.c | 1 + src/remote/remote_driver.c | 1 + src/util/virfile.c | 39 ++++++++++++++++++++++++++++++--------- src/util/virfile.h | 8 +++++++- 7 files changed, 43 insertions(+), 10 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index b41c6d4..a938d7e 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -20,6 +20,7 @@ abs_builddir = $(shell pwd) abs_topbuilddir = $(shell cd .. && pwd) abs_srcdir = $(shell cd $(srcdir) && pwd) +abs_topsrcdir = $(shell cd $(srcdir)/.. && pwd) # No libraries with the exception of LIBXML should be listed # here. List them against the individual XXX_la_CFLAGS targets @@ -32,6 +33,7 @@ INCLUDES = -I../gnulib/lib \ -I$(srcdir)/util \ -DIN_LIBVIRT \ -Dabs_topbuilddir="\"$(abs_topbuilddir)\"" \ + -Dabs_topsrcdir="\"$(abs_topsrcdir)\"" \ $(GETTEXT_CPPFLAGS) AM_CFLAGS = $(LIBXML_CFLAGS) \ diff --git a/src/driver.c b/src/driver.c index 1be32ef..1271597 100644 --- a/src/driver.c +++ b/src/driver.c @@ -57,6 +57,7 @@ virDriverLoadModule(const char *name) "libvirt_driver_", ".so", "src/.libs", + VIR_FILE_FIND_RESOURCE_VPATH_BUILD, LIBDIR "/libvirt/connection-driver", "LIBVIRT_DRIVER_DIR"))) return NULL; diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c index 2af3f22..53a7256 100644 --- a/src/locking/lock_driver_lockd.c +++ b/src/locking/lock_driver_lockd.c @@ -254,6 +254,7 @@ static virNetClientPtr virLockManagerLockDaemonConnectionNew(bool privileged, !(daemonPath = virFileFindResourceFull("virtlockd", NULL, NULL, "src", + VIR_FILE_FIND_RESOURCE_VPATH_BUILD, SBINDIR, "VIRTLOCKD_PATH"))) goto error; diff --git a/src/locking/lock_manager.c b/src/locking/lock_manager.c index ec90d04..554e9e2 100644 --- a/src/locking/lock_manager.c +++ b/src/locking/lock_manager.c @@ -143,6 +143,7 @@ virLockManagerPluginPtr virLockManagerPluginNew(const char *name, NULL, ".so", "src/.libs", + VIR_FILE_FIND_RESOURCE_VPATH_BUILD, LIBDIR "/libvirt/lock-driver", "LIBVIRT_LOCK_MANAGER_PLUGIN_DIR"))) goto cleanup; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index d4fd658..4830a48 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -888,6 +888,7 @@ doRemoteOpen(virConnectPtr conn, !(daemonPath = virFileFindResourceFull("libvirtd", NULL, NULL, "daemon", + VIR_FILE_FIND_RESOURCE_VPATH_BUILD, SBINDIR, "LIBVIRTD_PATH"))) goto failed; diff --git a/src/util/virfile.c b/src/util/virfile.c index 1b004d6..e60896f 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -1618,7 +1618,8 @@ static bool useDirOverride; * @filename: libvirt distributed filename without any path * @prefix: optional string to prepend to filename * @suffix: optional string to append to filename - * @builddir: location of the binary in the source tree build tree + * @builddir: location of the filename in the build tree + * @vpath: switch between source and build directory in VPATH build * @installdir: location of the installed binary * @envname: environment variable used to override all dirs * @@ -1627,8 +1628,14 @@ static bool useDirOverride; * run from the source tree. Otherwise it will return the * path in the installed location. * + * If libvirt was built in VPATH, @vpath switches between source and build + * directory when useDirOverride is in effect. If @vpath is + * VIR_FILE_FIND_RESOURCE_VPATH_BUILD, @builddir refers to the build + * directory; VIR_FILE_FIND_RESOURCE_VPATH_SOURCE says @builddir is a + * subdirectory of the source directory. + * * If @envname is non-NULL it will override all other - * directory lookup + * directory lookup. * * Only use this with @filename files that are part of * the libvirt tree, not 3rd party binaries/files. @@ -1640,11 +1647,14 @@ virFileFindResourceFull(const char *filename, const char *prefix, const char *suffix, const char *builddir, + virFileFindResourceVPath vpath, const char *installdir, const char *envname) { char *ret = NULL; const char *envval = envname ? virGetEnvBlockSUID(envname) : NULL; + const char *base = ""; + const char *path = ""; if (!prefix) prefix = ""; @@ -1652,16 +1662,25 @@ virFileFindResourceFull(const char *filename, suffix = ""; if (envval) { - if (virAsprintf(&ret, "%s/%s%s%s", envval, prefix, filename, suffix) < 0) - return NULL; + base = envval; } else if (useDirOverride) { - if (virAsprintf(&ret, "%s/%s/%s%s%s", abs_topbuilddir, builddir, prefix, filename, suffix) < 0) - return NULL; + switch (vpath) { + case VIR_FILE_FIND_RESOURCE_VPATH_BUILD: + base = abs_topbuilddir; + break; + case VIR_FILE_FIND_RESOURCE_VPATH_SOURCE: + base = abs_topsrcdir; + break; + } + path = builddir; } else { - if (virAsprintf(&ret, "%s/%s%s%s", installdir, prefix, filename, suffix) < 0) - return NULL; + base = installdir; } + if (virAsprintf(&ret, "%s%s%s/%s%s%s", + base, *path ? "/" : "", path, prefix, filename, suffix) < 0) + return NULL; + VIR_DEBUG("Resolved '%s' to '%s'", filename, ret); return ret; } @@ -1671,7 +1690,9 @@ virFileFindResource(const char *filename, const char *builddir, const char *installdir) { - return virFileFindResourceFull(filename, NULL, NULL, builddir, installdir, NULL); + return virFileFindResourceFull(filename, NULL, NULL, + builddir, VIR_FILE_FIND_RESOURCE_VPATH_BUILD, + installdir, NULL); } diff --git a/src/util/virfile.h b/src/util/virfile.h index 403d0ba..0e481c2 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -163,6 +163,11 @@ int virFileIsLink(const char *linkpath) char *virFindFileInPath(const char *file); +typedef enum { + VIR_FILE_FIND_RESOURCE_VPATH_BUILD, + VIR_FILE_FIND_RESOURCE_VPATH_SOURCE +} virFileFindResourceVPath; + char *virFileFindResource(const char *filename, const char *builddir, const char *installdir) @@ -171,9 +176,10 @@ char *virFileFindResourceFull(const char *filename, const char *prefix, const char *suffix, const char *builddir, + virFileFindResourceVPath vpath, const char *installdir, const char *envname) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5); + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(6); void virFileActivateDirOverride(const char *argv0) ATTRIBUTE_NONNULL(1); -- 2.3.0
On Mon, Feb 16, 2015 at 03:11:29PM +0100, Jiri Denemark wrote:
Not all file we want to find using virFileFindResource{,Full} are generated when libvirt is built, some of them (such as RNG schemas) are distributed with sources. The current API was not able to find source files if libvirt was built in VPATH.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/Makefile.am | 2 ++ src/driver.c | 1 + src/locking/lock_driver_lockd.c | 1 + src/locking/lock_manager.c | 1 + src/remote/remote_driver.c | 1 + src/util/virfile.c | 39 ++++++++++++++++++++++++++++++--------- src/util/virfile.h | 8 +++++++- 7 files changed, 43 insertions(+), 10 deletions(-)
diff --git a/src/Makefile.am b/src/Makefile.am index b41c6d4..a938d7e 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -20,6 +20,7 @@ abs_builddir = $(shell pwd) abs_topbuilddir = $(shell cd .. && pwd) abs_srcdir = $(shell cd $(srcdir) && pwd) +abs_topsrcdir = $(shell cd $(srcdir)/.. && pwd)
# No libraries with the exception of LIBXML should be listed # here. List them against the individual XXX_la_CFLAGS targets @@ -32,6 +33,7 @@ INCLUDES = -I../gnulib/lib \ -I$(srcdir)/util \ -DIN_LIBVIRT \ -Dabs_topbuilddir="\"$(abs_topbuilddir)\"" \ + -Dabs_topsrcdir="\"$(abs_topsrcdir)\"" \ $(GETTEXT_CPPFLAGS)
AM_CFLAGS = $(LIBXML_CFLAGS) \ diff --git a/src/driver.c b/src/driver.c index 1be32ef..1271597 100644 --- a/src/driver.c +++ b/src/driver.c @@ -57,6 +57,7 @@ virDriverLoadModule(const char *name) "libvirt_driver_", ".so", "src/.libs", + VIR_FILE_FIND_RESOURCE_VPATH_BUILD,
Instead of adding yet another parameter, why don't we just change the calling convention of virFileFindResource so that instead of assuming abs_topbuilddir we just make the caller prepend the builddir. eg instead of pasing 'src/.libs' we pass abs_topbuilddir "/src/.libs" letting the compiler concatenate these. The enum you define is basically doing exactly that but with an extra level of indirection which doesn't seem to buy us anything
@@ -1640,11 +1647,14 @@ virFileFindResourceFull(const char *filename, const char *prefix, const char *suffix, const char *builddir, + virFileFindResourceVPath vpath, const char *installdir, const char *envname) { char *ret = NULL; const char *envval = envname ? virGetEnvBlockSUID(envname) : NULL; + const char *base = ""; + const char *path = "";
if (!prefix) prefix = ""; @@ -1652,16 +1662,25 @@ virFileFindResourceFull(const char *filename, suffix = "";
if (envval) { - if (virAsprintf(&ret, "%s/%s%s%s", envval, prefix, filename, suffix) < 0) - return NULL; + base = envval; } else if (useDirOverride) { - if (virAsprintf(&ret, "%s/%s/%s%s%s", abs_topbuilddir, builddir, prefix, filename, suffix) < 0) - return NULL;
eg, simply change this to be if (virAsprintf(&ret, "%s/%s/%s%s%s", builddir, prefix, filename, suffix) < 0) return NULL; Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Mon, Feb 16, 2015 at 14:26:56 +0000, Daniel P. Berrange wrote:
On Mon, Feb 16, 2015 at 03:11:29PM +0100, Jiri Denemark wrote:
Not all file we want to find using virFileFindResource{,Full} are generated when libvirt is built, some of them (such as RNG schemas) are distributed with sources. The current API was not able to find source files if libvirt was built in VPATH.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/Makefile.am | 2 ++ src/driver.c | 1 + src/locking/lock_driver_lockd.c | 1 + src/locking/lock_manager.c | 1 + src/remote/remote_driver.c | 1 + src/util/virfile.c | 39 ++++++++++++++++++++++++++++++--------- src/util/virfile.h | 8 +++++++- 7 files changed, 43 insertions(+), 10 deletions(-)
diff --git a/src/Makefile.am b/src/Makefile.am index b41c6d4..a938d7e 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -20,6 +20,7 @@ abs_builddir = $(shell pwd) abs_topbuilddir = $(shell cd .. && pwd) abs_srcdir = $(shell cd $(srcdir) && pwd) +abs_topsrcdir = $(shell cd $(srcdir)/.. && pwd)
# No libraries with the exception of LIBXML should be listed # here. List them against the individual XXX_la_CFLAGS targets @@ -32,6 +33,7 @@ INCLUDES = -I../gnulib/lib \ -I$(srcdir)/util \ -DIN_LIBVIRT \ -Dabs_topbuilddir="\"$(abs_topbuilddir)\"" \ + -Dabs_topsrcdir="\"$(abs_topsrcdir)\"" \ $(GETTEXT_CPPFLAGS)
AM_CFLAGS = $(LIBXML_CFLAGS) \ diff --git a/src/driver.c b/src/driver.c index 1be32ef..1271597 100644 --- a/src/driver.c +++ b/src/driver.c @@ -57,6 +57,7 @@ virDriverLoadModule(const char *name) "libvirt_driver_", ".so", "src/.libs", + VIR_FILE_FIND_RESOURCE_VPATH_BUILD,
Instead of adding yet another parameter, why don't we just change the calling convention of virFileFindResource so that instead of assuming abs_topbuilddir we just make the caller prepend the builddir.
eg instead of pasing 'src/.libs' we pass
abs_topbuilddir "/src/.libs"
letting the compiler concatenate these. The enum you define is basically doing exactly that but with an extra level of indirection which doesn't seem to buy us anything
Yeah, I guess, that would work too, although it will either need one more allocation or making all calls to the non-Full variant of this API more complex. But we can live with it since virFileFind* APIs are used at about 11 places... Jirka
Both RNG schemas and cpu_map.xml are distributed in source tarball. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/conf/domain_conf.c | 6 +++--- src/cpu/cpu_map.c | 6 +++--- src/libvirt_private.syms | 1 + src/util/virfile.c | 10 ++++++++++ src/util/virfile.h | 4 ++++ 5 files changed, 21 insertions(+), 6 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b13cae8..b3d63f8 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -12886,9 +12886,9 @@ virDomainDefParseXML(xmlDocPtr xml, bool primaryVideo = false; if (flags & VIR_DOMAIN_DEF_PARSE_VALIDATE) { - char *schema = virFileFindResource("domain.rng", - "docs/schemas", - PKGDATADIR "/schemas"); + char *schema = virFileFindResourceSrc("domain.rng", + "docs/schemas", + PKGDATADIR "/schemas"); if (!schema) return NULL; if (virXMLValidateAgainstSchema(schema, xml) < 0) { diff --git a/src/cpu/cpu_map.c b/src/cpu/cpu_map.c index b77f688..8c005d4 100644 --- a/src/cpu/cpu_map.c +++ b/src/cpu/cpu_map.c @@ -86,9 +86,9 @@ int cpuMapLoad(const char *arch, int element; char *mapfile; - if (!(mapfile = virFileFindResource("cpu_map.xml", - "src/cpu", - PKGDATADIR))) + if (!(mapfile = virFileFindResourceSrc("cpu_map.xml", + "src/cpu", + PKGDATADIR))) return -1; VIR_DEBUG("Loading CPU map from %s", mapfile); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c07a561..488cde6 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1346,6 +1346,7 @@ virFileFindHugeTLBFS; virFileFindMountPoint; virFileFindResource; virFileFindResourceFull; +virFileFindResourceSrc; virFileGetHugepageSize; virFileGetMountReverseSubtree; virFileGetMountSubtree; diff --git a/src/util/virfile.c b/src/util/virfile.c index e60896f..fc3d0a2 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -1695,6 +1695,16 @@ virFileFindResource(const char *filename, installdir, NULL); } +char * +virFileFindResourceSrc(const char *filename, + const char *srcdir, + const char *installdir) +{ + return virFileFindResourceFull(filename, NULL, NULL, + srcdir, VIR_FILE_FIND_RESOURCE_VPATH_SOURCE, + installdir, NULL); +} + /** * virFileActivateDirOverride: diff --git a/src/util/virfile.h b/src/util/virfile.h index 0e481c2..ddce797 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -172,6 +172,10 @@ char *virFileFindResource(const char *filename, const char *builddir, const char *installdir) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); +char *virFileFindResourceSrc(const char *filename, + const char *srcdir, + const char *installdir) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); char *virFileFindResourceFull(const char *filename, const char *prefix, const char *suffix, -- 2.3.0
participants (2)
-
Daniel P. Berrange -
Jiri Denemark