[libvirt] [PATCHv5 0/4] Introduce APIs to extract DHCP leases info

This API returns the leases information stored in the DHCP leases file of dnsmasq for a given virtual network. It contacts the bridge network driver, which parses a custom leases file created by libvirt. It supports two methods: 1. Return info for all network interfaces connected to a given virtual network 2. Return information for a particular network interface in a given virtual network by providing its MAC Address v5 Created a helper file to generate custom leases for libvirt. Since dnsmasq doesn't provide the MAC Address in case of DHCPv6, the need for this file was proposed. The design of virNetworkDHCPLeases struct has been updated and the previous use of union has been removed. OOM errors have been taken care of in the remote driver and remote daemon. Discussion on struct design: https://www.redhat.com/archives/libvir-list/2013-October/msg00326.html Discussion on helper file: https://www.redhat.com/archives/libvir-list/2013-October/msg00989.html v4 * Added support for DHCPv6, updated lease file parsing method Refer: https://www.redhat.com/archives/libvir-list/2013-September/msg01554.html v3 * Mostly small nits, change in MACRO names, use of virSocketAddrGetIpPrefix to retrieve IP prefix from @dom XML. Refer: https://www.redhat.com/archives/libvir-list/2013-September/msg00832.html v2 * Since DHCPv6 is supposed to be suported in future, virNetworkGetDHCPLeasesForMAC changed, prefix and virIPAddrType added in virNetworkDHCPLeases struct. Refer: https://www.redhat.com/archives/libvir-list/2013-September/msg00732.html v1 * Refer: https://www.redhat.com/archives/libvir-list/2013-September/msg00620.html * The need for these APIs were result of a RFC was proposed on the list. Refer: http://www.redhat.com/archives/libvir-list/2013-July/msg01603.html Nehal J Wani (4): net-dhcp-leases: Implement the public APIs net-dhcp-leases: Implement the remote protocol net-dhcp-leases: Private implementation inside network driver net-dhcp-leases: Add virsh support daemon/remote.c | 176 ++++++++++++++++++++++++++++++ include/libvirt/libvirt.h.in | 35 ++++++ src/Makefile.am | 21 ++++ src/driver.h | 13 +++ src/libvirt.c | 197 ++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 7 ++ src/network/bridge_driver.c | 248 +++++++++++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 187 ++++++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 54 +++++++++- src/remote_protocol-structs | 38 +++++++ src/rpc/gendispatch.pl | 1 + src/util/leaseshelper.c | 185 ++++++++++++++++++++++++++++++++ tools/virsh-network.c | 118 ++++++++++++++++++++ tools/virsh.pod | 6 ++ 14 files changed, 1285 insertions(+), 1 deletion(-) create mode 100644 src/util/leaseshelper.c -- 1.8.1.4

