[libvirt] [RFC] net-dhcp-leases: Query: Reg: Leases API Script

Nehal J Wani nehaljw.kkd1 at gmail.com
Thu Oct 24 15:01:26 UTC 2013


I tried writing the helper program, didn't send v5, since I have some doubts.
The program works. Problems listed below.

diff --git a/src/Makefile.am b/src/Makefile.am
index 21b9caf..ef88132 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -825,6 +825,9 @@ STORAGE_HELPER_DISK_SOURCES =
                 \
 UTIL_IO_HELPER_SOURCES =                                       \
                util/iohelper.c

+UTIL_IO_LEASES_SOURCES =                                       \
+               util/leaseshelper.c
+
 # Network filters
 NWFILTER_DRIVER_SOURCES =                                              \
                nwfilter/nwfilter_driver.h nwfilter/nwfilter_driver.c   \
@@ -2384,6 +2387,23 @@ libvirt_iohelper_CFLAGS = \
                $(NULL)
 endif WITH_LIBVIRTD

+if WITH_LIBVIRTD
+libexec_PROGRAMS += libvirt_leaseshelper
+libvirt_leaseshelper_SOURCES = $(UTIL_IO_LEASES_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 3ce3130..182a426 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -1058,6 +1058,8 @@
networkBuildDhcpDaemonCommandLine(virNetworkObjPtr network,

     cmd = virCommandNew(dnsmasqCapsGetBinaryPath(caps));
     virCommandAddArgFormat(cmd, "--conf-file=%s", configfile);
+    virCommandAddArgFormat(cmd, "--dhcp-script=%s",
"/home/Wani/libvirt/src/libvirt_leaseshelper");
+    //virCommandAddArgFormat(cmd, "--dhcp-script=%s",
"/var/lib/libvirt/dnsmasq/dhcp-script");
     *cmdout = cmd;
     ret = 0;
 cleanup:


/*
 * leasehelper.c:
 *
 * Copyright (C) 2013-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 at gmail.com>
 *
 */

#include <config.h>

#include <stdio.h>
#include <stdlib.h>

#include "virutil.h"
#include "virfile.h"
#include "virbuffer.h"
#include "virstring.h"
#include "virerror.h"
#include "viralloc.h"

/**
 * 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 7
/**
 * 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 NULL_STR(s) ((s) ? (s) : "*")

int
main(int argc, char **argv) {
    /* Doesn't hurt to check */
    if (argc < 4)
        return -1;

    const char *action = argv[1];
    const char *interface = NULL_STR(getenv("DNSMASQ_INTERFACE"));
    const char *expirytime = NULL_STR(getenv("DNSMASQ_LEASE_EXPIRES"));
    const char *mac = argv[2];
    const char *ip = argv[3];
    const char *iaid = NULL_STR(getenv("DNSMASQ_IAID"));
    const char *hostname = NULL_STR(getenv("DNSMASQ_SUPPLIED_HOSTNAME"));
    const char *clientid = NULL_STR(getenv("DNSMASQ_CLIENT_ID"));
    const char *lease_file = "/var/lib/libvirt/dnsmasq/dnsmasq-ip-mac.status";
    const char *leases_str = 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;
    virBuffer buf_new_lease = VIR_BUFFER_INITIALIZER;
    virBuffer buf_all_leases = VIR_BUFFER_INITIALIZER;
    FILE *fp = NULL;

    if (getenv("DNSMASQ_IAID")) {
        mac = NULL_STR(getenv("DNSMASQ_MAC"));
        clientid = argv[2];
    }

    /* Make sure the file exists. If not, 'touch' it */
    fp = fopen(lease_file, "a+");
    fclose(fp);

    /* 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")) {
                add = true;
                /* Enter new lease */
                virBufferAsprintf(&buf_new_lease, "%s %s %s %s %s %s
%s\n", interface,
                                  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 (delete && STREQ(lease_fields[4], ip))
            continue;
        else {
            virBufferAsprintf(&buf_all_leases, "%s %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],
                              lease_fields[6]);

            if (virBufferError(&buf_all_leases)) {
                virBufferFreeAndReset(&buf_all_leases);
                //virReportOOMError();
                goto cleanup;
            }
        }
    }

    if (add)
        virBufferAsprintf(&buf_all_leases, "%s",
virBufferContentAndReset(&buf_new_lease));

    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;
}


Q1. Does every file have to include "* Copyright (C) 2013-2014 Red
Hat, Inc." in its copyright? (hacking.html doesn't mention in detail)
Q2. I am not able to use  virReportOOMError();. It keeps asking for
VIR_FROM_THIS
Q3. Currently, I have hard-coded the location of custom-leasefile:
 const char *lease_file = "/var/lib/libvirt/dnsmasq/dnsmasq-ip-mac.status";
If we don't hard-code the location of file, then the program will have
to ask src/network/bridge_driver.c, which is not allowed, as it is
private to the network driver.
Now, we have two options, either we leave the above as only one file
and add leases of all networks to it, or, have different files for
each network.
The only difficulty in option 1 is, when a network is destroyed, its
corresponding leases will also have to be deleted in the custom lease
file. If we go for option 2, and have different files for each
network, how will the above program know, while custom lease file has
to be updated?

-- 
Nehal J Wani

On Wed, Oct 23, 2013 at 3:46 AM, Daniel P. Berrange <berrange at redhat.com> wrote:
> On Tue, Oct 22, 2013 at 09:27:53PM +0530, Nehal J Wani wrote:
>> On Tue, Oct 22, 2013 at 9:10 PM, Eric Blake <eblake at redhat.com> wrote:
>> > On 10/22/2013 04:15 PM, Daniel P. Berrange wrote:
>> >> On Tue, Oct 22, 2013 at 04:15:37PM +0530, Nehal J Wani wrote:
>> >>> Q1. The --dhcp-script option will require libvirt to provide a script
>> >>> or executable file to be run. Now as the man page says, this file is
>> >>> executed "Whenever a new DHCP  ease is created, or an old one
>> >>> destroyed". Life was easy with the script, as it used the command sed
>> >>> to remove the destroyed lease. eblake had suggested me to use a C
>> >>> program instead. So I wanted to finalize whether to go with C or
>> >>> continue with shell script.
>> >>
>> >> Libvirt aims to avoid shell code whereever possible, since it is really
>> >> a very bad language from terms of reliability and security. eg quoting
>> >> rules are easy to get wrong, error handling is awful, portability is
>> >> non-trivial.
>> >
>> > For examples of writing a helper C program, see how src/util/iohelper.c
>> > is compiled into libvirt_iohelper.
>> >
>>
>> What are libvirt's views on python as a helper program?
>
> No to python for this role. This should be C.
>
> Daniel
> --
> |: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
> |: http://libvirt.org              -o-             http://virt-manager.org :|
> |: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
> |: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|



-- 
Nehal J Wani




More information about the libvir-list mailing list