We either use the value from the environment variable, or learn it from the existing lease file. In the second case, the pointer would be pointing into the JSON object of the first lease with a DUID, owned by leases_array, then leases_array_new. Always allocate the string instead, making obvious who should free the string. --- src/network/leaseshelper.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/network/leaseshelper.c b/src/network/leaseshelper.c index 6930310..3d2dace 100644 --- a/src/network/leaseshelper.c +++ b/src/network/leaseshelper.c @@ -122,7 +122,7 @@ main(int argc, char **argv) const char *interface = virGetEnvAllowSUID("DNSMASQ_INTERFACE"); const char *exptime_tmp = virGetEnvAllowSUID("DNSMASQ_LEASE_EXPIRES"); const char *hostname = virGetEnvAllowSUID("DNSMASQ_SUPPLIED_HOSTNAME"); - const char *server_duid = virGetEnvAllowSUID("DNSMASQ_SERVER_DUID"); + char *server_duid = NULL; long long currtime = 0; long long expirytime = 0; size_t i = 0; @@ -210,6 +210,9 @@ main(int argc, char **argv) clientid = argv[2]; } + if (VIR_STRDUP(server_duid, virGetEnvAllowSUID("DNSMASQ_SERVER_DUID")) < 0) + goto cleanup; + if (virAsprintf(&custom_lease_file, LOCALSTATEDIR "/lib/libvirt/dnsmasq/%s.status", interface) < 0) @@ -351,11 +354,11 @@ main(int argc, char **argv) /* This is an ipv6 lease */ if ((server_duid_tmp = virJSONValueObjectGetString(lease_tmp, "server-duid"))) { - if (!server_duid) { + if (!server_duid && VIR_STRDUP(server_duid, server_duid_tmp) < 0) { /* 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; + goto cleanup; } } else { /* Inject server-duid into those ipv6 leases which @@ -472,6 +475,7 @@ main(int argc, char **argv) VIR_FREE(pid_file); VIR_FREE(exptime); + VIR_FREE(server_duid); VIR_FREE(lease_entries); VIR_FREE(custom_lease_file); virJSONValueFree(lease_new); -- 2.4.6

Introduce virLeaseReadCustomLeaseFile which will populate the new leases array with all the leases, except for expired ones and the ones matching 'ip_to_delete'. This removes five variables from main(). --- src/network/leaseshelper.c | 194 +++++++++++++++++++++++++-------------------- 1 file changed, 108 insertions(+), 86 deletions(-) diff --git a/src/network/leaseshelper.c b/src/network/leaseshelper.c index 3d2dace..8bf1c01 100644 --- a/src/network/leaseshelper.c +++ b/src/network/leaseshelper.c @@ -105,35 +105,135 @@ VIR_ENUM_DECL(virLeaseAction); VIR_ENUM_IMPL(virLeaseAction, VIR_LEASE_ACTION_LAST, "add", "old", "del", "init"); +static int +virLeaseReadCustomLeaseFile(virJSONValuePtr leases_array_new, + const char *custom_lease_file, + const char *ip_to_delete, + char **server_duid) +{ + char *lease_entries = NULL; + virJSONValuePtr leases_array = NULL; + long long currtime = 0; + long long expirytime; + int custom_lease_file_len = 0; + virJSONValuePtr lease_tmp = NULL; + const char *ip_tmp = NULL; + const char *server_duid_tmp = NULL; + size_t i; + int ret = -1; + + currtime = (long long) time(NULL); + + /* Read entire contents */ + if ((custom_lease_file_len = virFileReadAll(custom_lease_file, + VIR_NETWORK_DHCP_LEASE_FILE_SIZE_MAX, + &lease_entries)) < 0) { + goto cleanup; + } + + /* Check for previous leases */ + if (custom_lease_file_len) { + if (!(leases_array = virJSONValueFromString(lease_entries))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("invalid json in file: %s, rewriting it"), + custom_lease_file); + } else { + if (!virJSONValueIsArray(leases_array)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("couldn't fetch array of leases")); + goto cleanup; + } + + i = 0; + while (i < virJSONValueArraySize(leases_array)) { + + if (!(lease_tmp = virJSONValueArrayGet(leases_array, i))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to parse json")); + goto cleanup; + } + + if (!(ip_tmp = virJSONValueObjectGetString(lease_tmp, "ip-address")) || + (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 < currtime) { + i++; + continue; + } + + /* Check whether lease has to be included or not */ + if (ip_to_delete && STREQ(ip_tmp, ip_to_delete)) { + i++; + continue; + } + + if (strchr(ip_tmp, ':')) { + /* This is an ipv6 lease */ + if ((server_duid_tmp + = virJSONValueObjectGetString(lease_tmp, "server-duid"))) { + if (!*server_duid && VIR_STRDUP(*server_duid, server_duid_tmp) < 0) { + /* 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*/ + goto cleanup; + } + } 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; + } + } + + /* Move old lease to new array */ + if (virJSONValueArrayAppend(leases_array_new, lease_tmp) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to create json")); + goto cleanup; + } + + ignore_value(virJSONValueArraySteal(leases_array, i)); + } + } + } + + ret = 0; + + cleanup: + virJSONValueFree(leases_array); + VIR_FREE(lease_entries); + return ret; +} + int main(int argc, char **argv) { char *exptime = NULL; char *pid_file = NULL; - char *lease_entries = NULL; char *custom_lease_file = NULL; const char *ip = NULL; const char *mac = NULL; const char *ip_tmp = NULL; const char *leases_str = NULL; - const char *server_duid_tmp = NULL; const char *iaid = virGetEnvAllowSUID("DNSMASQ_IAID"); const char *clientid = virGetEnvAllowSUID("DNSMASQ_CLIENT_ID"); const char *interface = virGetEnvAllowSUID("DNSMASQ_INTERFACE"); const char *exptime_tmp = virGetEnvAllowSUID("DNSMASQ_LEASE_EXPIRES"); const char *hostname = virGetEnvAllowSUID("DNSMASQ_SUPPLIED_HOSTNAME"); char *server_duid = NULL; - long long currtime = 0; long long expirytime = 0; size_t i = 0; int action = -1; int pid_file_fd = -1; int rv = EXIT_FAILURE; - int custom_lease_file_len = 0; bool delete = false; virJSONValuePtr lease_new = NULL; virJSONValuePtr lease_tmp = NULL; - virJSONValuePtr leases_array = NULL; virJSONValuePtr leases_array_new = NULL; virSetErrorFunc(NULL, NULL); @@ -230,13 +330,6 @@ main(int argc, char **argv) if (virFileTouch(custom_lease_file, 0644) < 0) goto cleanup; - /* Read entire contents */ - if ((custom_lease_file_len = virFileReadAll(custom_lease_file, - VIR_NETWORK_DHCP_LEASE_FILE_SIZE_MAX, - &lease_entries)) < 0) { - goto cleanup; - } - switch ((enum virLeaseActionFlags) action) { case VIR_LEASE_ACTION_ADD: case VIR_LEASE_ACTION_OLD: @@ -308,78 +401,9 @@ main(int argc, char **argv) goto cleanup; } - currtime = (long long) time(NULL); - - /* Check for previous leases */ - if (custom_lease_file_len) { - if (!(leases_array = virJSONValueFromString(lease_entries))) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("invalid json in file: %s, rewriting it"), - custom_lease_file); - } else { - if (!virJSONValueIsArray(leases_array)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("couldn't fetch array of leases")); - goto cleanup; - } - - i = 0; - while (i < virJSONValueArraySize(leases_array)) { - - if (!(lease_tmp = virJSONValueArrayGet(leases_array, i))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to parse json")); - goto cleanup; - } - - if (!(ip_tmp = virJSONValueObjectGetString(lease_tmp, "ip-address")) || - (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 < currtime) { - i++; - continue; - } - - /* Check whether lease has to be included or not */ - if (delete && STREQ(ip_tmp, ip)) { - i++; - continue; - } - - if (strchr(ip_tmp, ':')) { - /* This is an ipv6 lease */ - if ((server_duid_tmp - = virJSONValueObjectGetString(lease_tmp, "server-duid"))) { - if (!server_duid && VIR_STRDUP(server_duid, server_duid_tmp) < 0) { - /* 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*/ - goto cleanup; - } - } 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; - } - } - - /* Move old lease to new array */ - if (virJSONValueArrayAppend(leases_array_new, lease_tmp) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to create json")); - goto cleanup; - } - - ignore_value(virJSONValueArraySteal(leases_array, i)); - } - } - } + if (virLeaseReadCustomLeaseFile(leases_array_new, custom_lease_file, + delete ? ip : NULL, &server_duid) < 0) + goto cleanup; switch ((enum virLeaseActionFlags) action) { case VIR_LEASE_ACTION_INIT: @@ -476,10 +500,8 @@ main(int argc, char **argv) VIR_FREE(pid_file); VIR_FREE(exptime); VIR_FREE(server_duid); - VIR_FREE(lease_entries); VIR_FREE(custom_lease_file); virJSONValueFree(lease_new); - virJSONValueFree(leases_array); virJSONValueFree(leases_array_new); return rv; -- 2.4.6

Introduce a function for printing the leases on the 'init' operation. --- src/network/leaseshelper.c | 130 +++++++++++++++++++++++++-------------------- 1 file changed, 73 insertions(+), 57 deletions(-) diff --git a/src/network/leaseshelper.c b/src/network/leaseshelper.c index 8bf1c01..94686ee 100644 --- a/src/network/leaseshelper.c +++ b/src/network/leaseshelper.c @@ -210,6 +210,77 @@ virLeaseReadCustomLeaseFile(virJSONValuePtr leases_array_new, return ret; } +static int +virLeasePrintLeases(virJSONValuePtr leases_array_new, + const char *server_duid) +{ + virJSONValuePtr lease_tmp = NULL; + const char *ip_tmp = NULL; + long long expirytime = 0; + int ret = -1; + size_t i; + + /* 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 */ + + /* Traversing the ipv4 leases */ + for (i = 0; i < virJSONValueArraySize(leases_array_new); i++) { + lease_tmp = virJSONValueArrayGet(leases_array_new, i); + if (!(ip_tmp = virJSONValueObjectGetString(lease_tmp, "ip-address"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to parse json")); + goto cleanup; + } + if (!strchr(ip_tmp, ':')) { + if (virJSONValueObjectGetNumberLong(lease_tmp, "expiry-time", + &expirytime) < 0) + continue; + + 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"))); + } + } + + /* Traversing the ipv6 leases */ + if (server_duid) { + printf("duid %s\n", server_duid); + for (i = 0; i < virJSONValueArraySize(leases_array_new); i++) { + lease_tmp = virJSONValueArrayGet(leases_array_new, i); + if (!(ip_tmp = virJSONValueObjectGetString(lease_tmp, "ip-address"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to parse json")); + goto cleanup; + } + if (strchr(ip_tmp, ':')) { + if (virJSONValueObjectGetNumberLong(lease_tmp, "expiry-time", + &expirytime) < 0) + continue; + + 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"))); + } + } + } + + ret = 0; + + cleanup: + return ret; +} + int main(int argc, char **argv) { @@ -218,7 +289,6 @@ main(int argc, char **argv) char *custom_lease_file = NULL; const char *ip = NULL; const char *mac = NULL; - const char *ip_tmp = NULL; const char *leases_str = NULL; const char *iaid = virGetEnvAllowSUID("DNSMASQ_IAID"); const char *clientid = virGetEnvAllowSUID("DNSMASQ_CLIENT_ID"); @@ -227,13 +297,11 @@ main(int argc, char **argv) const char *hostname = virGetEnvAllowSUID("DNSMASQ_SUPPLIED_HOSTNAME"); char *server_duid = NULL; long long expirytime = 0; - size_t i = 0; int action = -1; int pid_file_fd = -1; int rv = EXIT_FAILURE; bool delete = false; virJSONValuePtr lease_new = NULL; - virJSONValuePtr lease_tmp = NULL; virJSONValuePtr leases_array_new = NULL; virSetErrorFunc(NULL, NULL); @@ -407,60 +475,8 @@ main(int argc, char **argv) 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 */ - - /* Traversing the ipv4 leases */ - for (i = 0; i < virJSONValueArraySize(leases_array_new); i++) { - lease_tmp = virJSONValueArrayGet(leases_array_new, i); - if (!(ip_tmp = virJSONValueObjectGetString(lease_tmp, "ip-address"))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to parse json")); - goto cleanup; - } - if (!strchr(ip_tmp, ':')) { - if (virJSONValueObjectGetNumberLong(lease_tmp, "expiry-time", - &expirytime) < 0) - continue; - - 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"))); - } - } - - /* Traversing the ipv6 leases */ - if (server_duid) { - printf("duid %s\n", server_duid); - for (i = 0; i < virJSONValueArraySize(leases_array_new); i++) { - lease_tmp = virJSONValueArrayGet(leases_array_new, i); - if (!(ip_tmp = virJSONValueObjectGetString(lease_tmp, "ip-address"))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to parse json")); - goto cleanup; - } - if (strchr(ip_tmp, ':')) { - if (virJSONValueObjectGetNumberLong(lease_tmp, "expiry-time", - &expirytime) < 0) - continue; - - 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"))); - } - } - } + if (virLeasePrintLeases(leases_array_new, server_duid) < 0) + goto cleanup; break; -- 2.4.6

