[libvirt] [RFC] net-dhcp-leases: Query: Reg: Leases API Script
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. Q2. The above executable file will be writing the custom formatted lease parameters to a file "dnsmasq-ip-mac.status" (suggestion open for name). This newly created/updated file will be parsed by the API. We need to decide the format for the file. Do we continue with space separated parameters as before? Q3. What should be the location of the above two files? Is there any example that I can follow in libvirt for deploying custom-script files/C programs which are not to be linked? Script is attached. -- Nehal J Wani
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.
Q2. The above executable file will be writing the custom formatted lease parameters to a file "dnsmasq-ip-mac.status" (suggestion open for name). This newly created/updated file will be parsed by the API. We need to decide the format for the file. Do we continue with space separated parameters as before?
Ideally do not invent any new format - use a format that we already have a parser for - eg the src/util/virconf.h or src/util/virkeyfile.h APIs for loading configs.
Q3. What should be the location of the above two files? Is there any example that I can follow in libvirt for deploying custom-script files/C programs which are not to be linked?
We aim to put such state in /var/lib/libvirt/network/ if it needs to survive reboots, or /var/run/libvirt/network if it does not need to survive reboots 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 :|
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.
Q2. The above executable file will be writing the custom formatted lease parameters to a file "dnsmasq-ip-mac.status" (suggestion open for name). This newly created/updated file will be parsed by the API. We need to decide the format for the file. Do we continue with space separated parameters as before?
Ideally do not invent any new format - use a format that we already have a parser for - eg the src/util/virconf.h or src/util/virkeyfile.h APIs for loading configs.
or XML, if the config file format is insufficient. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Tue, Oct 22, 2013 at 9:10 PM, Eric Blake <eblake@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?
Q2. The above executable file will be writing the custom formatted lease parameters to a file "dnsmasq-ip-mac.status" (suggestion open for name). This newly created/updated file will be parsed by the API. We need to decide the format for the file. Do we continue with space separated parameters as before?
Ideally do not invent any new format - use a format that we already have a parser for - eg the src/util/virconf.h or src/util/virkeyfile.h APIs for loading configs.
or XML, if the config file format is insufficient.
Actually, the leases file generated by dnsmasq follows the format of space separated parameters (For which I had created a parser in the earlier versions of the patch). But considering the fact that each time the script is called, in case the first argument is "del" or "old" I'll need to replace the existing lease with the new one, for which I am a bit confused as to which format should be followed and whether it would be easier to code it in C (I can't think of anything easier than loading the whole file into buffer and then replacing the corresponding line, and then writing back to the file { Hoping that existing semaphores will take care of the reader-writers problem :) } ). Is life more easy handling XML, or ini-style formats (using the existing helpers)?
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
-- Nehal J Wani
On 10/22/2013 04:57 PM, Nehal J Wani wrote:
On Tue, Oct 22, 2013 at 9:10 PM, Eric Blake <eblake@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?
C would be better: we want libvirtd to run even on systems that don't have python installed. But if you can demonstrate what you want your program to do by first mocking it up in a different language to get the management of file handling between your program and libvirtd correct, and only then convert to a proper C program, it wouldn't be the end of the world.
Ideally do not invent any new format - use a format that we already have a parser for - eg the src/util/virconf.h or src/util/virkeyfile.h APIs for loading configs.
or XML, if the config file format is insufficient.
Actually, the leases file generated by dnsmasq follows the format of space separated parameters (For which I had created a parser in the earlier versions of the patch). But considering the fact that each time the script is called, in case the first argument is "del" or "old" I'll need to replace the existing lease with the new one, for which I am a bit confused as to which format should be followed and whether it would be easier to code it in C (I can't think of anything easier than loading the whole file into buffer and then replacing the corresponding line, and then writing back to the file { Hoping that existing semaphores will take care of the reader-writers problem :) } ). Is life more easy handling XML, or ini-style formats (using the existing helpers)?
I guess it all depends on what sort of data you are trying to store in the file for reuse by libvirt the next time it parses the file. Reusing your code that directly parsed the dnsmasq file by making your file use the same format is probably an acceptable form of reuse, as long as we are relatively sure that there is no way to introduce ambiguity in the parse due to crazy user naming conventions. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
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@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 :|
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@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@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@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
ping On Thu, Oct 24, 2013 at 11:01 AM, Nehal J Wani <nehaljw.kkd1@gmail.com> 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@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@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@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
-- Nehal J Wani
On Thu, Oct 24, 2013 at 8:31 PM, Nehal J Wani <nehaljw.kkd1@gmail.com> 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@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
ping -- Nehal J Wani
participants (3)
-
Daniel P. Berrange -
Eric Blake -
Nehal J Wani