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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list