We do not care if the mac was specified in the delete section, we are going to delete the record anyway. Also move the comment about adding ipv6 leases to the ADD case. --- src/network/leaseshelper.c | 34 +++++++++++++++------------------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/src/network/leaseshelper.c b/src/network/leaseshelper.c index 94686ee..eb1c0c7 100644 --- a/src/network/leaseshelper.c +++ b/src/network/leaseshelper.c @@ -401,9 +401,21 @@ main(int argc, char **argv) switch ((enum virLeaseActionFlags) action) { case VIR_LEASE_ACTION_ADD: case VIR_LEASE_ACTION_OLD: + /* Custom ipv6 leases *will not* be created if the env-var DNSMASQ_MAC + * is not set. In the special case, when the $(interface).status file + * is not already present and dnsmasq is (re)started, the corresponding + * ipv6 custom lease will be created only when the guest sends the + * 'old' action for its existing ipv6 interfaces. + * + * According to rfc3315, the combination of DUID and IAID can be used + * to uniquely identify each ipv6 guest interface. So, in future, if + * we introduce virNetworkGetDHCPLeaseBy(IAID|DUID|IAID+DUID) for ipv6 + * interfaces, then, the following if condition won't be required, as + * the new lease will be created irrespective of whether the MACID is + * known or not. + */ if (!mac) break; - delete = true; /* Create new lease */ if (!(lease_new = virJSONValueNewObject())) { @@ -435,27 +447,11 @@ main(int argc, char **argv) if (expirytime && virJSONValueObjectAppendNumberLong(lease_new, "expiry-time", expirytime) < 0) goto cleanup; - break; + /* fallthrough */ case VIR_LEASE_ACTION_DEL: + /* Delete the corresponding lease, if it already exists */ delete = true; - /* Custom ipv6 leases *will not* be created if the env-var DNSMASQ_MAC - * is not set. In the special case, when the $(interface).status file - * is not already present and dnsmasq is (re)started, the corresponding - * ipv6 custom lease will be created only when the guest sends the - * 'old' action for its existing ipv6 interfaces. - * - * According to rfc3315, the combination of DUID and IAID can be used - * to uniquely identify each ipv6 guest interface. So, in future, if - * we introduce virNetworkGetDHCPLeaseBy(IAID|DUID|IAID+DUID) for ipv6 - * interfaces, then, the following if condition won't be required, as - * the new lease will be created irrespective of whether the MACID is - * known or not. - */ - if (mac) { - /* Delete the corresponding lease, if it already exists */ - delete = true; - } break; case VIR_LEASE_ACTION_INIT: -- 2.4.6

For the actions ADD and OLD, split out creating the new lease object, along with processing all the program args and environment variables that are only needed in this case. --- src/network/leaseshelper.c | 156 ++++++++++++++++++++++++++------------------- 1 file changed, 91 insertions(+), 65 deletions(-) diff --git a/src/network/leaseshelper.c b/src/network/leaseshelper.c index eb1c0c7..547e1d8 100644 --- a/src/network/leaseshelper.c +++ b/src/network/leaseshelper.c @@ -281,22 +281,102 @@ virLeasePrintLeases(virJSONValuePtr leases_array_new, return ret; } +static int +virLeaseNewFromArgv(virJSONValuePtr *lease_ret, + int argc, + char **argv, + const char *ip, + const char *server_duid) +{ + virJSONValuePtr lease_new = NULL; + const char *clientid = virGetEnvAllowSUID("DNSMASQ_CLIENT_ID"); + const char *exptime_tmp = virGetEnvAllowSUID("DNSMASQ_LEASE_EXPIRES"); + const char *hostname = virGetEnvAllowSUID("DNSMASQ_SUPPLIED_HOSTNAME"); + const char *iaid = virGetEnvAllowSUID("DNSMASQ_IAID"); + const char *mac = NULL; + long long expirytime = 0; + char *exptime = NULL; + int ret = -1; + + /* In case hostname is known, it is the 5th argument */ + if (argc == 5) + hostname = argv[4]; + + /* In case hostname is still unknown, use the last known one */ + if (!hostname) + hostname = virGetEnvAllowSUID("DNSMASQ_OLD_HOSTNAME"); + + /* Check if it is an IPv6 lease */ + if (iaid) { + mac = virGetEnvAllowSUID("DNSMASQ_MAC"); + clientid = argv[2]; + } else { + mac = argv[2]; + } + + if (!mac) { + ret = 0; + goto cleanup; + } + + if (exptime_tmp) { + if (VIR_STRDUP(exptime, exptime_tmp) < 0) + goto cleanup; + + /* Removed extraneous trailing space in DNSMASQ_LEASE_EXPIRES + * (dnsmasq < 2.52) */ + if (exptime[strlen(exptime) - 1] == ' ') + exptime[strlen(exptime) - 1] = '\0'; + } + + if (!exptime || + virStrToLong_ll(exptime, NULL, 10, &expirytime) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to convert lease expiry time to long long: %s"), + NULLSTR(exptime)); + goto cleanup; + } + + /* Create new lease */ + if (!(lease_new = virJSONValueNewObject())) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to create json")); + goto cleanup; + } + + if (iaid && virJSONValueObjectAppendString(lease_new, "iaid", iaid) < 0) + goto cleanup; + if (ip && virJSONValueObjectAppendString(lease_new, "ip-address", ip) < 0) + goto cleanup; + if (mac && virJSONValueObjectAppendString(lease_new, "mac-address", mac) < 0) + goto cleanup; + if (hostname && virJSONValueObjectAppendString(lease_new, "hostname", hostname) < 0) + 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; + + ret = 0; + *lease_ret = lease_new; + lease_new = NULL; + cleanup: + VIR_FREE(exptime); + virJSONValueFree(lease_new); + return ret; +} + int main(int argc, char **argv) { - char *exptime = NULL; char *pid_file = NULL; char *custom_lease_file = NULL; const char *ip = NULL; - const char *mac = NULL; const char *leases_str = NULL; - const char *iaid = virGetEnvAllowSUID("DNSMASQ_IAID"); - const char *clientid = virGetEnvAllowSUID("DNSMASQ_CLIENT_ID"); const char *interface = virGetEnvAllowSUID("DNSMASQ_INTERFACE"); - const char *exptime_tmp = virGetEnvAllowSUID("DNSMASQ_LEASE_EXPIRES"); - const char *hostname = virGetEnvAllowSUID("DNSMASQ_SUPPLIED_HOSTNAME"); char *server_duid = NULL; - long long expirytime = 0; int action = -1; int pid_file_fd = -1; int rv = EXIT_FAILURE; @@ -347,37 +427,12 @@ main(int argc, char **argv) goto cleanup; ip = argv[3]; - mac = argv[2]; if ((action = virLeaseActionTypeFromString(argv[1])) < 0) { fprintf(stderr, _("Unsupported action: %s\n"), argv[1]); exit(EXIT_FAILURE); } - /* In case hostname is known, it is the 5th argument */ - if (argc == 5) - hostname = argv[4]; - - /* In case hostname is still unknown, use the last known one */ - if (!hostname) - hostname = virGetEnvAllowSUID("DNSMASQ_OLD_HOSTNAME"); - - if (exptime_tmp) { - if (VIR_STRDUP(exptime, exptime_tmp) < 0) - goto cleanup; - - /* Removed extraneous trailing space in DNSMASQ_LEASE_EXPIRES - * (dnsmasq < 2.52) */ - if (exptime[strlen(exptime) - 1] == ' ') - exptime[strlen(exptime) - 1] = '\0'; - } - - /* Check if it is an IPv6 lease */ - if (iaid) { - mac = virGetEnvAllowSUID("DNSMASQ_MAC"); - clientid = argv[2]; - } - if (VIR_STRDUP(server_duid, virGetEnvAllowSUID("DNSMASQ_SERVER_DUID")) < 0) goto cleanup; @@ -401,6 +456,9 @@ main(int argc, char **argv) switch ((enum virLeaseActionFlags) action) { case VIR_LEASE_ACTION_ADD: case VIR_LEASE_ACTION_OLD: + /* Create new lease */ + if (virLeaseNewFromArgv(&lease_new, argc, argv, ip, server_duid) < 0) + goto cleanup; /* Custom ipv6 leases *will not* be created if the env-var DNSMASQ_MAC * is not set. In the special case, when the $(interface).status file * is not already present and dnsmasq is (re)started, the corresponding @@ -414,40 +472,9 @@ main(int argc, char **argv) * the new lease will be created irrespective of whether the MACID is * known or not. */ - if (!mac) + if (!lease_new) break; - /* Create new lease */ - if (!(lease_new = virJSONValueNewObject())) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to create json")); - goto cleanup; - } - - if (!exptime || - virStrToLong_ll(exptime, NULL, 10, &expirytime) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to convert lease expiry time to long long: %s"), - NULLSTR(exptime)); - goto cleanup; - } - - if (iaid && virJSONValueObjectAppendString(lease_new, "iaid", iaid) < 0) - goto cleanup; - if (ip && virJSONValueObjectAppendString(lease_new, "ip-address", ip) < 0) - goto cleanup; - if (mac && virJSONValueObjectAppendString(lease_new, "mac-address", mac) < 0) - goto cleanup; - if (hostname && virJSONValueObjectAppendString(lease_new, "hostname", hostname) < 0) - 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; - - /* fallthrough */ case VIR_LEASE_ACTION_DEL: /* Delete the corresponding lease, if it already exists */ @@ -510,7 +537,6 @@ main(int argc, char **argv) virPidFileReleasePath(pid_file, pid_file_fd); VIR_FREE(pid_file); - VIR_FREE(exptime); VIR_FREE(server_duid); VIR_FREE(custom_lease_file); virJSONValueFree(lease_new); -- 2.4.6