Introduce 3 new APIs, virNetworkGetDHCPLeases, virNetworkGetDHCPLeasesForMAC and virNetworkDHCPLeaseFree. * virNetworkGetDHCPLeases: returns the dhcp leases information for a given virtual network. For DHCPv4, the information returned: - Network Interface Name - Expiry Time - MAC address (can be NULL, only in rare cases) - IAID (NULL) - IPv4 address (with type and prefix) - Hostname (can be NULL) - Client ID (can be NULL) For DHCPv6, the information returned: - Network Interface Name - Expiry Time - MAC address (can be NULL, only in rare cases) - IAID (can be NULL, only in rare cases) - IPv6 address (with type and prefix) - Hostname (can be NULL) - Client DUID Note: @mac, @iaid, @ipaddr, @clientid are in ASCII form, not raw bytes. Note: @expirytime can 0, in case the lease is for infinite time. * virNetworkGetDHCPLeasesForMAC: returns the dhcp leases information for a given virtual network and specified MAC Address. * virNetworkDHCPLeaseFree: allows the upper layer application to free the network interface object conveniently. There is no support for flags, so user is expected to pass 0 for both the APIs. include/libvirt/libvirt.h.in: * Define virNetworkGetDHCPLeases * Define virNetworkGetDHCPLeasesForMAC * Define virNetworkDHCPLeaseFree src/driver.h: * Define networkGetDHCPLeases * Define networkGetDHCPLeasesForMAC src/libvirt.c: * Implement virNetworkGetDHCPLeases * Implement virNetworkGetDHCPLeasesForMAC * Implement virNetworkDHCPLeaseFree src/libvirt_public.syms: * Export the new symbols src/util/leaseshelper.c: * Helper program to create custom leases file src/Makefile.am: * Add compiler flags for the helper file leaseshelper.c * Add virmacaddr.c to libvirt_setuid_rpc_client_la_SOURCES --- include/libvirt/libvirt.h.in | 35 ++++++++ src/Makefile.am | 21 +++++ src/driver.h | 13 +++ src/libvirt.c | 197 +++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 7 ++ src/util/leaseshelper.c | 185 ++++++++++++++++++++++++++++++++++++++++ 6 files changed, 458 insertions(+) create mode 100644 src/util/leaseshelper.c 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 @@ -2826,6 +2826,41 @@ int virConnectNumOfDefinedInterfaces (virConnectPtr conn); int virConnectListDefinedInterfaces (virConnectPtr conn, char **const names, int maxnames); + +typedef enum { + VIR_IP_ADDR_TYPE_IPV4, + VIR_IP_ADDR_TYPE_IPV6, + +#ifdef VIR_ENUM_SENTINELS + VIR_IP_ADDR_TYPE_LAST +#endif +} virIPAddrType; + +typedef struct _virNetworkDHCPLeases virNetworkDHCPLeases; +typedef virNetworkDHCPLeases *virNetworkDHCPLeasesPtr; +struct _virNetworkDHCPLeases { + 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 */ +}; + +void virNetworkDHCPLeaseFree(virNetworkDHCPLeasesPtr lease); + +int virNetworkGetDHCPLeases(virNetworkPtr network, + virNetworkDHCPLeasesPtr **leases, + unsigned int flags); + +int virNetworkGetDHCPLeasesForMAC(virNetworkPtr network, + const char *mac, + virNetworkDHCPLeasesPtr **leases, + unsigned int flags); + /* * virConnectListAllInterfaces: * diff --git a/src/Makefile.am b/src/Makefile.am index 87f5101..772171c 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -829,6 +829,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 \ @@ -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 \ @@ -2397,6 +2401,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/driver.h b/src/driver.h index 8cd164a..e000b17 100644 --- a/src/driver.h +++ b/src/driver.h @@ -1127,6 +1127,17 @@ typedef int int cookieinlen, unsigned int flags, int cancelled); +typedef int +(*virDrvNetworkGetDHCPLeases)(virNetworkPtr network, + virNetworkDHCPLeasesPtr **leases, + unsigned int flags); + +typedef int +(*virDrvNetworkGetDHCPLeasesForMAC)(virNetworkPtr network, + const char *mac, + virNetworkDHCPLeasesPtr **leases, + unsigned int flags); + typedef struct _virDriver virDriver; typedef virDriver *virDriverPtr; @@ -1458,6 +1469,8 @@ struct _virNetworkDriver { virDrvNetworkSetAutostart networkSetAutostart; virDrvNetworkIsActive networkIsActive; virDrvNetworkIsPersistent networkIsPersistent; + virDrvNetworkGetDHCPLeases networkGetDHCPLeases; + virDrvNetworkGetDHCPLeasesForMAC networkGetDHCPLeasesForMAC; }; 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" @@ -22051,3 +22052,199 @@ error: virDispatchError(dom->conn); return -1; } + +/** + * virNetworkGetDHCPLeases: + * @network: Pointer to network object + * @leases: Pointer to a variable to store the array containing details on + * obtained leases, or NULL if the list is not required (just returns + * number of leases). + * @flags: Extra flags, not used yet, so callers should always pass 0 + * + * For DHCPv4, the information returned: + * - Network Interface Name + * - Expiry Time + * - MAC address (can be NULL, only in rare cases) + * - IAID (NULL) + * - IPv4 address (with type and prefix) + * - Hostname (can be NULL) + * - Client ID (can be NULL) + * + * For DHCPv6, the information returned: + * - Network Interface Name + * - Expiry Time + * - MAC address (can be NULL, only in rare cases) + * - IAID (can be NULL, only in rare cases) + * - IPv6 address (with type and prefix) + * - Hostname (can be NULL) + * - Client DUID + * + * Note: @mac, @iaid, @ipaddr, @clientid are in ASCII form, not raw bytes. + * Note: @expirytime can 0, in case the lease is for infinite time. + * + * Returns the number of leases found or -1 and sets @leases to NULL in + * case of error. On success, the array stored into @leases is guaranteed to + * have an extra allocated element set to NULL but not included in the return + * count, to make iteration easier. The caller is responsible for calling + * virNetworkDHCPLeaseFree() on each array element, then calling free() on @leases. + * + * See also virNetworkGetDHCPLeasesForMAC() as a convenience for filtering + * the list to a single MAC address. + * + * Example of usage: + * + * virNetworkDHCPLeasesPtr *leases = NULL; + * virNetworkPtr network = ... obtain a network pointer here ...; + * size_t i; + * int nleases; + * unsigned int flags = 0; + * + * nleases = virNetworkGetDHCPLeases(network, &leases, flags); + * if (nleases < 0) + * error(); + * + * ... do something with returned values, for example: + * + * for (i = 0; i < nleases; i++) { + * virNetworkDHCPLeasesPtr lease = leases[i]; + * + * printf("Time(epoch): %lu, MAC address: %s, " + * "IP address: %s, Hostname: %s, ClientID: %s\n", + * leas->expirytime, lease->mac, lease->ipaddr, + * lease->hostname, lease->clientid); + * + * virNetworkDHCPLeaseFree(leases[i]); + * } + * + * free(leases); + * + */ +int +virNetworkGetDHCPLeases(virNetworkPtr network, + virNetworkDHCPLeasesPtr **leases, + unsigned int flags) +{ + virConnectPtr conn; + VIR_DEBUG("network=%p, leases=%p, flags=%x", + network, leases, flags); + + virResetLastError(); + + if (leases) + *leases = NULL; + + if (!VIR_IS_CONNECTED_NETWORK(network)) { + virLibNetworkError(VIR_ERR_INVALID_NETWORK, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + conn = network->conn; + + if (conn->networkDriver && conn->networkDriver->networkGetDHCPLeases) { + int ret; + ret = conn->networkDriver->networkGetDHCPLeases(network, leases, flags); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(network->conn); + return -1; +} + +/** + * virNetworkGetDHCPLeasesForMAC: + * @network: Pointer to network object + * @mac: ASCII formatted MAC address of an interface + * @leases: Pointer to a variable to store the array containing details on + * obtained leases, or NULL if the list is not required (just returns + * number of leases). + * @flags: extra flags, not used yet, so callers should always pass 0 + * + * The API fetches leases info of the interface which matches with the + * given @mac. There can be multiple leases for a single @mac because this + * API supports DHCPv6 too. + * + * Returns the number of leases found or -1 and sets @leases to NULL in case of + * error. On success, the array stored into @leases is guaranteed to have an + * extra allocated element set to NULL but not included in the return count, + * to make iteration easier. The caller is responsible for calling + * virNetworkDHCPLeaseFree() on each array element, then calling free() on @leases. + * + * See virNetworkGetDHCPLeases() for more details on list contents. + */ +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; + } + + conn = network->conn; + + if (conn->networkDriver && + conn->networkDriver->networkGetDHCPLeasesForMAC) { + int ret; + ret = conn->networkDriver->networkGetDHCPLeasesForMAC(network, mac, + leases, flags); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(network->conn); + return -1; +} + +/** + * virNetworkDHCPLeaseFree: + * @lease: pointer to a leases object + * + * Frees all the memory occupied by @lease. + */ +void +virNetworkDHCPLeaseFree(virNetworkDHCPLeasesPtr lease) +{ + if (!lease) + return; + VIR_FREE(lease->interface); + VIR_FREE(lease->mac); + VIR_FREE(lease->iaid); + VIR_FREE(lease->ipaddr); + VIR_FREE(lease->hostname); + VIR_FREE(lease->clientid); + VIR_FREE(lease); +} 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; + # .... define new API here using predicted next version number .... diff --git a/src/util/leaseshelper.c b/src/util/leaseshelper.c new file mode 100644 index 0000000..53d7d22 --- /dev/null +++ b/src/util/leaseshelper.c @@ -0,0 +1,185 @@ +/* + * leasehelper.c: Helper program to create custom leases file + * + * Copyright (C) 2013 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" +#include "configmake.h" + +#define VIR_FROM_THIS VIR_FROM_NETWORK + +/** + * 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 6 +/** + * 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 *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; + + 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); + + /* 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)); + + 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; +} -- 1.8.1.4

On Tue, Nov 26, 2013 at 2:35 AM, Nehal J Wani <nehaljw.kkd1@gmail.com> wrote:
Introduce 3 new APIs, virNetworkGetDHCPLeases, virNetworkGetDHCPLeasesForMAC and virNetworkDHCPLeaseFree.
* virNetworkGetDHCPLeases: returns the dhcp leases information for a given virtual network.
For DHCPv4, the information returned: - Network Interface Name - Expiry Time - MAC address (can be NULL, only in rare cases) - IAID (NULL) - IPv4 address (with type and prefix) - Hostname (can be NULL) - Client ID (can be NULL)
For DHCPv6, the information returned: - Network Interface Name - Expiry Time - MAC address (can be NULL, only in rare cases)
Maybe worth adding: MAC address in case of DHCPv6 will work only if the version of dnsmasq includes the change introduced by v2.67test15 (http://thekelleys.org.uk/gitweb/?p=dnsmasq.git;a=commit;h=89500e31f199e9ae1e...). Which means, the version should be greater than or equal to v2.67 (available in Fedora 20 and not in 19 or 18)
- IAID (can be NULL, only in rare cases) - IPv6 address (with type and prefix) - Hostname (can be NULL) - Client DUID

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

On Thu, Dec 12, 2013 at 5:14 PM, Daniel P. Berrange <berrange@redhat.com> wrote:
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.
The code didn't compile without making the above addition. It kept throwing the error: ../src/.libs/libvirt-setuid-rpc-client.a(libvirt_setuid_rpc_client_la-libvirt.o): In function `virNetworkGetDHCPLeasesForMAC': /home/Wani/Hobby/libvirt/src/libvirt.c:22187: undefined reference to `virMacAddrParse'
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.
Do you mean, I need to put this check inside networkGetDHCPLeasesForMAC in src/network/bridge_driver.c ?
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
I did run both the commands. Unfortunately, they didn't detect that I had been using getenv().
+ 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); }
Just a suggestion: It would be good if someone adds this to hacking.html
+ + 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.
Would do that in the next series. Thanks for reviewing the series. I'll continue to furnish 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 :|
-- Nehal J Wani

On Thu, Dec 12, 2013 at 06:00:17PM +0530, Nehal J Wani wrote:
On Thu, Dec 12, 2013 at 5:14 PM, Daniel P. Berrange <berrange@redhat.com> wrote:
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.
The code didn't compile without making the above addition. It kept throwing the error:
../src/.libs/libvirt-setuid-rpc-client.a(libvirt_setuid_rpc_client_la-libvirt.o): In function `virNetworkGetDHCPLeasesForMAC': /home/Wani/Hobby/libvirt/src/libvirt.c:22187: undefined reference to `virMacAddrParse'
That'd go away if you remove that check from the public API and put it in the driver code instead.
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.
Do you mean, I need to put this check inside networkGetDHCPLeasesForMAC in src/network/bridge_driver.c ?
If you need to parse it, then yes, but if you don't need to parse it then don't. 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 :|

Implement RPC calls for virNetworkGetDHCPLeases, virNetworkGetDHCPLeasesForMAC daemon/remote.c * Define remoteSerializeNetworkDHCPLeases, remoteDispatchNetworkGetDHCPLeases * Define remoteDispatchNetworkGetDHCPLeasesForMAC * Define helper function remoteSerializeDHCPLease src/remote/remote_driver.c * Define remoteNetworkGetDHCPLeases * Define remoteNetworkGetDHCPLeasesForMAC * Define helper function remoteSerializeDHCPLease src/remote/remote_protocol.x * New RPC procedure: REMOTE_PROC_NETWORK_GET_DHCP_LEASES * Define structs remote_network_dhcp_leases, remote_network_get_dhcp_leases_args, remote_network_get_dhcp_leases_ret * New RPC procedure: REMOTE_PROC_NETWORK_GET_DHCP_LEASES_FOR_MAC * Define structs remote_network_dhcp_leases_for_mac, remote_network_get_dhcp_leases_for_mac_args, remote_network_get_dhcp_leases_for_mac_ret src/remote_protocol-structs * New structs added src/rpc/gendispatch.pl * Add exception (s/Dhcp/DHCP) for auto-generating names of the remote functions in daemon/remote_dispatch.h --- daemon/remote.c | 176 ++++++++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 187 +++++++++++++++++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 54 ++++++++++++- src/remote_protocol-structs | 38 +++++++++ src/rpc/gendispatch.pl | 1 + 5 files changed, 455 insertions(+), 1 deletion(-) diff --git a/daemon/remote.c b/daemon/remote.c index decaecc..c2f13a8 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -5215,6 +5215,182 @@ cleanup: return rv; } +/* Copy contents of virNetworkDHCPLeasesPtr to remote_network_dhcp_lease */ +static int +remoteSerializeDHCPLease(remote_network_dhcp_lease *lease_dst, virNetworkDHCPLeasesPtr lease_src) +{ + char **mac_tmp = NULL; + char **iaid_tmp = NULL; + char **hostname_tmp = NULL; + char **clientid_tmp = NULL; + + if (VIR_ALLOC(mac_tmp) < 0 || + VIR_ALLOC(iaid_tmp) < 0 || + VIR_ALLOC(hostname_tmp) < 0 || + VIR_ALLOC(clientid_tmp) < 0) + goto error; + + lease_dst->expirytime = lease_src->expirytime; + lease_dst->type = lease_src->type; + lease_dst->prefix = lease_src->prefix; + + if (VIR_STRDUP(lease_dst->interface, lease_src->interface) < 0 || + VIR_STRDUP(lease_dst->ipaddr, lease_src->ipaddr) < 0 || + VIR_STRDUP(*mac_tmp, lease_src->mac) < 0 || + VIR_STRDUP(*iaid_tmp, lease_src->iaid) < 0 || + VIR_STRDUP(*hostname_tmp, lease_src->hostname) < 0 || + VIR_STRDUP(*clientid_tmp, lease_src->clientid) < 0) + goto error; + + lease_dst->mac = *mac_tmp ? mac_tmp : NULL; + lease_dst->iaid = *iaid_tmp ? iaid_tmp : NULL; + lease_dst->hostname = *hostname_tmp ? hostname_tmp : NULL; + lease_dst->clientid = *clientid_tmp ? clientid_tmp : NULL; + + return 0; + +error: + VIR_FREE(mac_tmp); + VIR_FREE(iaid_tmp); + VIR_FREE(hostname_tmp); + VIR_FREE(clientid_tmp); + return -1; +} + +static int +remoteDispatchNetworkGetDHCPLeases(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_network_get_dhcp_leases_args *args, + remote_network_get_dhcp_leases_ret *ret) +{ + int rv = -1; + size_t i; + struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); + virNetworkDHCPLeasesPtr *leases = NULL; + virNetworkPtr net = NULL; + int nleases = 0; + + if (!priv->conn) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + if (!(net = get_nonnull_network(priv->conn, args->net))) + goto cleanup; + + if ((nleases = virNetworkGetDHCPLeases(net, + args->need_results ? &leases : NULL, + args->flags)) < 0) + goto cleanup; + + if (nleases > REMOTE_NETWORK_DHCP_LEASES_MAX) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Number of leases is %d, which exceeds max limit: %d"), + nleases, REMOTE_NETWORK_DHCP_LEASES_MAX); + return -1; + } + + if (leases && nleases) { + if (VIR_ALLOC_N(ret->leases.leases_val, nleases) < 0) + goto cleanup; + + ret->leases.leases_len = nleases; + + for (i = 0; i < nleases; i++) { + if (remoteSerializeDHCPLease(ret->leases.leases_val + i, leases[i]) < 0) + goto cleanup; + } + + } else { + ret->leases.leases_len = 0; + ret->leases.leases_val = NULL; + } + + ret->ret = nleases; + + rv = 0; + +cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + if (leases) { + for (i = 0; i < nleases; i++) + virNetworkDHCPLeaseFree(leases[i]); + VIR_FREE(leases); + } + virNetworkFree(net); + return rv; +} + + +static int +remoteDispatchNetworkGetDHCPLeasesForMAC(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_network_get_dhcp_leases_for_mac_args *args, + remote_network_get_dhcp_leases_for_mac_ret *ret) +{ + int rv = -1; + size_t i; + struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); + virNetworkDHCPLeasesPtr *leases = NULL; + virNetworkPtr net = NULL; + int nleases = 0; + + if (!priv->conn) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + if (!(net = get_nonnull_network(priv->conn, args->net))) + goto cleanup; + + if ((nleases = virNetworkGetDHCPLeasesForMAC(net, args->mac, + args->need_results ? &leases : NULL, + args->flags)) < 0) + goto cleanup; + + if (nleases > REMOTE_NETWORK_DHCP_LEASES_MAX) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Number of leases is %d, which exceeds max limit: %d"), + nleases, REMOTE_NETWORK_DHCP_LEASES_MAX); + return -1; + } + + if (leases && nleases) { + if (VIR_ALLOC_N(ret->leases.leases_val, nleases) < 0) + goto cleanup; + + ret->leases.leases_len = nleases; + + for (i = 0; i < nleases; i++) { + if (remoteSerializeDHCPLease(ret->leases.leases_val + i, leases[i]) < 0) + goto cleanup; + } + + } else { + ret->leases.leases_len = 0; + ret->leases.leases_val = NULL; + } + + ret->ret = nleases; + + rv = 0; + +cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + if (leases) { + for (i = 0; i < nleases; i++) + virNetworkDHCPLeaseFree(leases[i]); + VIR_FREE(leases); + } + virNetworkFree(net); + return rv; +} /*----- Helpers. -----*/ diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index df7558b..83da5b2 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -1687,6 +1687,51 @@ error: return -1; } +/* Copy contents of remote_network_dhcp_lease to virNetworkDHCPLeasesPtr */ +static int +remoteSerializeDHCPLease(virNetworkDHCPLeasesPtr lease_dst, remote_network_dhcp_lease *lease_src) +{ + lease_dst->expirytime = lease_src->expirytime; + lease_dst->type = lease_src->type; + lease_dst->prefix = lease_src->prefix; + + if (VIR_STRDUP(lease_dst->interface, lease_src->interface) < 0) + goto error; + + if (VIR_STRDUP(lease_dst->ipaddr, lease_src->ipaddr) < 0) + goto error; + + if (lease_src->mac) { + if (VIR_STRDUP(lease_dst->mac, *lease_src->mac) < 0) + goto error; + } else + lease_src->mac = NULL; + + if (lease_src->iaid) { + if (VIR_STRDUP(lease_dst->iaid, *lease_src->iaid) < 0) + goto error; + } else + lease_src->iaid = NULL; + + if (lease_src->hostname) { + if (VIR_STRDUP(lease_dst->hostname, *lease_src->hostname) < 0) + goto error; + } else + lease_src->hostname = NULL; + + if (lease_src->clientid) { + if (VIR_STRDUP(lease_dst->clientid, *lease_src->clientid) < 0) + goto error; + } else + lease_src->clientid = NULL; + + return 0; + +error: + virNetworkDHCPLeaseFree(lease_dst); + return -1; +} + static int remoteDomainBlockStatsFlags(virDomainPtr domain, const char *path, @@ -6678,6 +6723,145 @@ done: return rv; } + +static int +remoteNetworkGetDHCPLeases(virNetworkPtr net, + virNetworkDHCPLeasesPtr **leases, + unsigned int flags) +{ + int rv = -1; + size_t i; + struct private_data *priv = net->conn->networkPrivateData; + remote_network_get_dhcp_leases_args args; + remote_network_get_dhcp_leases_ret ret; + + virNetworkDHCPLeasesPtr *leases_ret = NULL; + remoteDriverLock(priv); + + make_nonnull_network(&args.net, net); + args.flags = flags; + args.need_results = !!leases; + + memset(&ret, 0, sizeof(ret)); + + if (call(net->conn, priv, 0, REMOTE_PROC_NETWORK_GET_DHCP_LEASES, + (xdrproc_t)xdr_remote_network_get_dhcp_leases_args, (char *)&args, + (xdrproc_t)xdr_remote_network_get_dhcp_leases_ret, (char *)&ret) == -1) { + goto done; + } + + if (ret.leases.leases_len > REMOTE_NETWORK_DHCP_LEASES_MAX) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Number of leases is %d, which exceeds max limit: %d"), + ret.leases.leases_len, REMOTE_NETWORK_DHCP_LEASES_MAX); + goto cleanup; + } + + if (leases) { + if (ret.leases.leases_len && + VIR_ALLOC_N(leases_ret, ret.leases.leases_len + 1) < 0) { + goto cleanup; + } + + for (i = 0; i < ret.leases.leases_len; i++) { + if (VIR_ALLOC(leases_ret[i]) < 0) + goto cleanup; + + if (remoteSerializeDHCPLease(leases_ret[i], &ret.leases.leases_val[i]) < 0) + goto cleanup; + } + + *leases = leases_ret; + leases_ret = NULL; + } + + rv = ret.ret; + +cleanup: + if (leases_ret) { + for (i = 0; i < ret.leases.leases_len; i++) + virNetworkDHCPLeaseFree(leases_ret[i]); + VIR_FREE(leases_ret); + } + xdr_free((xdrproc_t)xdr_remote_network_get_dhcp_leases_ret, + (char *) &ret); + +done: + remoteDriverUnlock(priv); + return rv; +} + + +static int +remoteNetworkGetDHCPLeasesForMAC(virNetworkPtr net, + const char *mac, + virNetworkDHCPLeasesPtr **leases, + unsigned int flags) +{ + int rv = -1; + size_t i; + struct private_data *priv = net->conn->networkPrivateData; + remote_network_get_dhcp_leases_for_mac_args args; + remote_network_get_dhcp_leases_for_mac_ret ret; + + virNetworkDHCPLeasesPtr *leases_ret = NULL; + remoteDriverLock(priv); + + make_nonnull_network(&args.net, net); + args.mac = (char *) mac; + args.flags = flags; + args.need_results = !!leases; + + memset(&ret, 0, sizeof(ret)); + + if (call(net->conn, priv, 0, REMOTE_PROC_NETWORK_GET_DHCP_LEASES_FOR_MAC, + (xdrproc_t)xdr_remote_network_get_dhcp_leases_for_mac_args, (char *)&args, + (xdrproc_t)xdr_remote_network_get_dhcp_leases_for_mac_ret, (char *)&ret) == -1) { + goto done; + } + + if (ret.leases.leases_len > REMOTE_NETWORK_DHCP_LEASES_MAX) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Number of leases is %d, which exceeds max limit: %d"), + ret.leases.leases_len, REMOTE_NETWORK_DHCP_LEASES_MAX); + goto cleanup; + } + + if (leases) { + if (ret.leases.leases_len && + VIR_ALLOC_N(leases_ret, ret.leases.leases_len + 1) < 0) { + goto cleanup; + } + + for (i = 0; i < ret.leases.leases_len; i++) { + if (VIR_ALLOC(leases_ret[i]) < 0) + goto cleanup; + + if (remoteSerializeDHCPLease(leases_ret[i], &ret.leases.leases_val[i]) < 0) + goto cleanup; + } + + *leases = leases_ret; + leases_ret = NULL; + } + + rv = ret.ret; + +cleanup: + if (leases_ret) { + for (i = 0; i < ret.leases.leases_len; i++) + virNetworkDHCPLeaseFree(leases_ret[i]); + VIR_FREE(leases_ret); + } + xdr_free((xdrproc_t)xdr_remote_network_get_dhcp_leases_for_mac_ret, + (char *) &ret); + +done: + remoteDriverUnlock(priv); + return rv; +} + + static void remoteDomainEventQueue(struct private_data *priv, virDomainEventPtr event) { @@ -6810,6 +6994,7 @@ make_nonnull_domain_snapshot(remote_nonnull_domain_snapshot *snapshot_dst, virDo make_nonnull_domain(&snapshot_dst->dom, snapshot_src->domain); } + /*----------------------------------------------------------------------*/ unsigned long remoteVersion(void) @@ -7038,6 +7223,8 @@ static virNetworkDriver network_driver = { .networkSetAutostart = remoteNetworkSetAutostart, /* 0.3.0 */ .networkIsActive = remoteNetworkIsActive, /* 0.7.3 */ .networkIsPersistent = remoteNetworkIsPersistent, /* 0.7.3 */ + .networkGetDHCPLeases = remoteNetworkGetDHCPLeases, /* 1.1.4 */ + .networkGetDHCPLeasesForMAC = remoteNetworkGetDHCPLeasesForMAC, /* 1.1.4 */ }; static virInterfaceDriver interface_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index f942670..60939b1 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -235,6 +235,11 @@ const REMOTE_DOMAIN_JOB_STATS_MAX = 64; /* Upper limit on number of CPU models */ const REMOTE_CONNECT_CPU_MODELS_MAX = 8192; +/* + * Upper limit on the maximum number of leases in one lease file + */ +const REMOTE_NETWORK_DHCP_LEASES_MAX = 32768; + /* UUID. VIR_UUID_BUFLEN definition comes from libvirt.h */ typedef opaque remote_uuid[VIR_UUID_BUFLEN]; @@ -2849,6 +2854,41 @@ struct remote_connect_get_cpu_model_names_ret { int ret; }; +struct remote_network_dhcp_lease { + remote_nonnull_string interface; + hyper expirytime; + int type; + remote_string mac; + remote_string iaid; + remote_nonnull_string ipaddr; + unsigned int prefix; + remote_string hostname; + remote_string clientid; +}; + +struct remote_network_get_dhcp_leases_args { + remote_nonnull_network net; + int need_results; + unsigned int flags; +}; + +struct remote_network_get_dhcp_leases_ret { + remote_network_dhcp_lease leases<REMOTE_NETWORK_DHCP_LEASES_MAX>; + unsigned int ret; +}; + +struct remote_network_get_dhcp_leases_for_mac_args { + remote_nonnull_network net; + remote_nonnull_string mac; + int need_results; + unsigned int flags; +}; + +struct remote_network_get_dhcp_leases_for_mac_ret { + remote_network_dhcp_lease leases<REMOTE_NETWORK_DHCP_LEASES_MAX>; + unsigned int ret; +}; + /*----- Protocol. -----*/ /* Define the program number, protocol version and procedure numbers here. */ @@ -5018,5 +5058,17 @@ enum remote_procedure { * @generate: none * @acl: connect:read */ - REMOTE_PROC_CONNECT_GET_CPU_MODEL_NAMES = 312 + REMOTE_PROC_CONNECT_GET_CPU_MODEL_NAMES = 312, + + /** + * @generate: none + * @acl: network:read + */ + REMOTE_PROC_NETWORK_GET_DHCP_LEASES = 313, + + /** + * @generate: none + * @acl: network:read + */ + REMOTE_PROC_NETWORK_GET_DHCP_LEASES_FOR_MAC = 314 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 98d2d5b..8d533c9 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2328,6 +2328,42 @@ struct remote_connect_get_cpu_model_names_ret { } models; int ret; }; +struct remote_network_dhcp_lease { + remote_nonnull_string interface; + int64_t expirytime; + int type; + remote_string mac; + remote_string iaid; + remote_nonnull_string ipaddr; + u_int prefix; + remote_string hostname; + remote_string clientid; +}; +struct remote_network_get_dhcp_leases_args { + remote_nonnull_network net; + int need_results; + u_int flags; +}; +struct remote_network_get_dhcp_leases_ret { + struct { + u_int leases_len; + remote_network_dhcp_lease * leases_val; + } leases; + u_int ret; +}; +struct remote_network_get_dhcp_leases_for_mac_args { + remote_nonnull_network net; + remote_nonnull_string mac; + int need_results; + u_int flags; +}; +struct remote_network_get_dhcp_leases_for_mac_ret { + struct { + u_int leases_len; + remote_network_dhcp_lease * leases_val; + } leases; + u_int ret; +}; enum remote_procedure { REMOTE_PROC_CONNECT_OPEN = 1, REMOTE_PROC_CONNECT_CLOSE = 2, @@ -2641,4 +2677,6 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_CREATE_WITH_FILES = 310, REMOTE_PROC_DOMAIN_EVENT_DEVICE_REMOVED = 311, REMOTE_PROC_CONNECT_GET_CPU_MODEL_NAMES = 312, + REMOTE_PROC_NETWORK_GET_DHCP_LEASES = 313, + REMOTE_PROC_NETWORK_GET_DHCP_LEASES_FOR_MAC = 314, }; diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index ceb1ad8..5a503cf 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -66,6 +66,7 @@ sub fixup_name { $name =~ s/Fstrim$/FSTrim/; $name =~ s/Scsi/SCSI/; $name =~ s/Wwn$/WWN/; + $name =~ s/Dhcp$/DHCP/; return $name; } -- 1.8.1.4

On Tue, Nov 26, 2013 at 02:35:59AM +0530, Nehal J Wani wrote:
diff --git a/daemon/remote.c b/daemon/remote.c index decaecc..c2f13a8 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -5215,6 +5215,182 @@ cleanup: return rv; }
+/* Copy contents of virNetworkDHCPLeasesPtr to remote_network_dhcp_lease */ +static int +remoteSerializeDHCPLease(remote_network_dhcp_lease *lease_dst, virNetworkDHCPLeasesPtr lease_src) +{ + char **mac_tmp = NULL; + char **iaid_tmp = NULL; + char **hostname_tmp = NULL; + char **clientid_tmp = NULL; + + if (VIR_ALLOC(mac_tmp) < 0 || + VIR_ALLOC(iaid_tmp) < 0 || + VIR_ALLOC(hostname_tmp) < 0 || + VIR_ALLOC(clientid_tmp) < 0) + goto error; + + lease_dst->expirytime = lease_src->expirytime; + lease_dst->type = lease_src->type; + lease_dst->prefix = lease_src->prefix; + + if (VIR_STRDUP(lease_dst->interface, lease_src->interface) < 0 || + VIR_STRDUP(lease_dst->ipaddr, lease_src->ipaddr) < 0 || + VIR_STRDUP(*mac_tmp, lease_src->mac) < 0 || + VIR_STRDUP(*iaid_tmp, lease_src->iaid) < 0 || + VIR_STRDUP(*hostname_tmp, lease_src->hostname) < 0 || + VIR_STRDUP(*clientid_tmp, lease_src->clientid) < 0) + goto error; + + lease_dst->mac = *mac_tmp ? mac_tmp : NULL; + lease_dst->iaid = *iaid_tmp ? iaid_tmp : NULL; + lease_dst->hostname = *hostname_tmp ? hostname_tmp : NULL; + lease_dst->clientid = *clientid_tmp ? clientid_tmp : NULL; + + return 0; + +error:
What about lease_dist->interface and lease_dst->ipaddr - shouldn't we free them too ?
+ VIR_FREE(mac_tmp); + VIR_FREE(iaid_tmp); + VIR_FREE(hostname_tmp); + VIR_FREE(clientid_tmp);
Leaking the contents of each of these ie you need VIR_FREE(*mac_tmp) before here too
+ return -1; +}
+ +static int +remoteDispatchNetworkGetDHCPLeases(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_network_get_dhcp_leases_args *args, + remote_network_get_dhcp_leases_ret *ret) +{ + int rv = -1; + size_t i; + struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); + virNetworkDHCPLeasesPtr *leases = NULL; + virNetworkPtr net = NULL; + int nleases = 0; + + if (!priv->conn) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + if (!(net = get_nonnull_network(priv->conn, args->net))) + goto cleanup; + + if ((nleases = virNetworkGetDHCPLeases(net, + args->need_results ? &leases : NULL, + args->flags)) < 0) + goto cleanup; + + if (nleases > REMOTE_NETWORK_DHCP_LEASES_MAX) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Number of leases is %d, which exceeds max limit: %d"), + nleases, REMOTE_NETWORK_DHCP_LEASES_MAX); + return -1;
'goto cleanup' otherwise you're leaking memory
+ } + + if (leases && nleases) { + if (VIR_ALLOC_N(ret->leases.leases_val, nleases) < 0) + goto cleanup; + + ret->leases.leases_len = nleases; + + for (i = 0; i < nleases; i++) { + if (remoteSerializeDHCPLease(ret->leases.leases_val + i, leases[i]) < 0) + goto cleanup; + } + + } else { + ret->leases.leases_len = 0; + ret->leases.leases_val = NULL; + } + + ret->ret = nleases; + + rv = 0; + +cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + if (leases) { + for (i = 0; i < nleases; i++) + virNetworkDHCPLeaseFree(leases[i]); + VIR_FREE(leases); + } + virNetworkFree(net); + return rv; +} + + +static int +remoteDispatchNetworkGetDHCPLeasesForMAC(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_network_get_dhcp_leases_for_mac_args *args, + remote_network_get_dhcp_leases_for_mac_ret *ret) +{ + int rv = -1; + size_t i; + struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); + virNetworkDHCPLeasesPtr *leases = NULL; + virNetworkPtr net = NULL; + int nleases = 0; + + if (!priv->conn) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + if (!(net = get_nonnull_network(priv->conn, args->net))) + goto cleanup; + + if ((nleases = virNetworkGetDHCPLeasesForMAC(net, args->mac, + args->need_results ? &leases : NULL, + args->flags)) < 0) + goto cleanup; + + if (nleases > REMOTE_NETWORK_DHCP_LEASES_MAX) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Number of leases is %d, which exceeds max limit: %d"), + nleases, REMOTE_NETWORK_DHCP_LEASES_MAX); + return -1; + } + + if (leases && nleases) { + if (VIR_ALLOC_N(ret->leases.leases_val, nleases) < 0) + goto cleanup; + + ret->leases.leases_len = nleases; + + for (i = 0; i < nleases; i++) { + if (remoteSerializeDHCPLease(ret->leases.leases_val + i, leases[i]) < 0) + goto cleanup; + } + + } else { + ret->leases.leases_len = 0; + ret->leases.leases_val = NULL; + } + + ret->ret = nleases; + + rv = 0; + +cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + if (leases) { + for (i = 0; i < nleases; i++) + virNetworkDHCPLeaseFree(leases[i]); + VIR_FREE(leases); + } + virNetworkFree(net); + return rv; +}
/*----- Helpers. -----*/ diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index df7558b..83da5b2 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -1687,6 +1687,51 @@ error: return -1; }
+/* Copy contents of remote_network_dhcp_lease to virNetworkDHCPLeasesPtr */ +static int +remoteSerializeDHCPLease(virNetworkDHCPLeasesPtr lease_dst, remote_network_dhcp_lease *lease_src) +{ + lease_dst->expirytime = lease_src->expirytime; + lease_dst->type = lease_src->type; + lease_dst->prefix = lease_src->prefix; + + if (VIR_STRDUP(lease_dst->interface, lease_src->interface) < 0) + goto error; + + if (VIR_STRDUP(lease_dst->ipaddr, lease_src->ipaddr) < 0) + goto error; + + if (lease_src->mac) { + if (VIR_STRDUP(lease_dst->mac, *lease_src->mac) < 0) + goto error; + } else + lease_src->mac = NULL; + + if (lease_src->iaid) { + if (VIR_STRDUP(lease_dst->iaid, *lease_src->iaid) < 0) + goto error; + } else + lease_src->iaid = NULL; + + if (lease_src->hostname) { + if (VIR_STRDUP(lease_dst->hostname, *lease_src->hostname) < 0) + goto error; + } else + lease_src->hostname = NULL; + + if (lease_src->clientid) { + if (VIR_STRDUP(lease_dst->clientid, *lease_src->clientid) < 0) + goto error; + } else + lease_src->clientid = NULL; + + return 0; + +error: + virNetworkDHCPLeaseFree(lease_dst); + return -1; +} + static int remoteDomainBlockStatsFlags(virDomainPtr domain, const char *path, @@ -6678,6 +6723,145 @@ done: return rv; }
+ +static int +remoteNetworkGetDHCPLeases(virNetworkPtr net, + virNetworkDHCPLeasesPtr **leases, + unsigned int flags) +{ + int rv = -1; + size_t i; + struct private_data *priv = net->conn->networkPrivateData; + remote_network_get_dhcp_leases_args args; + remote_network_get_dhcp_leases_ret ret; + + virNetworkDHCPLeasesPtr *leases_ret = NULL; + remoteDriverLock(priv); + + make_nonnull_network(&args.net, net); + args.flags = flags; + args.need_results = !!leases; + + memset(&ret, 0, sizeof(ret)); + + if (call(net->conn, priv, 0, REMOTE_PROC_NETWORK_GET_DHCP_LEASES, + (xdrproc_t)xdr_remote_network_get_dhcp_leases_args, (char *)&args, + (xdrproc_t)xdr_remote_network_get_dhcp_leases_ret, (char *)&ret) == -1) { + goto done; + } + + if (ret.leases.leases_len > REMOTE_NETWORK_DHCP_LEASES_MAX) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Number of leases is %d, which exceeds max limit: %d"), + ret.leases.leases_len, REMOTE_NETWORK_DHCP_LEASES_MAX); + goto cleanup; + } + + if (leases) { + if (ret.leases.leases_len && + VIR_ALLOC_N(leases_ret, ret.leases.leases_len + 1) < 0) { + goto cleanup; + } + + for (i = 0; i < ret.leases.leases_len; i++) { + if (VIR_ALLOC(leases_ret[i]) < 0) + goto cleanup; + + if (remoteSerializeDHCPLease(leases_ret[i], &ret.leases.leases_val[i]) < 0) + goto cleanup; + } + + *leases = leases_ret; + leases_ret = NULL; + } + + rv = ret.ret; + +cleanup: + if (leases_ret) { + for (i = 0; i < ret.leases.leases_len; i++) + virNetworkDHCPLeaseFree(leases_ret[i]); + VIR_FREE(leases_ret); + } + xdr_free((xdrproc_t)xdr_remote_network_get_dhcp_leases_ret, + (char *) &ret); + +done: + remoteDriverUnlock(priv); + return rv; +} + + +static int +remoteNetworkGetDHCPLeasesForMAC(virNetworkPtr net, + const char *mac, + virNetworkDHCPLeasesPtr **leases, + unsigned int flags) +{ + int rv = -1; + size_t i; + struct private_data *priv = net->conn->networkPrivateData; + remote_network_get_dhcp_leases_for_mac_args args; + remote_network_get_dhcp_leases_for_mac_ret ret; + + virNetworkDHCPLeasesPtr *leases_ret = NULL; + remoteDriverLock(priv); + + make_nonnull_network(&args.net, net); + args.mac = (char *) mac; + args.flags = flags; + args.need_results = !!leases; + + memset(&ret, 0, sizeof(ret)); + + if (call(net->conn, priv, 0, REMOTE_PROC_NETWORK_GET_DHCP_LEASES_FOR_MAC, + (xdrproc_t)xdr_remote_network_get_dhcp_leases_for_mac_args, (char *)&args, + (xdrproc_t)xdr_remote_network_get_dhcp_leases_for_mac_ret, (char *)&ret) == -1) { + goto done; + } + + if (ret.leases.leases_len > REMOTE_NETWORK_DHCP_LEASES_MAX) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Number of leases is %d, which exceeds max limit: %d"), + ret.leases.leases_len, REMOTE_NETWORK_DHCP_LEASES_MAX); + goto cleanup; + } + + if (leases) { + if (ret.leases.leases_len && + VIR_ALLOC_N(leases_ret, ret.leases.leases_len + 1) < 0) { + goto cleanup; + } + + for (i = 0; i < ret.leases.leases_len; i++) { + if (VIR_ALLOC(leases_ret[i]) < 0) + goto cleanup; + + if (remoteSerializeDHCPLease(leases_ret[i], &ret.leases.leases_val[i]) < 0) + goto cleanup; + } + + *leases = leases_ret; + leases_ret = NULL; + } + + rv = ret.ret; + +cleanup: + if (leases_ret) { + for (i = 0; i < ret.leases.leases_len; i++) + virNetworkDHCPLeaseFree(leases_ret[i]); + VIR_FREE(leases_ret); + } + xdr_free((xdrproc_t)xdr_remote_network_get_dhcp_leases_for_mac_ret, + (char *) &ret); + +done: + remoteDriverUnlock(priv); + return rv; +} + + static void remoteDomainEventQueue(struct private_data *priv, virDomainEventPtr event) { @@ -6810,6 +6994,7 @@ make_nonnull_domain_snapshot(remote_nonnull_domain_snapshot *snapshot_dst, virDo make_nonnull_domain(&snapshot_dst->dom, snapshot_src->domain); }
+ /*----------------------------------------------------------------------*/
unsigned long remoteVersion(void) @@ -7038,6 +7223,8 @@ static virNetworkDriver network_driver = { .networkSetAutostart = remoteNetworkSetAutostart, /* 0.3.0 */ .networkIsActive = remoteNetworkIsActive, /* 0.7.3 */ .networkIsPersistent = remoteNetworkIsPersistent, /* 0.7.3 */ + .networkGetDHCPLeases = remoteNetworkGetDHCPLeases, /* 1.1.4 */ + .networkGetDHCPLeasesForMAC = remoteNetworkGetDHCPLeasesForMAC, /* 1.1.4 */ };
static virInterfaceDriver interface_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index f942670..60939b1 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -235,6 +235,11 @@ const REMOTE_DOMAIN_JOB_STATS_MAX = 64; /* Upper limit on number of CPU models */ const REMOTE_CONNECT_CPU_MODELS_MAX = 8192;
+/* + * Upper limit on the maximum number of leases in one lease file + */ +const REMOTE_NETWORK_DHCP_LEASES_MAX = 32768; + /* UUID. VIR_UUID_BUFLEN definition comes from libvirt.h */ typedef opaque remote_uuid[VIR_UUID_BUFLEN];
@@ -2849,6 +2854,41 @@ struct remote_connect_get_cpu_model_names_ret { int ret; };
+struct remote_network_dhcp_lease { + remote_nonnull_string interface; + hyper expirytime; + int type; + remote_string mac; + remote_string iaid; + remote_nonnull_string ipaddr; + unsigned int prefix; + remote_string hostname; + remote_string clientid; +}; + +struct remote_network_get_dhcp_leases_args { + remote_nonnull_network net; + int need_results; + unsigned int flags; +}; + +struct remote_network_get_dhcp_leases_ret { + remote_network_dhcp_lease leases<REMOTE_NETWORK_DHCP_LEASES_MAX>; + unsigned int ret; +}; + +struct remote_network_get_dhcp_leases_for_mac_args { + remote_nonnull_network net; + remote_nonnull_string mac; + int need_results; + unsigned int flags; +}; + +struct remote_network_get_dhcp_leases_for_mac_ret { + remote_network_dhcp_lease leases<REMOTE_NETWORK_DHCP_LEASES_MAX>; + unsigned int ret; +}; +
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 :|

By querying the driver for the path of the custom leases file for the given virtual network and parsing it to retrieve info. src/network/bridge_driver.c: * Implement networkGetDHCPLeases * Implement networkGetDHCPLeasesForMAC * Implement networkGetDHCPLeasesHelper --- src/network/bridge_driver.c | 248 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 248 insertions(+) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 83dc931..1aedf86 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -109,6 +109,20 @@ static int networkPlugBandwidth(virNetworkObjPtr net, virDomainNetDefPtr iface); static int networkUnplugBandwidth(virNetworkObjPtr net, virDomainNetDefPtr iface); +/** + * 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 + +/** + * 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 6 static virNetworkDriverStatePtr driverState = NULL; @@ -147,6 +161,16 @@ networkDnsmasqLeaseFileNameFunc networkDnsmasqLeaseFileName = networkDnsmasqLeaseFileNameDefault; static char * +networkDnsmasqLeaseFileNameCustom(const char *bridge) +{ + char *leasefile; + + ignore_value(virAsprintf(&leasefile, "%s/%s.status", + driverState->dnsmasqStateDir, bridge)); + return leasefile; +} + +static char * networkDnsmasqConfigFileName(const char *netname) { char *conffile; @@ -182,6 +206,7 @@ networkRemoveInactive(virNetworkDriverStatePtr driver, virNetworkObjPtr net) { char *leasefile = NULL; + char *customleasefile = NULL; char *radvdconfigfile = NULL; char *configfile = NULL; char *radvdpidbase = NULL; @@ -200,6 +225,9 @@ networkRemoveInactive(virNetworkDriverStatePtr driver, if (!(leasefile = networkDnsmasqLeaseFileName(def->name))) goto cleanup; + if (!(customleasefile = networkDnsmasqLeaseFileNameCustom(def->bridge))) + goto cleanup; + if (!(radvdconfigfile = networkRadvdConfigFileName(def->name))) goto cleanup; @@ -216,6 +244,7 @@ networkRemoveInactive(virNetworkDriverStatePtr driver, /* dnsmasq */ dnsmasqDelete(dctx); unlink(leasefile); + unlink(customleasefile); unlink(configfile); /* radvd */ @@ -1058,6 +1087,9 @@ networkBuildDhcpDaemonCommandLine(virNetworkObjPtr network, cmd = virCommandNew(dnsmasqCapsGetBinaryPath(caps)); virCommandAddArgFormat(cmd, "--conf-file=%s", configfile); + + /* This helper is used to create cutom leases file for libvirt */ + virCommandAddArgFormat(cmd, "--dhcp-script=%s", LIBEXECDIR "/libvirt_leaseshelper"); *cmdout = cmd; ret = 0; cleanup: @@ -2991,6 +3023,220 @@ cleanup: return ret; } +/* This function parses the leases file of dnsmasq. + * + * An example of custom leases file content: + * + * 1385245780 52:54:00:2f:ba:76 * 192.168.150.153 * * + * 1385245781 52:54:00:2f:ba:76 3127926 2001:db8:ca2:2:1::6c * 00:04:76:00:cf:ae:b3:0b:fc:cd:0e:22:2e:97:76:65:74:ec + * 1385245964 52:54:00:44:7c:d7 * 192.168.150.219 iiit-ad885e4aa1 01:52:54:00:44:7c:d7 + * 1385245964 52:54:00:44:7c:d7 * 192.168.150.219 * 01:52:54:00:44:7c:d7 + * 1385246016 52:54:00:5d:99:92 * 192.168.150.212 iiit-ad885e4aa1 01:52:54:00:5d:99:92 + * 1385246041 52:54:00:3b:16:e0 * 192.168.150.207 * * + * 1385246081 52:54:00:db:dd:98 * 192.168.150.234 * * + * 1385246088 52:54:00:db:dd:98 14409112 2001:db8:ca2:2:1::6d * 00:04:76:00:cf:ae:b3:0b:fc:cd:0e:22:2e:97:76:65:74:ec + * + */ +static int +networkGetDHCPLeasesHelper(virNetworkObjPtr obj, + const char *mac, + virNetworkDHCPLeasesPtr **leases) +{ + int rv = -1; + size_t i = 0; + size_t nleases = 0; + char *lease_file = NULL; + char **lease_fields = NULL; + char *lease_entries = NULL; + char *lease_entry = NULL; + virNetworkDHCPLeasesPtr *leases_ret = NULL; + virNetworkDHCPLeasesPtr lease = NULL; + int lease_file_len = 0; + bool ipv6 = false; + bool need_results = !!leases; + + /* Retrieve custom leases file location */ + lease_file = networkDnsmasqLeaseFileNameCustom(obj->def->bridge); + + if ((lease_file_len = virFileReadAll(lease_file, + VIR_NETWORK_DHCP_LEASE_FILE_SIZE_MAX, + &lease_entries)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("No leases available for network: %s"), obj->def->name); + goto cleanup; + } + + lease_entry = lease_entries[0] == '\0' ? NULL : lease_entries; + + while (lease_entry) { + int nfields = 0; + ipv6 = false; + + char *eol = strchr(lease_entry, '\n'); + *eol = '\0'; + + /* Split the lease line */ + if (!(lease_fields = virStringSplit(lease_entry, " ", + VIR_NETWORK_DHCP_LEASE_FIELDS))) + goto error; + + 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) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Number of lease params aren't equal to: %d"), + VIR_NETWORK_DHCP_LEASE_FIELDS); + goto error; + } + + if (mac && virMacAddrCompare(mac, lease_fields[1])) { + virStringFreeList(lease_fields); + continue; + } + + if (need_results) { + if (VIR_ALLOC(lease) < 0) + goto error; + + /* Convert expirytime here */ + if (virStrToLong_ll(lease_fields[0], NULL, 10, &(lease->expirytime)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to convert lease expiry time to integer: %s"), + lease_fields[0]); + goto error; + } + + /* Unlike IPv4, IPv6 uses ':' instead of '.' as separator */ + ipv6 = strchr(lease_fields[3], ':') ? true : false; + + lease->type = ipv6 ? VIR_IP_ADDR_TYPE_IPV6 : VIR_IP_ADDR_TYPE_IPV4; + lease->prefix = virSocketAddrGetIpPrefix(&obj->def->ips->address, + &obj->def->ips->netmask, + obj->def->ips->prefix); + + if ((VIR_STRDUP(lease->mac, lease_fields[1]) < 0) || + (VIR_STRDUP(lease->ipaddr, lease_fields[3]) < 0) || + (VIR_STRDUP(lease->interface, obj->def->bridge) < 0)) + goto error; + + /* IAID is NULL in case of ipv4 */ + if (STREQ(lease_fields[2], "*")) + lease->iaid = NULL; + else if (VIR_STRDUP(lease->iaid, lease_fields[2]) < 0) + goto error; + + /* Only two fields, hostname and clientid can be NULL + * Refer: http://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2013q3/007544.html + */ + if (STREQ(lease_fields[4], "*")) + lease->hostname = NULL; + else if (VIR_STRDUP(lease->hostname, lease_fields[4]) < 0) + goto error; + + if (STREQ(lease_fields[5], "*")) + lease->clientid = NULL; + else if (VIR_STRDUP(lease->clientid, lease_fields[5]) < 0) + goto error; + + if (VIR_INSERT_ELEMENT(leases_ret, nleases, nleases, lease) < 0) + goto error; + } + else + nleases++; + + VIR_FREE(lease); + virStringFreeList(lease_fields); + } + + lease_fields = NULL; + + if (need_results && mac && !leases_ret) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("no lease with matching MAC address: %s"), mac); + goto error; + } + + if (leases_ret) { + /* NULL terminated array */ + ignore_value(VIR_REALLOC_N(leases_ret, nleases + 1)); + *leases = leases_ret; + leases_ret = NULL; + } + + rv = nleases; + +cleanup: + VIR_FREE(lease); + VIR_FREE(lease_file); + VIR_FREE(lease_entries); + if (lease_fields) + virStringFreeList(lease_fields); + return rv; + +error: + if (leases_ret) { + for (i = 0; i < nleases; i++) + virNetworkDHCPLeaseFree(leases_ret[i]); + VIR_FREE(leases_ret); + } + goto cleanup; +} + +static int +networkGetDHCPLeases(virNetworkPtr network, + virNetworkDHCPLeasesPtr **leases, + unsigned int flags) +{ + int rv = -1; + virNetworkObjPtr obj; + + virCheckFlags(0, -1); + + if (!(obj = networkObjFromNetwork(network))) + return rv; + + if (virNetworkGetDHCPLeasesEnsureACL(network->conn, obj->def) < 0) + goto cleanup; + + rv = networkGetDHCPLeasesHelper(obj, NULL, leases); + +cleanup: + if (obj) + virNetworkObjUnlock(obj); + return rv; +} + +static int +networkGetDHCPLeasesForMAC(virNetworkPtr network, + const char *mac, + virNetworkDHCPLeasesPtr **leases, + unsigned int flags) +{ + int rv = -1; + virNetworkObjPtr obj; + + virCheckFlags(0, -1); + + if (!(obj = networkObjFromNetwork(network))) + return rv; + + if (virNetworkGetDHCPLeasesForMACEnsureACL(network->conn, obj->def) < 0) + goto cleanup; + + rv = networkGetDHCPLeasesHelper(obj, mac, leases); + +cleanup: + if (obj) + virNetworkObjUnlock(obj); + return rv; +} static virNetworkDriver networkDriver = { "Network", @@ -3015,6 +3261,8 @@ static virNetworkDriver networkDriver = { .networkSetAutostart = networkSetAutostart, /* 0.2.1 */ .networkIsActive = networkIsActive, /* 0.7.3 */ .networkIsPersistent = networkIsPersistent, /* 0.7.3 */ + .networkGetDHCPLeases = networkGetDHCPLeases, /* 1.1.4 */ + .networkGetDHCPLeasesForMAC = networkGetDHCPLeasesForMAC, /* 1.1.4 */ }; static virStateDriver networkStateDriver = { -- 1.8.1.4

On Tue, Nov 26, 2013 at 02:36:00AM +0530, Nehal J Wani wrote:
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 83dc931..1aedf86 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c
@@ -2991,6 +3023,220 @@ cleanup: return ret; }
+/* This function parses the leases file of dnsmasq. + * + * An example of custom leases file content: + * + * 1385245780 52:54:00:2f:ba:76 * 192.168.150.153 * * + * 1385245781 52:54:00:2f:ba:76 3127926 2001:db8:ca2:2:1::6c * 00:04:76:00:cf:ae:b3:0b:fc:cd:0e:22:2e:97:76:65:74:ec + * 1385245964 52:54:00:44:7c:d7 * 192.168.150.219 iiit-ad885e4aa1 01:52:54:00:44:7c:d7 + * 1385245964 52:54:00:44:7c:d7 * 192.168.150.219 * 01:52:54:00:44:7c:d7 + * 1385246016 52:54:00:5d:99:92 * 192.168.150.212 iiit-ad885e4aa1 01:52:54:00:5d:99:92 + * 1385246041 52:54:00:3b:16:e0 * 192.168.150.207 * * + * 1385246081 52:54:00:db:dd:98 * 192.168.150.234 * * + * 1385246088 52:54:00:db:dd:98 14409112 2001:db8:ca2:2:1::6d * 00:04:76:00:cf:ae:b3:0b:fc:cd:0e:22:2e:97:76:65:74:ec + * + */ +static int +networkGetDHCPLeasesHelper(virNetworkObjPtr obj, + const char *mac, + virNetworkDHCPLeasesPtr **leases) +{ + int rv = -1; + size_t i = 0; + size_t nleases = 0; + char *lease_file = NULL; + char **lease_fields = NULL; + char *lease_entries = NULL; + char *lease_entry = NULL; + virNetworkDHCPLeasesPtr *leases_ret = NULL; + virNetworkDHCPLeasesPtr lease = NULL; + int lease_file_len = 0; + bool ipv6 = false; + bool need_results = !!leases; + + /* Retrieve custom leases file location */ + lease_file = networkDnsmasqLeaseFileNameCustom(obj->def->bridge); + + if ((lease_file_len = virFileReadAll(lease_file, + VIR_NETWORK_DHCP_LEASE_FILE_SIZE_MAX, + &lease_entries)) < 0) {
Small indentation alignemnt bug.
+ virReportError(VIR_ERR_INTERNAL_ERROR, + _("No leases available for network: %s"), obj->def->name);
Hmm, if we have started the network, but no VMs have yet booted, then will we find that the lease file does not exist ? Or is the lease file guaranteed to exist, even if no leases have been assigned yet. Basically we must make sure we don't return an error if no leases have been handed out yet - just return 0 lease results.
+ goto cleanup; + } + + lease_entry = lease_entries[0] == '\0' ? NULL : lease_entries; + + while (lease_entry) { + int nfields = 0; + ipv6 = false; + + char *eol = strchr(lease_entry, '\n'); + *eol = '\0'; + + /* Split the lease line */ + if (!(lease_fields = virStringSplit(lease_entry, " ", + VIR_NETWORK_DHCP_LEASE_FIELDS))) + goto error; + + 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) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Number of lease params aren't equal to: %d"), + VIR_NETWORK_DHCP_LEASE_FIELDS); + goto error; + } + + if (mac && virMacAddrCompare(mac, lease_fields[1])) { + virStringFreeList(lease_fields); + continue; + } + + if (need_results) { + if (VIR_ALLOC(lease) < 0) + goto error; + + /* Convert expirytime here */ + if (virStrToLong_ll(lease_fields[0], NULL, 10, &(lease->expirytime)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to convert lease expiry time to integer: %s"), + lease_fields[0]); + goto error; + } + + /* Unlike IPv4, IPv6 uses ':' instead of '.' as separator */ + ipv6 = strchr(lease_fields[3], ':') ? true : false; + + lease->type = ipv6 ? VIR_IP_ADDR_TYPE_IPV6 : VIR_IP_ADDR_TYPE_IPV4; + lease->prefix = virSocketAddrGetIpPrefix(&obj->def->ips->address, + &obj->def->ips->netmask, + obj->def->ips->prefix); + + if ((VIR_STRDUP(lease->mac, lease_fields[1]) < 0) || + (VIR_STRDUP(lease->ipaddr, lease_fields[3]) < 0) || + (VIR_STRDUP(lease->interface, obj->def->bridge) < 0)) + goto error; + + /* IAID is NULL in case of ipv4 */ + if (STREQ(lease_fields[2], "*")) + lease->iaid = NULL; + else if (VIR_STRDUP(lease->iaid, lease_fields[2]) < 0) + goto error; + + /* Only two fields, hostname and clientid can be NULL + * Refer: http://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2013q3/007544.html + */ + if (STREQ(lease_fields[4], "*")) + lease->hostname = NULL; + else if (VIR_STRDUP(lease->hostname, lease_fields[4]) < 0) + goto error; + + if (STREQ(lease_fields[5], "*")) + lease->clientid = NULL; + else if (VIR_STRDUP(lease->clientid, lease_fields[5]) < 0) + goto error; + + if (VIR_INSERT_ELEMENT(leases_ret, nleases, nleases, lease) < 0) + goto error; + } + else + nleases++; + + VIR_FREE(lease); + virStringFreeList(lease_fields); + } + + lease_fields = NULL; + + if (need_results && mac && !leases_ret) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("no lease with matching MAC address: %s"), mac); + goto error; + } + + if (leases_ret) { + /* NULL terminated array */ + ignore_value(VIR_REALLOC_N(leases_ret, nleases + 1)); + *leases = leases_ret; + leases_ret = NULL; + } + + rv = nleases; + +cleanup: + VIR_FREE(lease); + VIR_FREE(lease_file); + VIR_FREE(lease_entries); + if (lease_fields) + virStringFreeList(lease_fields); + return rv; + +error: + if (leases_ret) { + for (i = 0; i < nleases; i++) + virNetworkDHCPLeaseFree(leases_ret[i]); + VIR_FREE(leases_ret); + } + goto cleanup; +} + +static int +networkGetDHCPLeases(virNetworkPtr network, + virNetworkDHCPLeasesPtr **leases, + unsigned int flags) +{ + int rv = -1; + virNetworkObjPtr obj; + + virCheckFlags(0, -1); + + if (!(obj = networkObjFromNetwork(network))) + return rv; + + if (virNetworkGetDHCPLeasesEnsureACL(network->conn, obj->def) < 0) + goto cleanup; + + rv = networkGetDHCPLeasesHelper(obj, NULL, leases); + +cleanup: + if (obj) + virNetworkObjUnlock(obj); + return rv; +} + +static int +networkGetDHCPLeasesForMAC(virNetworkPtr network, + const char *mac, + virNetworkDHCPLeasesPtr **leases, + unsigned int flags) +{ + int rv = -1; + virNetworkObjPtr obj; + + virCheckFlags(0, -1); + + if (!(obj = networkObjFromNetwork(network))) + return rv; + + if (virNetworkGetDHCPLeasesForMACEnsureACL(network->conn, obj->def) < 0) + goto cleanup; + + rv = networkGetDHCPLeasesHelper(obj, mac, leases); + +cleanup: + if (obj) + virNetworkObjUnlock(obj); + return rv; +}
static virNetworkDriver networkDriver = { "Network", @@ -3015,6 +3261,8 @@ static virNetworkDriver networkDriver = { .networkSetAutostart = networkSetAutostart, /* 0.2.1 */ .networkIsActive = networkIsActive, /* 0.7.3 */ .networkIsPersistent = networkIsPersistent, /* 0.7.3 */ + .networkGetDHCPLeases = networkGetDHCPLeases, /* 1.1.4 */ + .networkGetDHCPLeasesForMAC = networkGetDHCPLeasesForMAC, /* 1.1.4 */
s/1.1.4/1.2.1/ now Regards, 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 :|

+ virReportError(VIR_ERR_INTERNAL_ERROR, + _("No leases available for network: %s"), obj->def->name);
Hmm, if we have started the network, but no VMs have yet booted, then will we find that the lease file does not exist ? Or is the lease file guaranteed to exist, even if no leases have been assigned yet.
Basically we must make sure we don't return an error if no leases have been handed out yet - just return 0 lease results.
The custom lease file - <network-name>.status is not guaranteed to exist. It will exist only if lease(s) have been handed out for that network or the lease(s) have expired but the network is still on. So, I'll just add rv = 0 in the above block before going to cleanup, so that error is not reported in case the file doesn't exist. -- Nehal J Wani

Use virNetworkGetDHCPLeases and virNetworkGetDHCPLeasesForMAC in virsh. The new feature supports the follwing methods: 1. Retrieve leases info for a given virtual network 2. Retrieve leases info for given network interface tools/virsh-domain-monitor.c * Introduce new command : net-dhcp-leases Example Usage: net-dhcp-leases <network> [mac] virsh # net-dhcp-leases --network default6 Expiry Time MAC address Protocol IP address Hostname Client ID or DUID ------------------------------------------------------------------------------------------------------------------- 2013-11-24 03:59:40 52:54:00:2f:ba:76 ipv4 192.168.150.153/24 (null) (null) 2013-11-24 03:59:41 52:54:00:2f:ba:76 ipv6 2001:db8:ca2:2:1::6c/24 (null) 00:04:76:00:cf:ae:b3:0b:fc:cd:0e:22:2e:97:76:65:74:ec 2013-11-24 04:04:01 52:54:00:3b:16:e0 ipv4 192.168.150.207/24 (null) (null) 2013-11-24 04:02:44 52:54:00:44:7c:d7 ipv4 192.168.150.219/24 iiit-ad885e4aa1 01:52:54:00:44:7c:d7 2013-11-24 04:02:44 52:54:00:44:7c:d7 ipv4 192.168.150.219/24 (null) 01:52:54:00:44:7c:d7 2013-11-24 04:03:36 52:54:00:5d:99:92 ipv4 192.168.150.212/24 iiit-ad885e4aa1 01:52:54:00:5d:99:92 2013-11-24 04:04:41 52:54:00:db:dd:98 ipv4 192.168.150.234/24 (null) (null) 2013-11-24 04:04:48 52:54:00:db:dd:98 ipv6 2001:db8:ca2:2:1::6d/24 (null) 00:04:76:00:cf:ae:b3:0b:fc:cd:0e:22:2e:97:76:65:74:ec tools/virsh.pod * Document new command --- tools/virsh-network.c | 118 ++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 6 +++ 2 files changed, 124 insertions(+) diff --git a/tools/virsh-network.c b/tools/virsh-network.c index 44a676b..270f91b 100644 --- a/tools/virsh-network.c +++ b/tools/virsh-network.c @@ -1129,6 +1129,118 @@ cmdNetworkEdit(vshControl *ctl, const vshCmd *cmd) return ret; } +/* + * "net-dhcp-leases" command + */ +static const vshCmdInfo info_network_dhcp_leases[] = { + {.name = "help", + .data = N_("print lease info for a given network") + }, + {.name = "desc", + .data = N_("Print lease info for a given network") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_network_dhcp_leases[] = { + {.name = "network", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("network name or uuid") + }, + {.name = "mac", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_NONE, + .help = N_("MAC address") + }, + {.name = NULL} +}; + +static int +vshNetworkDHCPLeaseSorter(const void *a, const void *b) +{ + int rv = -1; + + virNetworkDHCPLeasesPtr *lease1 = (virNetworkDHCPLeasesPtr *) a; + virNetworkDHCPLeasesPtr *lease2 = (virNetworkDHCPLeasesPtr *) b; + + if (*lease1 && !*lease2) + return -1; + + if (!*lease1) + return *lease2 != NULL; + + rv = vshStrcasecmp((*lease1)->mac, (*lease2)->mac); + return rv; +} + +static bool +cmdNetworkDHCPLeases(vshControl *ctl, const vshCmd *cmd) +{ + const char *name = NULL; + const char *mac = NULL; + virNetworkDHCPLeasesPtr *leases = NULL; + int nleases = 0; + bool ret = false; + size_t i; + unsigned int flags = 0; + virNetworkPtr network = NULL; + + if (vshCommandOptString(cmd, "mac", &mac) < 0) + return false; + + if (!(network = vshCommandOptNetwork(ctl, cmd, &name))) + return false; + + nleases = mac ? virNetworkGetDHCPLeasesForMAC(network, mac, &leases, flags) + : virNetworkGetDHCPLeases(network, &leases, flags); + + if (nleases < 0) { + vshError(ctl, _("Failed to get leases info for %s"), name); + goto cleanup; + } + + /* Sort the list according to MAC Address/IAID */ + qsort(leases, nleases, sizeof(*leases), vshNetworkDHCPLeaseSorter); + + vshPrintExtra(ctl, " %-20s %-17s %-10s %-25s %-15s %s\n%s%s\n", + _("Expiry Time"), _("MAC address"), _("Protocol"), + _("IP address"), _("Hostname"), _("Client ID or DUID"), + "----------------------------------------------------------", + "---------------------------------------------------------"); + + for (i = 0; i < nleases; i++) { + const char *type = NULL; + char *cidr_format = NULL; + virNetworkDHCPLeasesPtr lease = leases[i]; + time_t expirytime_tmp = lease->expirytime; + struct tm ts; + char expirytime[32]; + ts = *localtime_r(&expirytime_tmp, &ts); + strftime(expirytime, sizeof(expirytime), "%Y-%m-%d %H:%M:%S", &ts); + + type = (lease->type == VIR_IP_ADDR_TYPE_IPV4) ? "ipv4" : + (lease->type == VIR_IP_ADDR_TYPE_IPV6) ? "ipv6" : ""; + + ignore_value(virAsprintf(&cidr_format, "%s/%d", + lease->ipaddr, lease->prefix)); + + vshPrintExtra(ctl, " %-20s %-17s %-10s %-25s %-15s %s\n", + expirytime, NULLSTR(lease->mac), NULLSTR(type), cidr_format, + NULLSTR(lease->hostname), NULLSTR(lease->clientid)); + } + + ret = true; + +cleanup: + if (leases) { + for (i = 0; i < nleases; i++) + virNetworkDHCPLeaseFree(leases[i]); + VIR_FREE(leases); + } + virNetworkFree(network); + return ret; +} const vshCmdDef networkCmds[] = { {.name = "net-autostart", @@ -1209,5 +1321,11 @@ const vshCmdDef networkCmds[] = { .info = info_network_uuid, .flags = 0 }, + {.name = "net-dhcp-leases", + .handler = cmdNetworkDHCPLeases, + .opts = opts_network_dhcp_leases, + .info = info_network_dhcp_leases, + .flags = 0, + }, {.name = NULL} }; diff --git a/tools/virsh.pod b/tools/virsh.pod index dac9a08..e47be66 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -2340,6 +2340,12 @@ If I<--current> is specified, affect the current network state. Both I<--live> and I<--config> flags may be given, but I<--current> is exclusive. Not specifying any flag is the same as specifying I<--current>. +=item B<net-dhcp-leases> I<network> [I<mac>] + +Get a list of dhcp leases for all network interfaces connected to the given +virtual I<network> or limited output just for one interface if I<mac> is +specified. + =back =head1 INTERFACE COMMANDS -- 1.8.1.4

On Tue, Nov 26, 2013 at 02:36:01AM +0530, Nehal J Wani wrote:
Use virNetworkGetDHCPLeases and virNetworkGetDHCPLeasesForMAC in virsh.
The new feature supports the follwing methods:
1. Retrieve leases info for a given virtual network
2. Retrieve leases info for given network interface
tools/virsh-domain-monitor.c * Introduce new command : net-dhcp-leases Example Usage: net-dhcp-leases <network> [mac]
virsh # net-dhcp-leases --network default6 Expiry Time MAC address Protocol IP address Hostname Client ID or DUID ------------------------------------------------------------------------------------------------------------------- 2013-11-24 03:59:40 52:54:00:2f:ba:76 ipv4 192.168.150.153/24 (null) (null) 2013-11-24 03:59:41 52:54:00:2f:ba:76 ipv6 2001:db8:ca2:2:1::6c/24 (null) 00:04:76:00:cf:ae:b3:0b:fc:cd:0e:22:2e:97:76:65:74:ec 2013-11-24 04:04:01 52:54:00:3b:16:e0 ipv4 192.168.150.207/24 (null) (null) 2013-11-24 04:02:44 52:54:00:44:7c:d7 ipv4 192.168.150.219/24 iiit-ad885e4aa1 01:52:54:00:44:7c:d7 2013-11-24 04:02:44 52:54:00:44:7c:d7 ipv4 192.168.150.219/24 (null) 01:52:54:00:44:7c:d7 2013-11-24 04:03:36 52:54:00:5d:99:92 ipv4 192.168.150.212/24 iiit-ad885e4aa1 01:52:54:00:5d:99:92 2013-11-24 04:04:41 52:54:00:db:dd:98 ipv4 192.168.150.234/24 (null) (null) 2013-11-24 04:04:48 52:54:00:db:dd:98 ipv6 2001:db8:ca2:2:1::6d/24 (null) 00:04:76:00:cf:ae:b3:0b:fc:cd:0e:22:2e:97:76:65:74:ec
I wonder if we should use '-' instead of '(null)' here - I think it is probably a bit more user friendly. 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 12/12/13, Daniel P. Berrange <berrange@redhat.com> wrote:
On Tue, Nov 26, 2013 at 02:36:01AM +0530, Nehal J Wani wrote:
Use virNetworkGetDHCPLeases and virNetworkGetDHCPLeasesForMAC in virsh.
The new feature supports the follwing methods:
1. Retrieve leases info for a given virtual network
2. Retrieve leases info for given network interface
tools/virsh-domain-monitor.c * Introduce new command : net-dhcp-leases Example Usage: net-dhcp-leases <network> [mac]
virsh # net-dhcp-leases --network default6 Expiry Time MAC address Protocol IP address Hostname Client ID or DUID
------------------------------------------------------------------------------------------------------------------- 2013-11-24 03:59:40 52:54:00:2f:ba:76 ipv4 192.168.150.153/24 (null) (null) 2013-11-24 03:59:41 52:54:00:2f:ba:76 ipv6 2001:db8:ca2:2:1::6c/24 (null) 00:04:76:00:cf:ae:b3:0b:fc:cd:0e:22:2e:97:76:65:74:ec 2013-11-24 04:04:01 52:54:00:3b:16:e0 ipv4 192.168.150.207/24 (null) (null) 2013-11-24 04:02:44 52:54:00:44:7c:d7 ipv4 192.168.150.219/24 iiit-ad885e4aa1 01:52:54:00:44:7c:d7 2013-11-24 04:02:44 52:54:00:44:7c:d7 ipv4 192.168.150.219/24 (null) 01:52:54:00:44:7c:d7 2013-11-24 04:03:36 52:54:00:5d:99:92 ipv4 192.168.150.212/24 iiit-ad885e4aa1 01:52:54:00:5d:99:92 2013-11-24 04:04:41 52:54:00:db:dd:98 ipv4 192.168.150.234/24 (null) (null) 2013-11-24 04:04:48 52:54:00:db:dd:98 ipv6 2001:db8:ca2:2:1::6d/24 (null) 00:04:76:00:cf:ae:b3:0b:fc:cd:0e:22:2e:97:76:65:74:ec
I wonder if we should use '-' instead of '(null)' here - I think it is probably a bit more user friendly.
dnsmasq uses '*' in case of null strings. Its a matter of choice. Since this user-friendly option should be available for all APIs, should I add: /* * Use this instead of NULLSTR to make it more user-friendly. */ # define EMPTYSTR(s) ((s) ? (s) : "*") in src/internal.h ? -- Nehal J Wani

On Mon, Dec 16, 2013 at 07:00:25PM +0530, Nehal J Wani wrote:
On 12/12/13, Daniel P. Berrange <berrange@redhat.com> wrote:
On Tue, Nov 26, 2013 at 02:36:01AM +0530, Nehal J Wani wrote:
Use virNetworkGetDHCPLeases and virNetworkGetDHCPLeasesForMAC in virsh.
The new feature supports the follwing methods:
1. Retrieve leases info for a given virtual network
2. Retrieve leases info for given network interface
tools/virsh-domain-monitor.c * Introduce new command : net-dhcp-leases Example Usage: net-dhcp-leases <network> [mac]
virsh # net-dhcp-leases --network default6 Expiry Time MAC address Protocol IP address Hostname Client ID or DUID
------------------------------------------------------------------------------------------------------------------- 2013-11-24 03:59:40 52:54:00:2f:ba:76 ipv4 192.168.150.153/24 (null) (null) 2013-11-24 03:59:41 52:54:00:2f:ba:76 ipv6 2001:db8:ca2:2:1::6c/24 (null) 00:04:76:00:cf:ae:b3:0b:fc:cd:0e:22:2e:97:76:65:74:ec 2013-11-24 04:04:01 52:54:00:3b:16:e0 ipv4 192.168.150.207/24 (null) (null) 2013-11-24 04:02:44 52:54:00:44:7c:d7 ipv4 192.168.150.219/24 iiit-ad885e4aa1 01:52:54:00:44:7c:d7 2013-11-24 04:02:44 52:54:00:44:7c:d7 ipv4 192.168.150.219/24 (null) 01:52:54:00:44:7c:d7 2013-11-24 04:03:36 52:54:00:5d:99:92 ipv4 192.168.150.212/24 iiit-ad885e4aa1 01:52:54:00:5d:99:92 2013-11-24 04:04:41 52:54:00:db:dd:98 ipv4 192.168.150.234/24 (null) (null) 2013-11-24 04:04:48 52:54:00:db:dd:98 ipv6 2001:db8:ca2:2:1::6d/24 (null) 00:04:76:00:cf:ae:b3:0b:fc:cd:0e:22:2e:97:76:65:74:ec
I wonder if we should use '-' instead of '(null)' here - I think it is probably a bit more user friendly.
dnsmasq uses '*' in case of null strings. Its a matter of choice. Since this user-friendly option should be available for all APIs, should I add: /* * Use this instead of NULLSTR to make it more user-friendly. */ # define EMPTYSTR(s) ((s) ? (s) : "*") in src/internal.h ?
No, what dnsmasq uses is irrelevant here. I'm looking at virsh where other commands will use '-' for data that isn't present. eg the ID field $ virsh list --all Id Name State ---------------------------------------------------- - demo shut off - ppcdemo shut off - serial shut off 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 Tue, Nov 26, 2013 at 2:35 AM, Nehal J Wani <nehaljw.kkd1@gmail.com> wrote:
This API returns the leases information stored in the DHCP leases file of dnsmasq for a given virtual network. It contacts the bridge network driver, which parses a custom leases file created by libvirt.
It supports two methods:
1. Return info for all network interfaces connected to a given virtual network 2. Return information for a particular network interface in a given virtual network by providing its MAC Address
v5 Created a helper file to generate custom leases for libvirt. Since dnsmasq doesn't provide the MAC Address in case of DHCPv6, the need for this file was proposed. The design of virNetworkDHCPLeases struct has been updated and the previous use of union has been removed. OOM errors have been taken care of in the remote driver and remote daemon. Discussion on struct design: https://www.redhat.com/archives/libvir-list/2013-October/msg00326.html Discussion on helper file: https://www.redhat.com/archives/libvir-list/2013-October/msg00989.html
v4 * Added support for DHCPv6, updated lease file parsing method Refer: https://www.redhat.com/archives/libvir-list/2013-September/msg01554.html
v3 * Mostly small nits, change in MACRO names, use of virSocketAddrGetIpPrefix to retrieve IP prefix from @dom XML. Refer: https://www.redhat.com/archives/libvir-list/2013-September/msg00832.html
v2 * Since DHCPv6 is supposed to be suported in future, virNetworkGetDHCPLeasesForMAC changed, prefix and virIPAddrType added in virNetworkDHCPLeases struct. Refer: https://www.redhat.com/archives/libvir-list/2013-September/msg00732.html
v1 * Refer: https://www.redhat.com/archives/libvir-list/2013-September/msg00620.html
* The need for these APIs were result of a RFC was proposed on the list. Refer: http://www.redhat.com/archives/libvir-list/2013-July/msg01603.html
Nehal J Wani (4): net-dhcp-leases: Implement the public APIs net-dhcp-leases: Implement the remote protocol net-dhcp-leases: Private implementation inside network driver net-dhcp-leases: Add virsh support
daemon/remote.c | 176 ++++++++++++++++++++++++++++++ include/libvirt/libvirt.h.in | 35 ++++++ src/Makefile.am | 21 ++++ src/driver.h | 13 +++ src/libvirt.c | 197 ++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 7 ++ src/network/bridge_driver.c | 248 +++++++++++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 187 ++++++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 54 +++++++++- src/remote_protocol-structs | 38 +++++++ src/rpc/gendispatch.pl | 1 + src/util/leaseshelper.c | 185 ++++++++++++++++++++++++++++++++ tools/virsh-network.c | 118 ++++++++++++++++++++ tools/virsh.pod | 6 ++ 14 files changed, 1285 insertions(+), 1 deletion(-) create mode 100644 src/util/leaseshelper.c
-- 1.8.1.4
Ping. Once this is accepted, I'll integrate it with the virDomainInterfaceAddresses API. -- Nehal J Wani
participants (2)
-
Daniel P. Berrange
-
Nehal J Wani