On 07/11/14 02:19, 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 suppresses 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 applying
> the symlink technique, which helps us map events related to leases with their
> corresponding network bridges.
> Example:
> /var/lib/libvirt/dnsmasq/virbr0.helper -> /home/wani/libvirt/src/libvirt_leaseshelper
> /var/lib/libvirt/dnsmasq/virbr3.helper -> /home/wani/libvirt/src/libvirt_leaseshelper
>
> 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.
>
> ---
> src/network/bridge_driver.c | 43 +++++++++++-
> src/network/leaseshelper.c | 156 +++++++++++++++++++++++++++++++++++++-------
> 2 files changed, 175 insertions(+), 24 deletions(-)
Note this message is not a full review, just a few questions before I
carry on.
>
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index 6a2e760..1e1e53f 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -1287,9 +1303,30 @@ networkBuildDhcpDaemonCommandLine(virNetworkObjPtr network,
> LIBEXECDIR)))
> goto cleanup;
>
> + /* Symlink technique for dnsmasq lease file is needed so that libvirt
> + * can have complete control over the handling of leases database */
> +
> + /* Remove unwanted, old symlink */
> + if (unlink(pseudo_leaseshelper_path) < 0 &&
> + errno != ENOENT && errno != ENOTDIR) {
> + virReportSystemError(errno,
> + _("Failed to delete symlink '%s'"),
> + pseudo_leaseshelper_path);
> + goto cleanup;
> + }
> +
> + /* Create a new symlink */
> + if (symlink(leaseshelper_path, pseudo_leaseshelper_path) < 0) {
> + virReportSystemError(errno,
> + _("Failed to create symlink '%s' to '%s'"),
> + leaseshelper_path, pseudo_leaseshelper_path);
> + goto cleanup;
> + }
> +
> cmd = virCommandNew(dnsmasqCapsGetBinaryPath(caps));
> virCommandAddArgFormat(cmd, "--conf-file=%s", configfile);
> - virCommandAddArgFormat(cmd, "--dhcp-script=%s", leaseshelper_path);
> + virCommandAddArgFormat(cmd, "--dhcp-script=%s", pseudo_leaseshelper_path);
> + virCommandAddArgFormat(cmd, "--leasefile-ro");
Does dnsmasq pass through the rest of the environment we pass to it at
this point? The leaseshelper extracts quite a lot of stuff from the
environment variables that are passed by dnsmasq. In case it does we
could use an env variable to pass the interface name instead of linking
the helper and using different names.
A second issue that comes into my mind is compatibility with already
existing files. Does this work when you already have a lease file? (I
didn't try it, I'm just asking).
Peter
> main(int argc, char **argv)
> @@ -105,6 +113,7 @@ main(int argc, char **argv)
> char *pid_file = NULL;
> char *lease_entries = NULL;
> char *custom_lease_file = NULL;
> + char *server_duid = NULL;
> const char *ip = NULL;
> const char *mac = NULL;
> const char *iaid = virGetEnvAllowSUID("DNSMASQ_IAID");
> @@ -112,20 +121,26 @@ 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_env = virGetEnvAllowSUID("DNSMASQ_SERVER_DUID");
Here we extract a lot of stuff ...
> const char *leases_str = NULL;
> long long currtime = 0;
> long long expirytime = 0;
> size_t i = 0;
> + size_t count_ipv6 = 0;
> + size_t count_ipv4 = 0;
> int action = -1;
> int pid_file_fd = -1;
> int rv = EXIT_FAILURE;
> int custom_lease_file_len = 0;
> bool add = false;
> + bool init = false;
> bool delete = false;
> virJSONValuePtr lease_new = NULL;
> virJSONValuePtr lease_tmp = NULL;
> virJSONValuePtr leases_array = NULL;
> virJSONValuePtr leases_array_new = NULL;
> + virJSONValuePtr *leases_ipv4 = NULL;
> + virJSONValuePtr *leases_ipv6 = NULL;
>
> virSetErrorFunc(NULL, NULL);
> virSetErrorLogPriorityFunc(NULL);
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list