5:03 a.m.
On 13/09/13 05:53, Nehal J Wani wrote:
By querying the driver for the path of the 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 | 193 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 193 insertions(+)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 3a8be90..2cdea56 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -109,6 +109,36 @@ static int networkPlugBandwidth(virNetworkObjPtr net,
virDomainNetDefPtr iface);
static int networkUnplugBandwidth(virNetworkObjPtr net,
virDomainNetDefPtr iface);
+/**
+ * VIR_NETWORK_DHCPLEASE_LENGTH_MAX:
+ *
+ * Macro providing the maximum length of an entry in the leases file
+ * Refer: http://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2013q3/007402.html
+ */
+#define VIR_NETWORK_DHCPLEASE_LENGTH_MAX 2048
+
+/**
+ * VIR_NETWORK_DHCPLEASE_PARAMS:
+ *
+ * Macro providing the maximum number of parameters in an entry in
+ * the leases file
+ */
+#define VIR_NETWORK_DHCPLEASE_FIELDS 5
s/DHCPLEASE/DHCP_LEASE/,
+
+static int networkGetDHCPLeases(virNetworkPtr network,
+ virNetworkDHCPLeasesPtr **leases,
+ unsigned int flags);
+
+static int networkGetDHCPLeasesForMAC(virNetworkPtr network,
+ const char *mac,
+ virNetworkDHCPLeasesPtr **leases,
+ unsigned int flags);
+
+static int networkGetDHCPLeasesHelper(virNetworkPtr network,
+ virNetworkObjPtr obj,
+ const char *mac,
+ virNetworkDHCPLeasesPtr **leases,
+ unsigned int flags);
Not a big deal, but these function declarations can be removed.
static virNetworkDriverStatePtr driverState = NULL;
@@ -2980,6 +3010,167 @@ cleanup:
return ret;
}
+static int
+networkGetDHCPLeases(virNetworkPtr network,
+ virNetworkDHCPLeasesPtr **leases,
+ unsigned int flags)
+{
+ int rv = -1;
+ virNetworkObjPtr obj;
+
+ virCheckFlags(0, -1);
+
+ obj = networkObjFromNetwork(network);
+
+ if (virNetworkGetDHCPLeasesEnsureACL(network->conn, obj->def) < 0)
+ goto cleanup;
+
+ rv = networkGetDHCPLeasesHelper(network, obj, NULL, leases, flags);
+
+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);
+
+ obj = networkObjFromNetwork(network);
+
+ if (virNetworkGetDHCPLeasesForMACEnsureACL(network->conn, obj->def) < 0)
+ goto cleanup;
+
+ rv = networkGetDHCPLeasesHelper(network, obj, mac, leases, flags);
+
+cleanup:
+ if (obj)
+ virNetworkObjUnlock(obj);
+ return rv;
+}
+
+/* This function parese the leases file generated by dnsmasq.
s/generated by/of/,
+ * Example content:
+ * ::::::::::::::
+ * /var/lib/libvirt/dnsmasq/TestNetwork1.leases
+ * ::::::::::::::
Not good comment style, especially when the real comment has ":" character,
I'd like write it as:
* An example of leases file content:
*
* 1379024255 52:54:00:20:70:3d 192.168.105.240 * *
* 1379023351 52:54:00:b1:70:19 192.168.105.201 * *
+ * 1379024255 52:54:00:20:70:3d 192.168.105.240 * *
+ * 1379023351 52:54:00:b1:70:19 192.168.105.201 * *
+ */
+static int
+networkGetDHCPLeasesHelper(virNetworkPtr network,
+ virNetworkObjPtr obj,
+ const char *mac,
+ virNetworkDHCPLeasesPtr **leases,
+ unsigned int flags)
+{
+ int rv = -1;
+ size_t i = 0;
+ char *leasefile;
+ FILE *fp = NULL;
+ size_t nleases = 0;
+ char **leaseparams;
To be consistent with the macro name, this should be lease_fields
or leasefields.
+ virNetworkDHCPLeasesPtr *leases_ret = NULL;
+ char dhcpentry[VIR_NETWORK_DHCPLEASE_LENGTH_MAX];
s/DHCPLEASE/DHCP_LEASE/,
+
+ virCheckFlags(0, -1);
+
Redundant flags checking. You already did it in both networkGetDHCPLeases
and networkGetDHCPLeasesForMAC
+ if (!network) {
+ virReportError(VIR_ERR_NO_NETWORK, "%s",
+ _("no network with matching name"));
+ return -1;
+ }
+
+ /* Retrive leases file location */
+ leasefile = networkDnsmasqLeaseFileNameDefault(network->name);
+ if (!(fp = fopen(leasefile, "r"))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unable to open leases file: %s"), leasefile);
+ goto error;
s/error/cleanup/, since you don't have to free the lease_ret if fails
at this point.
+ }
+
+ while (fgets(dhcpentry, sizeof(dhcpentry), fp) != NULL) {
+ virNetworkDHCPLeasesPtr lease = NULL;
+
+ /* Remove newline */
+ dhcpentry[strlen(dhcpentry) - 1] = '\0';
+
+ /* split the lease line */
+ leaseparams = virStringSplit(dhcpentry, " ",
VIR_NETWORK_DHCPLEASE_FIELDS);
+
+ if (virStringListLength(leaseparams) != VIR_NETWORK_DHCPLEASE_FIELDS) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Number of lease params aren't equal to:
%d"),
+ VIR_NETWORK_DHCPLEASE_FIELDS);
+ goto error;
+ }
+
+ if (mac && STRNEQ(mac, leaseparams[1]))
+ continue;
+
+ /* We don't know the total number of leases yet */
This comment can be removed, it doesn't tell what the code does clearly,
instead, it's confused. The code explain itself clearly.
+ if (VIR_EXPAND_N(leases_ret, nleases, 1) < 0)
+ goto error;
+
+ if (VIR_ALLOC(leases_ret[nleases - 1]) < 0)
+ goto error;
+
+ lease = leases_ret[nleases - 1];
+
+ /* Convert expirytime here */
+ if (virStrToLong_ll(leaseparams[0], NULL, 10, &(lease->expirytime)) <
0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unable to convert lease expiry time to integer:
%s"),
+ leaseparams[0]);
+ goto error;
+ }
+
+ /* Hardcoded, as dnsmasq uses ipv4 */
+ lease->type = VIR_IP_ADDR_TYPE_IPV4;
+ lease->prefix =
virSocketAddrGetNumNetmaskBits(&obj->def->ips->netmask);
Hm, virSocketAddrGetNumNetmaskBits is used as a general purpose function,
but its name is too specific. But it doesn't relate with this patch.
+
+ if ((VIR_STRDUP(lease->mac, leaseparams[1]) < 0) ||
+ (VIR_STRDUP(lease->ipaddr, leaseparams[2]) < 0) ||
+ (VIR_STRDUP(lease->hostname, leaseparams[3]) < 0) ||
+ (VIR_STRDUP(lease->clientid, leaseparams[4]) < 0))
+ goto error;
+ }
+
+ if (mac && !leases_ret) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("no lease with matching MAC address: %s"), mac);
+ goto error;
+ }
+
+ if (leases_ret) {
+ /* trim the array to the final size */
Confused comment, you are not trimming anything. Consider about:
/* NULL terminated array */
+ ignore_value(VIR_REALLOC_N(leases_ret, nleases + 1));
+ *leases = leases_ret;
+ leases_ret = NULL;
+ rv = nleases;
+ }
+
+cleanup:
+ VIR_FORCE_FCLOSE(fp);
+ VIR_FREE(leasefile);
+ return rv;
+
+error:
+ if (leases_ret) {
+ for (i = 0; i < nleases; i++)
+ virNetworkDHCPLeaseFree(leases_ret[i]);
+ }
+ VIR_FREE(leases_ret);
+ goto cleanup;
+}
static virNetworkDriver networkDriver = {
"Network",
@@ -3004,6 +3195,8 @@ static virNetworkDriver networkDriver = {
.networkSetAutostart = networkSetAutostart, /* 0.2.1 */
.networkIsActive = networkIsActive, /* 0.7.3 */
.networkIsPersistent = networkIsPersistent, /* 0.7.3 */
+ .networkGetDHCPLeases = networkGetDHCPLeases, /* 1.1.3 */
+ .networkGetDHCPLeasesForMAC = networkGetDHCPLeasesForMAC, /* 1.1.3 */
};
static virStateDriver networkStateDriver = {