On Tue, Nov 26, 2013 at 02:35:58AM +0530, Nehal J Wani wrote:
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 5aad75c..20ea40a 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in + +typedef struct _virNetworkDHCPLeases virNetworkDHCPLeases; +typedef virNetworkDHCPLeases *virNetworkDHCPLeasesPtr; +struct _virNetworkDHCPLeases {
s/Leases/Lease/ - each struct only stores a single lease
+ char *interface; /* Network interface name */ + long long expirytime; /* Seconds since epoch */ + int type; /* virIPAddrType */ + char *mac; /* MAC address */ + char *iaid; /* IAID */ + char *ipaddr; /* IP address */ + unsigned int prefix; /* IP address prefix */ + char *hostname; /* Hostname */ + char *clientid; /* Client ID or DUID */ +};
@@ -2019,6 +2022,7 @@ libvirt_setuid_rpc_client_la_SOURCES = \ util/virtypedparam.c \ util/viruri.c \ util/virutil.c \ + util/virmacaddr.c \ util/viruuid.c \ conf/domain_event.c \ rpc/virnetsocket.c \
Don't add this here.
diff --git a/src/libvirt.c b/src/libvirt.c index eff44eb..9a0b872 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -68,6 +68,7 @@ #include "virstring.h" #include "virutil.h" #include "virtypedparam.h" +#include "virmacaddr.h"
#ifdef WITH_TEST # include "test/test_driver.h"
+int +virNetworkGetDHCPLeasesForMAC(virNetworkPtr network, + const char *mac, + virNetworkDHCPLeasesPtr **leases, + unsigned int flags) +{ + virConnectPtr conn; + virMacAddr addr; + + VIR_DEBUG("network=%p, mac=%s, leases=%p, flags=%x", + network, mac, leases, flags); + + virResetLastError(); + + if (leases) + *leases = NULL; + + virCheckNonNullArgGoto(mac, error); + + if (!VIR_IS_CONNECTED_NETWORK(network)) { + virLibNetworkError(VIR_ERR_INVALID_NETWORK, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + /* Validate the MAC address */ + if (virMacAddrParse(mac, &addr) < 0) { + virReportInvalidArg(mac, "%s", + _("Given MAC Address doesn't comply " + "with the standard (IEEE 802) format")); + goto error; + }
Don't do this here - it is the job of driver APIs todo semantic validation of parameters.
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index fe9b497..f1a9707 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -639,4 +639,11 @@ LIBVIRT_1.1.3 { virConnectGetCPUModelNames; } LIBVIRT_1.1.1;
+LIBVIRT_1.1.4 { + global: + virNetworkDHCPLeaseFree; + virNetworkGetDHCPLeases; + virNetworkGetDHCPLeasesForMAC; +} LIBVIRT_1.1.3;
Can move this into the 1.2.1 version block now.
+/* + * Use this when passing possibly-NULL strings to printf-a-likes. + */ +# define NULL_STR(s) ((s) ? (s) : "*")
I'd suggest calling this EMPTY_STR to avoid confusion with the existing NULLSTR macro which prints "(null)"
+ +int +main(int argc, char **argv) { + + /* Doesn't hurt to check */ + if (argc < 4) + return -1;
You should print an error message to stderr along with usage text.
+ + 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"));
All use of getenv is banned in libvirt code - please make sure to run 'make syntax-check' and 'make check'. Use virGetEnvAllowSUID here
+ 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; + virBuffer buf_new_lease = VIR_BUFFER_INITIALIZER; + virBuffer buf_all_leases = VIR_BUFFER_INITIALIZER;
You must call the following before invoking any other libvir APIs at all 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 (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);
We have a virFileTouch method todo this.
+ + /* 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\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 (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)); +
Need virBufferError check on buf_all_leases here
+ 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; +}
I'd suggest that the lease helper program should be added in a separate patch from the public API, sine there's no real dependancy between them. 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 :|