[libvirt] [PATCH v4] Add helper program to create custom leases

[Eric Blake]

On 03/21/2014 12:12 AM, Nehal J Wani wrote:
>>> @@ -1120,6 +1135,10 @@ networkBuildDhcpDaemonCommandLine(virNetworkObjPtr network,
>>>
>>>      cmd = virCommandNew(dnsmasqCapsGetBinaryPath(caps));
>>>      virCommandAddArgFormat(cmd, "--conf-file=%s", configfile);
>>> +
>>> +    /* This helper is used to create custom leases file for libvirt */
>>> +    virCommandAddArgFormat(cmd, "--dhcp-script=%s", LIBEXECDIR "/libvirt_leaseshelper");
>>
>> This is a bit hard-coded, and won't play nicely with ./run.  Ideally, we
>> should be constructing the name so that if argv[0] is an uninstalled
>> in-tree binary, then we convert to a name relative to the build tree
>> instead of LIBEXECDIR; that way, when using ./run, we test the
>> just-built libvirt_leaseshelper instead of a pre-installed version.
> 
> I'm not very clear about how to go about this. I understand that we
> want some magic to be set by ./run so that it understands where to
> pick the binary from, (I always have to build libvirt with
> --libexecdir=$PWD/src, since I never run make install) but what exact
> changes do I have to make here? Could you please give an example?

Do NOT use --libexecdir=$PWD/src - that is a sure recipe for a broken
setup.  --libexecdir should only ever point to the location where you
plan to install things, but you do NOT plan to install into your build tree.

An example for setting build-relative paths can be seen in commit
e562e82.  In fact, look at daemon/libvirtd.c: it specifically checks for
argv[0] starting with lt-libvirtd or .libs/libvirtd, both of which are
evidence of an in-tree build.  In those cases, it resets several
variables naming directories to be a build-relative file name; in all
other cases, the variables default to their install-relative name.  The
idea is that you do not directly want to do virCommandAddArgFormat(...
LIBEXECDIR "/libvirt_leaseshelper"), but instead want to do:

char *leaseshelper;
if (test for in-tree binary)
    VIR_STRDUP(leaseshelper,
          "../rel/to/build-tree/libvirt_leaseshelper");
else
    VIR_STRDUP(leaseshelper, LIBEXECDIR "/libvirt_leaseshelper");
virCommandAddArgFormat(..., leaseshelper);

(some hand-waving there, but hopefully enough to get the point across)


>>> +    } else {
>>> +        printf(_("Usage: %s ACTION MAC|CLIENTID IP HOSTNAME\n"
>>> +                 "   or: %s ACTION MAC|CLIENTID IP\n"),
>>
>> Could be compressed to one line as "%s ACTION MAC|CLIENTID IP
>> [HOSTNAME]".  Maybe worth listing the set of valid ACTION verbs.
> 
> Should I change this to:
> printf(_("Usage: %s ACTION MAC|CLIENTID IP [HOSTNAME]\n")
> and then display the meaning of 'ACTION' or simply do:
> printf(_("Usage: %s add|old|del mac|clientid ip [hostname]\n")

The latter works fine for me.  Or even two lines:

"Usage: %s add|old|del mac|clientid ip [hostname]\n"
"Designed for use with 'dnsmasq --dhcp-script\n"

-- 
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/20140321/996744df/attachment-0001.sig>