Instead of nested ifs, jump out early. Mostly whitespace changes. --- src/network/leaseshelper.c | 120 +++++++++++++++++++++++---------------------- 1 file changed, 62 insertions(+), 58 deletions(-) diff --git a/src/network/leaseshelper.c b/src/network/leaseshelper.c index 547e1d8..57e66e9 100644 --- a/src/network/leaseshelper.c +++ b/src/network/leaseshelper.c @@ -132,74 +132,78 @@ virLeaseReadCustomLeaseFile(virJSONValuePtr leases_array_new, } /* Check for previous leases */ - if (custom_lease_file_len) { - if (!(leases_array = virJSONValueFromString(lease_entries))) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("invalid json in file: %s, rewriting it"), - custom_lease_file); - } else { - if (!virJSONValueIsArray(leases_array)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("couldn't fetch array of leases")); - goto cleanup; - } + if (custom_lease_file_len == 0) { + ret = 0; + goto cleanup; + } - i = 0; - while (i < virJSONValueArraySize(leases_array)) { + if (!(leases_array = virJSONValueFromString(lease_entries))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("invalid json in file: %s, rewriting it"), + custom_lease_file); + ret = 0; + goto cleanup; + } - if (!(lease_tmp = virJSONValueArrayGet(leases_array, i))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to parse json")); - goto cleanup; - } + if (!virJSONValueIsArray(leases_array)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("couldn't fetch array of leases")); + goto cleanup; + } - if (!(ip_tmp = virJSONValueObjectGetString(lease_tmp, "ip-address")) || - (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 < currtime) { - i++; - continue; - } + i = 0; + while (i < virJSONValueArraySize(leases_array)) { + if (!(lease_tmp = virJSONValueArrayGet(leases_array, i))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to parse json")); + goto cleanup; + } - /* Check whether lease has to be included or not */ - if (ip_to_delete && STREQ(ip_tmp, ip_to_delete)) { - i++; - continue; - } + if (!(ip_tmp = virJSONValueObjectGetString(lease_tmp, "ip-address")) || + (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 < currtime) { + i++; + continue; + } - if (strchr(ip_tmp, ':')) { - /* This is an ipv6 lease */ - if ((server_duid_tmp - = virJSONValueObjectGetString(lease_tmp, "server-duid"))) { - if (!*server_duid && VIR_STRDUP(*server_duid, server_duid_tmp) < 0) { - /* 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*/ - goto cleanup; - } - } 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; - } - } + /* Check whether lease has to be included or not */ + if (ip_to_delete && STREQ(ip_tmp, ip_to_delete)) { + i++; + continue; + } - /* Move old lease to new array */ - if (virJSONValueArrayAppend(leases_array_new, lease_tmp) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to create json")); + if (strchr(ip_tmp, ':')) { + /* This is an ipv6 lease */ + if ((server_duid_tmp + = virJSONValueObjectGetString(lease_tmp, "server-duid"))) { + if (!*server_duid && VIR_STRDUP(*server_duid, server_duid_tmp) < 0) { + /* 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*/ goto cleanup; } - - ignore_value(virJSONValueArraySteal(leases_array, i)); + } 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; } } + + /* Move old lease to new array */ + if (virJSONValueArrayAppend(leases_array_new, lease_tmp) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to create json")); + goto cleanup; + } + + ignore_value(virJSONValueArraySteal(leases_array, i)); } ret = 0; -- 2.4.6