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

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

 



ping

On Thu, Oct 24, 2013 at 11:01 AM, Nehal J Wani <nehaljw.kkd1@xxxxxxxxx> wrote:
> 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@xxxxxxxxx>
>  *
>  */
>
> #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@xxxxxxxxxx> 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@xxxxxxxxxx> 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



-- 
Nehal J Wani

--
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]