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