On Jan 14, 2014, at 2:09 PM, Nehal J Wani <nehaljw.kkd1@xxxxxxxxx> 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". > > The format of each lease is: > <expiry-time (epoch time)> <mac> <iaid> <ip-address> <hostname> <clientid> > I feel like I'm bikesheding but is it the best idea to have a custom file format? I know our string handling code makes this really easy to do but it just has a slight code smell to make our own format. We link to stuff like yajl and libxml for JSON/XML support and its really simple to do so we could easily write out a JSON/XML file and read it in. Definitely don't rework the patch based on my comments because we'll 99.9% go with this way, I'm just asking a question I felt should be asked. > Example of custom leases file content: > 1385245780 52:54:00:2f:ba:76 * 192.168.150.153 * * > 1385245781 52:54:00:2f:ba:76 3127926 2001:db8:ca2:2:1::6c * 00:04:76:00:cf:ae:b3:0b:fc:cd:0e:22:2e:97:76:65:74:ec > 1385245964 52:54:00:44:7c:d7 * 192.168.150.219 iiit-ad885e4aa1 01:52:54:00:44:7c:d7 > 1385245964 52:54:00:44:7c:d7 * 192.168.150.219 * 01:52:54:00:44:7c:d7 > 1385246016 52:54:00:5d:99:92 * 192.168.150.212 iiit-ad885e4aa1 01:52:54:00:5d:99:92 > 1385246041 52:54:00:3b:16:e0 * 192.168.150.207 * * > 1385246081 52:54:00:db:dd:98 * 192.168.150.234 * * > 1385246088 52:54:00:db:dd:98 14409112 2001:db8:ca2:2:1::6d * 00:04:76:00:cf:ae:b3:0b:fc:cd:0e:22:2e:97:76:65:74:ec > > --- > As danpb suggested, I have split the previous patch into helper program & API > Refer: https://www.redhat.com/archives/libvir-list/2013-December/msg00694.html > Once this get ACKed, I'll send in the patches for the Leases API v6 > > src/Makefile.am | 20 ++++ > src/network/bridge_driver.c | 4 + > src/util/leaseshelper.c | 225 ++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 249 insertions(+) > create mode 100644 src/util/leaseshelper.c > > diff --git a/src/Makefile.am b/src/Makefile.am > index 57e163f..6e5b03c 100644 > --- a/src/Makefile.am > +++ b/src/Makefile.am > @@ -838,6 +838,9 @@ STORAGE_HELPER_DISK_SOURCES = \ > UTIL_IO_HELPER_SOURCES = \ > util/iohelper.c > > +UTIL_LEASES_HELPER_SOURCES = \ > + util/leaseshelper.c > + > # Network filters > NWFILTER_DRIVER_SOURCES = \ > nwfilter/nwfilter_driver.h nwfilter/nwfilter_driver.c \ > @@ -2408,6 +2411,23 @@ libvirt_iohelper_CFLAGS = \ > $(NULL) > endif WITH_LIBVIRTD > > +if WITH_LIBVIRTD > +libexec_PROGRAMS += libvirt_leaseshelper > +libvirt_leaseshelper_SOURCES = $(UTIL_LEASES_HELPER_SOURCES) > +libvirt_leaseshelper_LDFLAGS = \ > + $(NULL) > +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) > +endif WITH_LIBVIRTD > + > if WITH_STORAGE_DISK > if WITH_LIBVIRTD > libexec_PROGRAMS += libvirt_parthelper > diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c > index 95e4b65..2278dba 100644 > --- a/src/network/bridge_driver.c > +++ b/src/network/bridge_driver.c > @@ -1063,6 +1063,10 @@ networkBuildDhcpDaemonCommandLine(virNetworkObjPtr network, > > cmd = virCommandNew(dnsmasqCapsGetBinaryPath(caps)); > virCommandAddArgFormat(cmd, "--conf-file=%s", configfile); > + > + /* This helper is used to create cutom leases file for libvirt */ > + virCommandAddArgFormat(cmd, "--dhcp-script=%s", LIBEXECDIR "/libvirt_leaseshelper"); > + > *cmdout = cmd; > ret = 0; > cleanup: > diff --git a/src/util/leaseshelper.c b/src/util/leaseshelper.c > new file mode 100644 > index 0000000..486ebe3 > --- /dev/null > +++ b/src/util/leaseshelper.c > @@ -0,0 +1,225 @@ > +/* > + * leasehelper.c: Helper program to create custom leases file > + * > + * Copyright (C) 2013 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@xxxxxxxxx> > + * > + */ > + > +#include <config.h> > + > +#include <stdio.h> > +#include <stdlib.h> > + > +#include "virutil.h" > +#include "virthread.h" > +#include "virfile.h" > +#include "virbuffer.h" > +#include "virstring.h" > +#include "virerror.h" > +#include "viralloc.h" > +#include "configmake.h" > + > +#define VIR_FROM_THIS VIR_FROM_NETWORK > + > +/** > + * VIR_NETWORK_DHCP_LEASE_FIELDS: > + * > + * Macro providing the maximum number of fields in an entry in > + * the leases file > + */ > +#define VIR_NETWORK_DHCP_LEASE_FIELDS 6 > +/** > + * 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 2097152 > + > +/* > + * Use this when passing possibly-NULL strings to printf-a-likes. > + */ > +# define EMPTY_STR(s) ((s) ? (s) : "*") > + > +int > +main(int argc, char **argv) { > + > + /* Doesn't hurt to check */ > + if (argc < 4) { > + /* Refer man page of dnsmasq --dhcp-script for more details */ > + fprintf(stderr, "Usage: $program $action ${mac|clientid} $ip\n"); > + return -1; > + } > + > + const char *program_name = argv[0]; > + const char *action = argv[1]; > + const char *interface = EMPTY_STR(virGetEnvAllowSUID("DNSMASQ_INTERFACE")); > + const char *expirytime = EMPTY_STR(virGetEnvAllowSUID("DNSMASQ_LEASE_EXPIRES")); > + const char *mac = argv[2]; > + const char *ip = argv[3]; > + const char *iaid = EMPTY_STR(virGetEnvAllowSUID("DNSMASQ_IAID")); > + const char *hostname = EMPTY_STR(virGetEnvAllowSUID("DNSMASQ_SUPPLIED_HOSTNAME")); > + const char *clientid = EMPTY_STR(virGetEnvAllowSUID("DNSMASQ_CLIENT_ID")); > + const char *leases_str = NULL; > + char *lease_file = NULL; > + char *lease_entries = NULL; > + char *lease_entry = NULL; > + char **lease_fields = NULL; > + bool delete = false; > + bool add = false; > + int rv = -1; > + int lease_file_len = 0; > + FILE *fp = NULL; > + long long expirytime_tmp = 0; > + virBuffer buf_new_lease = VIR_BUFFER_INITIALIZER; > + virBuffer buf_all_leases = VIR_BUFFER_INITIALIZER; > + > + 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); > + } > + > + if (virAsprintf(&lease_file, "%s/%s.status", LOCALSTATEDIR > + "/lib/libvirt/dnsmasq/", interface) < 0) > + goto cleanup; > + > + if (virGetEnvAllowSUID("DNSMASQ_IAID")) { > + mac = EMPTY_STR(virGetEnvAllowSUID("DNSMASQ_MAC")); > + clientid = argv[2]; > + } > + > + /* Make sure dnsmasq knows the interface, otherwise something is wrong */ > + if (STREQ(interface, "*")) > + goto cleanup; > + > + /* Make sure the file exists. If not, 'touch' it */ > + if (virFileTouch(lease_file, 0644) < 0) > + goto cleanup; > + > + /* Read entire contents */ > + if ((lease_file_len = virFileReadAll(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")) { > + fprintf(stderr, "add|old\n"); > + add = true; > + /* Enter new lease */ > + virBufferAsprintf(&buf_new_lease, "%s %s %s %s %s %s\n", > + expirytime, mac, iaid, ip, hostname, clientid); > + > + if (virBufferError(&buf_new_lease)) { > + virBufferFreeAndReset(&buf_new_lease); > + virReportOOMError(); > + goto cleanup; > + } > + } > + } > + } > + > + lease_entry = lease_entries[0] == '\0' ? NULL : lease_entries; > + > + while (lease_entry) { > + int nfields = 0; > + > + char *eol = strchr(lease_entry, '\n'); > + *eol = '\0'; > + > + /* Split the lease line */ > + if (!(lease_fields = virStringSplit(lease_entry, " ", > + VIR_NETWORK_DHCP_LEASE_FIELDS))) > + goto cleanup; > + > + nfields = virStringListLength(lease_fields); > + > + /* Forward lease_entry to the next lease */ > + lease_entry = strchr(lease_entry, '\0'); > + if (lease_entry - lease_entries + 1 < lease_file_len) > + lease_entry++; > + else > + lease_entry = NULL; > + > + if (nfields != VIR_NETWORK_DHCP_LEASE_FIELDS) > + goto cleanup; > + > + if (virStrToLong_ll(lease_fields[0], NULL, 10, &expirytime_tmp) < 0) { > + virReportError(VIR_ERR_INTERNAL_ERROR, > + _("Unable to convert lease expiry time to integer: %s"), > + lease_fields[0]); > + goto cleanup; > + } > + > + /* Check whether lease has expired or not */ > + if (expirytime_tmp < (long long) time(NULL)) > + continue; > + else if (delete && STREQ(lease_fields[3], ip)) > + continue; > + else { > + virBufferAsprintf(&buf_all_leases, "%s %s %s %s %s %s\n", > + lease_fields[0], lease_fields[1], lease_fields[2], > + lease_fields[3], lease_fields[4], lease_fields[5]); > + > + if (virBufferError(&buf_all_leases)) { > + virBufferFreeAndReset(&buf_all_leases); > + virReportOOMError(); > + goto cleanup; > + } > + } > + } > + > + if (add) { > + virBufferAsprintf(&buf_all_leases, "%s", virBufferContentAndReset(&buf_new_lease)); > + > + if (virBufferError(&buf_all_leases)) { > + virBufferFreeAndReset(&buf_all_leases); > + virReportOOMError(); > + goto cleanup; > + } > + } > + > + rv = 0; > + > + /* Write to file */ > + leases_str = virBufferContentAndReset(&buf_all_leases); > + if (!leases_str) > + leases_str = ""; > + > + if (virFileWriteStr(lease_file, leases_str, 0) < 0) > + rv = -1; > + > +cleanup: > + VIR_FREE(lease_file); > + VIR_FREE(lease_entries); > + if (lease_fields) > + virStringFreeList(lease_fields); > + return rv; > +} > -- > 1.8.1.4 > > -- > libvir-list mailing list > libvir-list@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list