Re: [PATCH v2] Add helper program to create custom leases

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]