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

Introduce helper program to catch events from dnsmasq and maintain a custom lease file per network. It supports dhcpv4 and dhcpv6. The file is saved as "<interface-name>.status". Each lease contains the following info: <expiry-time (epoch time)> <mac> <iaid> <ip-address> <hostname> <clientid> Example of custom leases file content: [ { "iaid": "1221229", "ip-address": "2001:db8:ca2:2:1::95", "mac-address": "52:54:00:12:a2:6d", "hostname": "Fedora20", "client-id": "00:04:1a:c1:d9:6b:5a:0a:e2:bc:f8:4b:1e:37:2e:38:22:55", "expiry-time": 1393244216 }, { "ip-address": "192.168.150.208", "mac-address": "52:54:00:11:56:b3", "hostname": "Wani-PC", "client-id": "01:52:54:00:11:56:b3", "expiry-time": 1393244248 } ] src/Makefile.am: * Add options to compile the helper program src/network/bridge_driver.c: * Introduce networkDnsmasqLeaseFileNameCustom() * Invoke helper program along with dnsmasq * Delete the .status file when corresponding n/w is destroyed. src/network/leaseshelper.c * Helper program to create the custom lease file --- v4: * Addition of pidfile and a corresponding lock for it * Make correction for dnsmasq < 2.52 (Only IPv4) * Move helper file from src/util to src/network * Increase limit on max size of leases file v3: * Improved file handling, removed redundant copying, introduced --help and --version * Refer: https://www.redhat.com/archives/libvir-list/2014-February/msg01431.html v2: * Changed format to JSON * Refer: https://www.redhat.com/archives/libvir-list/2014-January/msg01234.html v1: * Refer: https://www.redhat.com/archives/libvir-list/2014-January/msg00626.html src/Makefile.am | 21 +++ src/network/bridge_driver.c | 19 +++ src/network/leaseshelper.c | 328 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 368 insertions(+), 0 deletions(-) create mode 100644 src/network/leaseshelper.c diff --git a/src/Makefile.am b/src/Makefile.am index 2eb7840..d3bcd66 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -849,6 +849,9 @@ STORAGE_HELPER_DISK_SOURCES = \ UTIL_IO_HELPER_SOURCES = \ util/iohelper.c +NETWORK_LEASES_HELPER_SOURCES = \ + network/leaseshelper.c + # Network filters NWFILTER_DRIVER_SOURCES = \ nwfilter/nwfilter_driver.h nwfilter/nwfilter_driver.c \ @@ -2450,6 +2453,24 @@ libvirt_iohelper_CFLAGS = \ $(AM_CFLAGS) \ $(PIE_CFLAGS) \ $(NULL) + +if WITH_NETWORK +libexec_PROGRAMS += libvirt_leaseshelper +libvirt_leaseshelper_SOURCES = $(NETWORK_LEASES_HELPER_SOURCES) +libvirt_leaseshelper_LDADD = \ + libvirt_util.la \ + ../gnulib/lib/libgnu.la +if WITH_DTRACE_PROBES +libvirt_leaseshelper_LDADD += libvirt_probes.lo +endif WITH_DTRACE_PROBES + +libvirt_leaseshelper_CFLAGS = \ + $(PIE_CFLAGS) \ + $(NULL) +else ! WITH_NETWORK +EXTRA_DIST += $(NETWORK_LEASES_HELPER_SOURCES) +endif ! WITH_NETWORK + endif WITH_LIBVIRTD if WITH_STORAGE_DISK diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 181541e..4d0ca4d 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -208,6 +208,16 @@ networkDnsmasqLeaseFileNameFunc networkDnsmasqLeaseFileName = networkDnsmasqLeaseFileNameDefault; static char * +networkDnsmasqLeaseFileNameCustom(const char *bridge) +{ + char *leasefile; + + ignore_value(virAsprintf(&leasefile, "%s/%s.status", + driverState->dnsmasqStateDir, bridge)); + return leasefile; +} + +static char * networkDnsmasqConfigFileName(const char *netname) { char *conffile; @@ -243,6 +253,7 @@ networkRemoveInactive(virNetworkDriverStatePtr driver, virNetworkObjPtr net) { char *leasefile = NULL; + char *customleasefile = NULL; char *radvdconfigfile = NULL; char *configfile = NULL; char *radvdpidbase = NULL; @@ -261,6 +272,9 @@ networkRemoveInactive(virNetworkDriverStatePtr driver, if (!(leasefile = networkDnsmasqLeaseFileName(def->name))) goto cleanup; + if (!(customleasefile = networkDnsmasqLeaseFileNameCustom(def->bridge))) + goto cleanup; + if (!(radvdconfigfile = networkRadvdConfigFileName(def->name))) goto cleanup; @@ -277,6 +291,7 @@ networkRemoveInactive(virNetworkDriverStatePtr driver, /* dnsmasq */ dnsmasqDelete(dctx); unlink(leasefile); + unlink(customleasefile); unlink(configfile); /* radvd */ @@ -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"); + *cmdout = cmd; ret = 0; cleanup: diff --git a/src/network/leaseshelper.c b/src/network/leaseshelper.c new file mode 100644 index 0000000..d5aa121 --- /dev/null +++ b/src/network/leaseshelper.c @@ -0,0 +1,328 @@ +/* + * leasehelper.c: Helper program to create custom leases file + * + * Copyright (C) 2014 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Nehal J Wani <nehaljw.kkd1@gmail.com> + * + * For IPv6 support, use dnsmasq >= 2.67 + */ + +#include <config.h> + +#include <stdio.h> +#include <stdlib.h> +#include <sys/stat.h> + +#include "virutil.h" +#include "virthread.h" +#include "virfile.h" +#include "virpidfile.h" +#include "virbuffer.h" +#include "virstring.h" +#include "virerror.h" +#include "viralloc.h" +#include "virjson.h" +#include "configmake.h" + +#define VIR_FROM_THIS VIR_FROM_NETWORK + +/** + * VIR_NETWORK_DHCP_LEASE_FILE_SIZE_MAX: + * + * Macro providing the upper limit on the size of leases file + */ +#define VIR_NETWORK_DHCP_LEASE_FILE_SIZE_MAX (32 * 1024 * 1024) + +static const char *program_name; + +/* Display version information. */ +static void +helperVersion(const char *argv0) +{ + printf("%s (%s) %s\n", argv0, PACKAGE_NAME, PACKAGE_VERSION); +} + +ATTRIBUTE_NORETURN static void +usage(int status) +{ + if (status) { + fprintf(stderr, _("%s: try --help for more details\n"), program_name); + } else { + printf(_("Usage: %s ACTION MAC|CLIENTID IP HOSTNAME\n" + " or: %s ACTION MAC|CLIENTID IP\n"), + program_name, program_name); + } + exit(status); +} + +static int +customLeaseRewriteFile(int fd, void *opaque) +{ + char **data = opaque; + + if (safewrite(fd, *data, strlen(*data)) < 0) + return -1; + + return 0; +} + +int +main(int argc, char **argv) +{ + char *exptime = NULL; + char *pid_file = NULL; + char *lease_entries = NULL; + char *custom_lease_file = NULL; + const char *ip = NULL; + const char *mac = NULL; + const char *action = NULL; + const char *iaid = virGetEnvAllowSUID("DNSMASQ_IAID"); + const char *clientid = virGetEnvAllowSUID("DNSMASQ_CLIENT_ID"); + const char *interface = virGetEnvAllowSUID("DNSMASQ_INTERFACE"); + const char *exptime_tmp = virGetEnvAllowSUID("DNSMASQ_LEASE_EXPIRES"); + const char *hostname = virGetEnvAllowSUID("DNSMASQ_SUPPLIED_HOSTNAME"); + const char *leases_str = NULL; + long long expirytime = 0; + size_t i = 0; + int size = 0; + int pid_file_fd = -1; + int rv = EXIT_FAILURE; + int custom_lease_file_len = 0; + bool add = false; + bool delete = false; + virJSONValuePtr lease_new = NULL; + virJSONValuePtr lease_tmp = NULL; + virJSONValuePtr leases_array = NULL; + virJSONValuePtr leases_array_new = NULL; + + virSetErrorFunc(NULL, NULL); + virSetErrorLogPriorityFunc(NULL); + + program_name = argv[0]; + + if (setlocale(LC_ALL, "") == NULL || + bindtextdomain(PACKAGE, LOCALEDIR) == NULL || + textdomain(PACKAGE) == NULL) { + fprintf(stderr, _("%s: initialization failed\n"), program_name); + exit(EXIT_FAILURE); + } + + if (virThreadInitialize() < 0 || + virErrorInitialize() < 0) { + fprintf(stderr, _("%s: initialization failed\n"), program_name); + exit(EXIT_FAILURE); + } + + /* Doesn't hurt to check */ + if (argc > 1) { + if(STREQ(argv[1], "--help")) + usage(EXIT_SUCCESS); + + if (STREQ(argv[1], "--version")) { + helperVersion(argv[0]); + exit(EXIT_SUCCESS); + } + } + + if (argc != 4 && argc != 5) { + /* Refer man page of dnsmasq --dhcp-script for more details */ + usage(EXIT_FAILURE); + } + + /* Make sure dnsmasq knows the interface. The interface name is not known + * when dnsmasq (re)starts and throws 'del' events for expired leases. + * So, if any old lease has expired, it will be automatically removed the + * next time this program is invoked */ + if (!interface) + goto cleanup; + + ip = argv[3]; + mac = argv[2]; + action = argv[1]; + + /* In case hostname is known, it is the 5th argument */ + if (argc == 5) + hostname = argv[4]; + + if (VIR_STRDUP(exptime, exptime_tmp) < 0) + goto cleanup; + + /* Removed extraneous trailing space in DNSMASQ_LEASE_EXPIRES (dnsmasq < 2.52) */ + if (exptime[strlen(exptime) - 1] == ' ') + exptime[strlen(exptime) - 1] = '\0'; + + /* Check if it is an IPv6 lease */ + if (virGetEnvAllowSUID("DNSMASQ_IAID")) { + mac = virGetEnvAllowSUID("DNSMASQ_MAC"); + clientid=argv[2]; + } + + if (virAsprintf(&custom_lease_file, "%s/%s.status", LOCALSTATEDIR + "/lib/libvirt/dnsmasq/", interface) < 0) + goto cleanup; + + if (VIR_STRDUP(pid_file, LOCALSTATEDIR "/run/leaseshelper.pid") < 0) + goto cleanup; + + /* Try to claim the pidfile, exiting if we can't */ + if ((pid_file_fd = virPidFileAcquirePath(pid_file, true, getpid())) < 0) + goto cleanup; + + /* Since interfaces can be hot plugged, we need to make sure that the + * corresponding custom lease file exists. If not, 'touch' it */ + if (virFileTouch(custom_lease_file, 0644) < 0) + goto cleanup; + + /* Read entire contents */ + if ((custom_lease_file_len = virFileReadAll(custom_lease_file, + VIR_NETWORK_DHCP_LEASE_FILE_SIZE_MAX, + &lease_entries)) < 0) { + goto cleanup; + } + + if (STREQ(action, "add") || STREQ(action, "old") || STREQ(action, "del")) { + if (mac || STREQ(action, "del")) { + /* Delete the corresponding lease */ + delete = true; + if (STREQ(action, "add") || STREQ(action, "old")) { + add = true; + /* Create new lease */ + if (!(lease_new = virJSONValueNewObject())) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to create json")); + goto cleanup; + } + + if (virStrToLong_ll(exptime, NULL, 10, &expirytime) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to convert lease expiry time to long long: %s"), + exptime); + goto cleanup; + } + + if ((iaid && virJSONValueObjectAppendString(lease_new, "iaid", + iaid) < 0) || + (ip && virJSONValueObjectAppendString(lease_new, "ip-address", + ip) < 0) || + (mac && virJSONValueObjectAppendString(lease_new, "mac-address", + mac) < 0) || + (hostname && virJSONValueObjectAppendString(lease_new, "hostname", + hostname) < 0) || + (clientid && virJSONValueObjectAppendString(lease_new, "client-id", + clientid) < 0) || + (expirytime && virJSONValueObjectAppendNumberLong(lease_new, "expiry-time", + expirytime) < 0)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to create json")); + goto cleanup; + } + } + } + } else { + fprintf(stderr, _("Unsupported action: %s\n"), action); + exit(EXIT_FAILURE); + } + + /* Check for previous leases */ + if (custom_lease_file_len) { + if (!(leases_array = virJSONValueFromString(lease_entries))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("invalid json in file: %s"), custom_lease_file); + goto cleanup; + } + + if ((size = virJSONValueArraySize(leases_array)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("couldn't fetch array of leases")); + goto cleanup; + } + } + + if (!(leases_array_new = virJSONValueNewArray())) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to create json")); + goto cleanup; + } + + for (i = 0; i < size; i++) { + const char *ip_tmp = NULL; + long long exptime_tmp = -1; + + if (!(lease_tmp = virJSONValueArrayGet(leases_array, i))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to parse json")); + goto cleanup; + } + + if (!(ip_tmp = virJSONValueObjectGetString(lease_tmp, "ip-address")) || + (virJSONValueObjectGetNumberLong(lease_tmp, "expiry-time", &exptime_tmp) < 0)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to parse json")); + goto cleanup; + } + + /* Check whether lease has expired or not */ + if (exptime_tmp < (long long) time(NULL)) + continue; + + /* Check whether lease has to be included or not */ + if (delete && STREQ(ip_tmp, ip)) + continue; + + /* Add old lease to new array */ + if (virJSONValueArrayAppend(leases_array_new, lease_tmp) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to create json")); + goto cleanup; + } + } + + if (add) { + if (virJSONValueArrayAppend(leases_array_new, lease_new) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to create json")); + goto cleanup; + } + } + + if (!(leases_str = virJSONValueToString(leases_array_new, true))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("empty json array")); + goto cleanup; + } + + /* Write to file */ + if (virFileRewrite(custom_lease_file, 0644, + customLeaseRewriteFile, &leases_str) < 0) + goto cleanup; + + rv = EXIT_SUCCESS; + +cleanup: + if (pid_file_fd != -1) + virPidFileReleasePath(pid_file, pid_file_fd); + + VIR_FREE(pid_file); + VIR_FREE(exptime_tmp); + VIR_FREE(custom_lease_file); + virJSONValueFree(lease_new); + virJSONValueFree(leases_array); + virJSONValueFree(leases_array_new); + + return rv; +} -- 1.7.1

