[libvirt] [PATCH v2] leaseshelper: improvements to support all events

[Eric Blake]

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 604 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140723/e272c538/attachment-0001.sig>