[libvirt] [PATCH v4] leaseshelper: improvements to support all events
Peter Krempa
pkrempa at redhat.com
Thu Nov 20 09:21:18 UTC 2014
On 11/18/14 18:16, 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 (*.status) will still be supported.
>
> v4: * Removed extra data structures for segregating ipv4 and ipv6 leases.
>
> v3: * Add server-duid as an entry in the lease object for every ipv6 lease.
> * Remove unnecessary variables and double copies.
> * Take value from DNSMASQ_OLD_HOSTNAME if hostname is not known.
> http://www.redhat.com/archives/libvir-list/2014-August/msg00028.html
>
> v2: http://www.redhat.com/archives/libvir-list/2014-July/msg01109.html
>
> v1: https://www.redhat.com/archives/libvir-list/2014-July/msg00568.html
>
>
> ---
> src/network/bridge_driver.c | 3 +
> src/network/leaseshelper.c | 142 +++++++++++++++++++++++++++++++++++---------
> 2 files changed, 117 insertions(+), 28 deletions(-)
>
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index 6cb421c..6ecbc37 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -1280,7 +1280,10 @@ networkBuildDhcpDaemonCommandLine(virNetworkObjPtr network,
>
> cmd = virCommandNew(dnsmasqCapsGetBinaryPath(caps));
> virCommandAddArgFormat(cmd, "--conf-file=%s", configfile);
> + /* Libvirt gains full control of leases database */
> + virCommandAddArgFormat(cmd, "--leasefile-ro");
As we are now starting dnsmasq with the libvirt managed database, we
should get rid of the lease file configuration option present in the
config file.
> virCommandAddArgFormat(cmd, "--dhcp-script=%s", leaseshelper_path);
> + virCommandAddEnvPair(cmd, "VIR_BRIDGE_NAME", network->def->bridge);
>
> *cmdout = cmd;
> ret = 0;
> diff --git a/src/network/leaseshelper.c b/src/network/leaseshelper.c
> index 5b3c9c3..fdd04a3 100644
> --- a/src/network/leaseshelper.c
> +++ b/src/network/leaseshelper.c
> @@ -50,6 +50,12 @@
> */
> #define VIR_NETWORK_DHCP_LEASE_FILE_SIZE_MAX (32 * 1024 * 1024)
>
> +/*
> + * Use this when passing possibly-NULL strings to printf-a-likes.
> + * Required for unknown parameters during init call.
> + */
> +#define EMPTY_STR(s) ((s) ? (s) : "*")
> +
> static const char *program_name;
>
> /* Display version information. */
> @@ -65,7 +71,7 @@ usage(int status)
> if (status) {
> fprintf(stderr, _("%s: try --help for more details\n"), program_name);
> } else {
> - printf(_("Usage: %s add|old|del mac|clientid ip [hostname]\n"
> + printf(_("Usage: %s add|old|del|init mac|clientid ip [hostname]\n"
> "Designed for use with 'dnsmasq --dhcp-script'\n"
> "Refer to man page of dnsmasq for more details'\n"),
> program_name);
> @@ -89,6 +95,7 @@ enum virLeaseActionFlags {
> VIR_LEASE_ACTION_ADD, /* Create new lease */
> VIR_LEASE_ACTION_OLD, /* Lease already exists, renew it */
> VIR_LEASE_ACTION_DEL, /* Delete the lease */
> + VIR_LEASE_ACTION_INIT, /* Tell dnsmasq of existing leases on restart */
>
> VIR_LEASE_ACTION_LAST
> };
> @@ -96,7 +103,7 @@ enum virLeaseActionFlags {
> VIR_ENUM_DECL(virLeaseAction);
>
> VIR_ENUM_IMPL(virLeaseAction, VIR_LEASE_ACTION_LAST,
> - "add", "old", "del");
> + "add", "old", "del", "init");
>
> int
> main(int argc, char **argv)
> @@ -107,12 +114,15 @@ 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 *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");
> - const char *leases_str = NULL;
> + const char *server_duid = virGetEnvAllowSUID("DNSMASQ_SERVER_DUID");
> long long currtime = 0;
> long long expirytime = 0;
> size_t i = 0;
> @@ -120,7 +130,6 @@ main(int argc, char **argv)
> int pid_file_fd = -1;
> int rv = EXIT_FAILURE;
> int custom_lease_file_len = 0;
> - bool add = false;
> bool delete = false;
> virJSONValuePtr lease_new = NULL;
> virJSONValuePtr lease_tmp = NULL;
> @@ -156,16 +165,17 @@ main(int argc, char **argv)
> }
> }
>
> - if (argc != 4 && argc != 5) {
> + if (argc != 4 && argc != 5 && argc != 2) {
> /* Refer man page of dnsmasq --dhcp-script for more details */
> usage(EXIT_FAILURE);
> }
>
> /* Make sure dnsmasq knows the interface. The interface name is not known
> - * when dnsmasq (re)starts and throws 'del' events for expired leases.
> - * So, if any old lease has expired, it will be automatically removed the
> - * next time this program is invoked */
> - if (!interface)
> + * via env variable set by dnsmasq when dnsmasq (re)starts and throws 'del'
> + * events for expired leases. So, libvirtd sets another env var for this
> + * purpose */
> + if (!interface &&
> + !(interface = virGetEnvAllowSUID("VIR_BRIDGE_NAME")))
> goto cleanup;
>
> ip = argv[3];
> @@ -176,6 +186,10 @@ main(int argc, char **argv)
> 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 (VIR_STRDUP(exptime, exptime_tmp) < 0)
> goto cleanup;
When dnsmasq is starting and invokes this helper with the "init"
parameter it does not populate the DNSMASQ_LEASE_EXPIRES env variable.
VIR_STRDUP then ignores that copy and ...
>
> @@ -184,7 +198,7 @@ main(int argc, char **argv)
> exptime[strlen(exptime) - 1] = '\0';
The program crashes here at the strlen() call thus it never populates
dnsmasq's internal lease database when started.
>
> /* Check if it is an IPv6 lease */
> - if (virGetEnvAllowSUID("DNSMASQ_IAID")) {
> + if (iaid) {
> mac = virGetEnvAllowSUID("DNSMASQ_MAC");
> clientid = argv[2];
> }
With the two things above fixed it looks good and seems to work
correctly with my test setup.
I'll be reposting this patch as I've already made the changes necessary
to make this work while trying to figure out why it doesn't work.
Peter
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20141120/2f7a7f33/attachment-0001.sig>
More information about the libvir-list
mailing list