Re: [libvirt] [PATCH v2] leaseshelper: improvements to support all events
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
Attachments:
- signature.asc (application/pgp-signature — 604 bytes)