On 07/22/2014 09:20 AM, Peter Krempa wrote:
> On 07/22/14 01:03, 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 (*.staus) will still be supported.
s/staus/status/
>> + else {
>> + if (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 (server_duid) {
>> + if (!(lease_new = virJSONValueNewObject())) {
>> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> + _("failed to create json"));
>> + goto cleanup;
>> + }
>> +
>> + if (virJSONValueObjectAppendString(lease_new,
>> + "server-duid", server_duid) < 0)
>> + goto cleanup;
>
> Hmm this is really odd. Why is the "server_duid" stored as a part of the
> leases array as it's a completely separate info and occuring only once?
Indeed. The pre-patch file looks like:
[
{
"iaid": "1221229",
"ip-address": "2001:db8:ca2:2:1::95",
"mac-address": "52:54:00:12:a2:6d",
"hostname": "Fedora20",
"client-id":
"00:04:1a:c1:d9:6b:5a:0a:e2:bc:f8:4b:1e:37:2e:38:22:55"
"expiry-time": 1393244216
},
...
]
I think the enhanced format should look like:
{
"server-duid": "...",
"leases": [
{
"iaid": "1221229",
"ip-address": "2001:db8:ca2:2:1::95",
"mac-address": "52:54:00:12:a2:6d",
"hostname": "Fedora20",
"client-id":
"00:04:1a:c1:d9:6b:5a:0a:e2:bc:f8:4b:1e:37:2e:38:22:55"
"expiry-time": 1393244216
},
...
]
}
which means that when you first parse the file, the new code has to
determine if the parsed JSON item is an object (new style) or an array
(old style); and grab out the array from either style for the rest of
the code to manipulate.
>
> Unfortunately, looking at the format of the lease file the array of
> leases is the top level element. Is there a way we could (without
> complicating the code too much) convert it to a object so that it's
> extensible?
I agree - it's better to rearrange the file to place the new content as
a sibling to the existing content by creating another wrapper layer
around both, rather than commandeering an array slot for non-lease
information.
>
> The change to using the ENV variable has turned out great, unfortunately
> there's a problem with the lease file JSON we need to clear before
> proceeding.
Looking forward to v3.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list