On 08/04/14 09:59, Nehal J Wani wrote:
This patch enables the helper program to detect event(s) triggered
when there
is a change in lease length or expiry and client-id. This transfers complete
control of leases database to libvirt and obsoletes use of the lease database
file (<network-name>.leases). That file will not be created, read, or written.
This is achieved by adding the option --leasefile-ro to dnsmasq and passing a
custom env var to leaseshelper, which helps us map events related to leases
with their corresponding network bridges, no matter what the event be.
Also, this requires the addition of a new non-lease entry in our custom lease
database: "server-duid". It is required to identify a DHCPv6 server.
Now that dnsmasq doesn't maintain its own leases database, it relies on our
helper program to tell it about previous leases and server duid. Thus, this
patch makes our leases program honor an extra action: "init", in which it
sends
the known info in a particular format to dnsmasq by printing it to stdout.
---
This is compatible with libvirt 1.2.6 as only additions have been
introduced, and the old leases file (*.status) will still be supported.
v3: * Add server-duid as an entry in the lease object for every ipv6 lease.
* Remove unnecessary variables and double copies.
* Take value from DNSMASQ_OLD_HOSTNAME if hostname is not known.
v2:
http://www.redhat.com/archives/libvir-list/2014-July/msg01109.html
v1:
https://www.redhat.com/archives/libvir-list/2014-July/msg00568.html
src/network/bridge_driver.c | 3 +
src/network/leaseshelper.c | 132 +++++++++++++++++++++++++++++++++++---------
2 files changed, 109 insertions(+), 26 deletions(-)
diff --git a/src/network/leaseshelper.c b/src/network/leaseshelper.c
index c8543a2..e984cbb 100644
--- a/src/network/leaseshelper.c
+++ b/src/network/leaseshelper.c
@@ -260,11 +275,13 @@ main(int argc, char **argv)
goto cleanup;
if (clientid && virJSONValueObjectAppendString(lease_new,
"client-id", clientid) < 0)
goto cleanup;
+ if (server_duid && virJSONValueObjectAppendString(lease_new,
"server-duid", server_duid) < 0)
+ goto cleanup;
if (expirytime && virJSONValueObjectAppendNumberLong(lease_new,
"expiry-time", expirytime) < 0)
goto cleanup;
}
}
- } else {
+ } else if (action != VIR_LEASE_ACTION_INIT) {
fprintf(stderr, _("Unsupported action: %s\n"),
virLeaseActionTypeToString(action));
exit(EXIT_FAILURE);
@@ -294,7 +311,7 @@ main(int argc, char **argv)
i = 0;
while (i < virJSONValueArraySize(leases_array)) {
const char *ip_tmp = NULL;
- long long expirytime_tmp = -1;
+ const char *server_duid_tmp = NULL;
if (!(lease_tmp = virJSONValueArrayGet(leases_array, i))) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -303,14 +320,13 @@ main(int argc, char **argv)
}
if (!(ip_tmp = virJSONValueObjectGetString(lease_tmp,
"ip-address")) ||
- (virJSONValueObjectGetNumberLong(lease_tmp, "expiry-time",
&expirytime_tmp) < 0)) {
+ (virJSONValueObjectGetNumberLong(lease_tmp, "expiry-time",
&expirytime) < 0)) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("failed to parse json"));
goto cleanup;
}
-
/* Check whether lease has expired or not */
- if (expirytime_tmp < currtime) {
+ if (expirytime < currtime) {
i++;
continue;
}
@@ -321,6 +337,30 @@ main(int argc, char **argv)
continue;
}
+ /* Store pointers to ipv4 and ipv6 leases */
+ if (strchr(ip_tmp, ':')) {
+ /* This is an ipv6 lease */
Is there a need to separate them? Can't we just flush them out from the
json array in the order they are stored?
+
ignore_value(VIR_APPEND_ELEMENT_COPY(leases_ipv6, count_ipv6, lease_tmp));
It would save us the need to copy the leases separately even if dnsmasq
isn't initializing.
Also you should probably check the return value, otherwise you will drop
leases silently on OOM.
+ if ((server_duid_tmp
+ = virJSONValueObjectGetString(lease_tmp,
"server-duid"))) {
+ if (!server_duid) {
+ /* Control reaches here when the 'action' is not for
an
+ * ipv6 lease or, for some weird reason the env var
+ * DNSMASQ_SERVER_DUID wasn't set*/
+ server_duid = server_duid_tmp;
+ }
+ } else {
+ /* Inject server-duid into those ipv6 leases which
+ * didn't have it previously, for example, those
+ * created by leaseshelper from libvirt 1.2.6 */
+ if (virJSONValueObjectAppendString(lease_tmp,
"server-duid", server_duid) < 0)
+ goto cleanup;
This part is required all the time as our format wasn't complete.
+ }
+ } else {
+ /* This is an ipv4 lease */
+ ignore_value(VIR_APPEND_ELEMENT_COPY(leases_ipv4, count_ipv4,
lease_tmp));
Again, do we need to sort them explicitly?
+ }
+
/* Move old lease to new array */
if (virJSONValueArrayAppend(leases_array_new, lease_tmp) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -333,31 +373,71 @@ main(int argc, char **argv)
}
}
- if (add) {
+ switch ((enum virLeaseActionFlags) action) {
+ case VIR_LEASE_ACTION_INIT:
+ /* Man page of dnsmasq says: the script (helper program, in our case)
+ * should write the saved state of the lease database, in dnsmasq
+ * leasefile format, to stdout and exit with zero exit code, when
+ * called with argument init. Format:
+ * $expirytime $mac $ip $hostname $clientid # For all ipv4 leases
+ * duid $server-duid # If DHCPv6 is present
+ * $expirytime $iaid $ip $hostname $clientduid # For all ipv6 leases */
Here you could just go through the json array and format each lease.
+ for (i = 0; i < count_ipv4; i++) {
+ lease_tmp = leases_ipv4[i];
+ virJSONValueObjectGetNumberLong(lease_tmp, "expiry-time",
&expirytime);
+ printf("%lld %s %s %s %s\n",
+ expirytime,
+ virJSONValueObjectGetString(lease_tmp, "mac-address"),
+ virJSONValueObjectGetString(lease_tmp, "ip-address"),
+ EMPTY_STR(virJSONValueObjectGetString(lease_tmp,
"hostname")),
+ EMPTY_STR(virJSONValueObjectGetString(lease_tmp,
"client-id")));
+ }
+ if (server_duid) {
+ printf("duid %s\n", server_duid);
+ for (i = 0; i < count_ipv6; i++) {
+ lease_tmp = leases_ipv6[i];
+ virJSONValueObjectGetNumberLong(lease_tmp, "expiry-time",
&expirytime);
+ printf("%lld %s %s %s %s\n",
+ expirytime,
+ virJSONValueObjectGetString(lease_tmp, "iaid"),
+ virJSONValueObjectGetString(lease_tmp, "ip-address"),
+ EMPTY_STR(virJSONValueObjectGetString(lease_tmp,
"hostname")),
+ EMPTY_STR(virJSONValueObjectGetString(lease_tmp,
"client-id")));
+ }
+ }
+ break;
+
+ case VIR_LEASE_ACTION_OLD:
+ case VIR_LEASE_ACTION_ADD:
if (virJSONValueArrayAppend(leases_array_new, lease_new) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("failed to create json"));
goto cleanup;
}
lease_new = NULL;
- }
- if (!(leases_str = virJSONValueToString(leases_array_new, true))) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("empty json array"));
- goto cleanup;
- }
+ default:
+ if (!(leases_str = virJSONValueToString(leases_array_new, true))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("empty json array"));
+ goto cleanup;
+ }
- /* Write to file */
- if (virFileRewrite(custom_lease_file, 0644,
- customLeaseRewriteFile, &leases_str) < 0)
- goto cleanup;
+ /* Write to file */
+ if (virFileRewrite(custom_lease_file, 0644,
+ customLeaseRewriteFile, &leases_str) < 0)
+ goto cleanup;
+ }
rv = EXIT_SUCCESS;
cleanup:
if (pid_file_fd != -1)
virPidFileReleasePath(pid_file, pid_file_fd);
+ for (i = 0; i < count_ipv4; i++)
+ VIR_FREE(leases_ipv4);
+ for (i = 0; i < count_ipv6; i++)
+ VIR_FREE(leases_ipv6);
VIR_FREE(pid_file);
VIR_FREE(exptime);
Otherwise looks good.
Peter