On 03/24/2014 02:54 PM, Eric Blake wrote:
On 03/23/2014 02:22 AM, Nehal J Wani wrote:
When libvirtd is run from a build directory without being installed, it should not depend on files from a libvirt package installed in the system. Currently, APIs defined in src/ don't know whether libvirtd is being run from the build dir or the installed dir. The following additions provide the functionality to do so:
src/util/virutil.c *virSetUninstalledDir *virGetUninstalledDir
Example usage (utility = lxc|iohelper): char *path_tmp = virGetUninstalledDir(); if (path_tmp) /* do stuff with ("%s/../../src/libvirt_<utility>", path_tmp) */ else /* do stuff with (LIBEXECDIR "/libvirt_<utility>") */
Hmm - we already have virFDStreamSetIOHelper() for overriding the location of libvirt_iohelper, but I like your approach as something a bit more generic. Do we really need both mechanisms, or should your series include a followup patch to nuke virFDStreamSetIOHelper() and instead teach the testsuite to use your new functions (that is, testutils.c should probably call virSetUninstalledDir).
A bit more to help others not following the IRC conversation. As it is, virFDStreamSetIOHelper() is not as useful as it could be. Observe this 'git grep' result: ./src/lxc/lxc_conf.c:95: LIBEXECDIR "/libvirt_lxc", ./src/lxc/lxc_conf.c:114: LIBEXECDIR "/libvirt_lxc", ./src/fdstream.c:81:static const char *iohelper_path = LIBEXECDIR "/libvirt_iohelper"; ./src/fdstream.c:86: iohelper_path = LIBEXECDIR "/libvirt_iohelper"; ./src/util/virfile.c:239: ret->cmd = virCommandNewArgList(LIBEXECDIR "/libvirt_iohelper", ./src/storage/storage_backend_disk.c:43:#define PARTHELPER LIBEXECDIR "/libvirt_parthelper" Note that the testsuite knows how to run uninstalled libvirt_iohelper when testing fdstream.c directly, but also note that virfile.c uses libvirt_iohelper but never uses its overridden location - our existing override is only half-baked. And even though we have the override hook for iohelper, daemon/libvirt.c main() is not taking advantage of the hook - which means if you build libvirt.git on a system where libvirt is not installed, and try to use ./run to test your just-built binary, it will fail anywhere it tries to use libvirt_iohelper (because it will be trying to use the installed location, instead of the just-built in-tree location). So I'm definitely in favor of this patch, but think it needs more patches in the series before it is ready to push.
Series is incomplete. It's not worth adding the new function unless you also have followup patches that use the new function.
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org