[libvirt] [PATCH] Add utility functions for storing uninstalled location

[Eric Blake]

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 604 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140324/ca540c7e/attachment-0001.sig>