On 03/17/2014 03:30 PM, Nehal J Wani wrote:
Introduce helper program to catch events from dnsmasq and maintain a custom lease file per network. It supports dhcpv4 and dhcpv6. The file is saved as "<interface-name>.status".
@@ -243,6 +253,7 @@ networkRemoveInactive(virNetworkDriverStatePtr driver, virNetworkObjPtr net) { char *leasefile = NULL; + char *customleasefile = NULL; char *radvdconfigfile = NULL; char *configfile = NULL; char *radvdpidbase = NULL; @@ -261,6 +272,9 @@ networkRemoveInactive(virNetworkDriverStatePtr driver, if (!(leasefile = networkDnsmasqLeaseFileName(def->name))) goto cleanup;
+ if (!(customleasefile = networkDnsmasqLeaseFileNameCustom(def->bridge))) + goto cleanup; +
Memory leak - customleasefile is malloc'd memory, but the cleanup label never frees it.
@@ -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.
+++ b/src/network/leaseshelper.c @@ -0,0 +1,328 @@ +/* + * leasehelper.c: Helper program to create custom leases file + * + * Copyright (C) 2014 Red Hat, Inc.
You're allowed to claim copyright if you'd like; you don't have to assign it all to Red Hat.
+ +ATTRIBUTE_NORETURN static void +usage(int status) +{ + if (status) { + fprintf(stderr, _("%s: try --help for more details\n"), program_name); + } 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.
+ size_t i = 0; + int size = 0;
Usually something named 'size' is of type 'size_t' - but I guess I'll have to see how you use it.
+ + if (argc != 4 && argc != 5) { + /* Refer man page of dnsmasq --dhcp-script for more details */
This comment might also be useful as actual output of the --help option.
+ /* Check if it is an IPv6 lease */ + if (virGetEnvAllowSUID("DNSMASQ_IAID")) { + mac = virGetEnvAllowSUID("DNSMASQ_MAC"); + clientid=argv[2];
Space around =
+ } + + if (virAsprintf(&custom_lease_file, "%s/%s.status", LOCALSTATEDIR + "/lib/libvirt/dnsmasq/", interface) < 0)
Unusual line break; I would have done: if (virAsprintf(&custom_lease_file, "%s/%s.status", LOCALSTATEDIR "/lib/libvirt/dnsmasq/", interface) < 0) or even: if (virAsprintf(&custom_lease_file, LOCALSTATEDIR "/lib/libvirt/dnsmasq/%s.status", interface) < 0)
+ + if (STREQ(action, "add") || STREQ(action, "old") || STREQ(action, "del")) { + if (mac || STREQ(action, "del")) {
This is a lot of repeated STREQ. It might be nicer if you use VIR_ENUM_IMPL earlier on, and convert the incoming string to an enum value; then the rest of your code will use integer comparisons instead of repeat string comparisons.
+ + if ((iaid && virJSONValueObjectAppendString(lease_new, "iaid", + iaid) < 0) || + (ip && virJSONValueObjectAppendString(lease_new, "ip-address", + ip) < 0) ||
This works, but it's harder to debug (gdb is rather picky about how it puts breakpoints across chains of && and || when optimization gets involved). It may be a bit nicer to have multiple if statements: if (iaid && virJSON...() < 0) goto cleanup; if (ip && virJSON...() < 0) goto cleanup; ...
+ + if ((size = virJSONValueArraySize(leases_array)) < 0) {
Ah, so size really is an 'int'. You were right here.
+ for (i = 0; i < size; i++) {
+ /* Check whether lease has expired or not */ + if (exptime_tmp < (long long) time(NULL))
This calls time() in a loop, which can get a bit expensive on some hardware, as well as a bit unpredictable (the time changes over the course of the loop). Better might be to call time() outside the loop, then compare against that value inside the loop. Looks like you are converging on something worth applying. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

@@ -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?
+ +ATTRIBUTE_NORETURN static void +usage(int status) +{ + if (status) { + fprintf(stderr, _("%s: try --help for more details\n"), program_name); + } 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") -- Nehal J Wani

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
participants (2)
-
Eric Blake
-
Nehal J Wani