10:50 p.m.
I posted this just before 0.8.0 is out and it was bad timing, I suppose.
So try to resend.
The following series of patches is a prototype implementation of dnsmasq
module. It implements an idea originally suggested by Dan-san and can
address the problem that too many --dhcp-host args hitting ARG_MAX limit
I reported last year [1].
[1]
https://www.redhat.com/archives/libvir-list/2009-October/msg00216.html
Thanks,
Satoru SATOH
10:53 p.m.
New subject: [libvirt] [Resend][PATCH 1/3] Add dnsmasq module
This patch adds the files implements dnsmasq (hostsfile) module.
Signed-off-by: Satoru SATOH <satoru.satoh(a)gmail.com>
---
src/util/dnsmasq.c | 326 ++++++++++++++++++++++++++++++++++++++++++++++++++++
src/util/dnsmasq.h | 59 ++++++++++
2 files changed, 385 insertions(+), 0 deletions(-)
create mode 100644 src/util/dnsmasq.c
create mode 100644 src/util/dnsmasq.h
diff --git a/src/util/dnsmasq.c b/src/util/dnsmasq.c
new file mode 100644
index 0000000..b46654e
--- /dev/null
+++ b/src/util/dnsmasq.c
@@ -0,0 +1,326 @@
+/*
+ * Copyright (C) 2010 Satoru SATOH <satoru.satoh(a)gmail.com>
+ * Copyright (C) 2007-2009 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ * Based on iptables.c
+ */
+
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdarg.h>
+#include <string.h>
+#include <errno.h>
+#include <limits.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <signal.h>
+
+#ifdef HAVE_PATHS_H
+#include <paths.h>
+#endif
+
+#include "internal.h"
+#include "dnsmasq.h"
+#include "util.h"
+#include "memory.h"
+#include "virterror_internal.h"
+#include "logging.h"
+
+#define VIR_FROM_THIS VIR_FROM_NETWORK
+
+static void
+dhcphostFree(dnsmasqDhcpHost *host)
+{
+ VIR_FREE(host->host);
+}
+
+static void
+hostsfileFree(dnsmasqHostsfile *hostsfile)
+{
+ int i;
+
+ if (hostsfile->hosts) {
+ for (i = 0; i < hostsfile->nhosts; i++)
+ dhcphostFree(&hostsfile->hosts[i]);
+
+ VIR_FREE(hostsfile->hosts);
+
+ hostsfile->nhosts = 0;
+ }
+
+ hostsfile->path[0] = '\0';
+
+ VIR_FREE(hostsfile);
+}
+
+static int
+hostsfileAdd(dnsmasqHostsfile *hostsfile,
+ const char *mac,
+ const char *ip,
+ const char *name)
+{
+ if (VIR_REALLOC_N(hostsfile->hosts, hostsfile->nhosts + 1) < 0)
+ goto alloc_error;
+
+ if (name) {
+ if (virAsprintf(&hostsfile->hosts[hostsfile->nhosts].host,
"%s,%s,%s",
+ mac, ip, name) < 0) {
+ goto alloc_error;
+ }
+ } else {
+ if (virAsprintf(&hostsfile->hosts[hostsfile->nhosts].host,
"%s,%s",
+ mac, ip) < 0) {
+ goto alloc_error;
+ }
+ }
+
+ hostsfile->nhosts++;
+
+ return 0;
+
+ alloc_error:
+ virReportSystemError(ENOMEM,
+ _("Failed to add dhcp host entry: mac=%s, ip=%s, name=%s\n"),
+ mac, ip, (name ? name : "(null)"));
+ return -1;
+}
+
+static dnsmasqHostsfile *
+hostsfileNew(const char *name,
+ const char *config_dir)
+{
+ int err;
+ dnsmasqHostsfile *hostsfile;
+
+ if (VIR_ALLOC(hostsfile) < 0)
+ return NULL;
+
+ hostsfile->hosts = NULL;
+ hostsfile->nhosts = 0;
+
+ if ((err = virFileMakePath(config_dir))) {
+ virReportSystemError(err, _("cannot create config directory
'%s'"),
+ config_dir);
+ goto error;
+ }
+
+ if (virFileBuildPath(config_dir, name, ".hostsfile", hostsfile->path,
+ sizeof(hostsfile->path)) < 0) {
+ virReportSystemError(err,
+ _("config filename '%s/%s.%s' is too long"),
+ config_dir, name, ".hostsfile");
+ goto error;
+ }
+
+ return hostsfile;
+
+ error:
+ hostsfileFree(hostsfile);
+ return NULL;
+}
+
+static int
+hostsfileWrite(const char *path, dnsmasqDhcpHost *hosts, int nhosts)
+{
+ char tmp[PATH_MAX];
+ FILE *f;
+ int istmp;
+ int i;
+
+ if (nhosts == 0 && unlink(path) == 0)
+ return 0;
+
+ if (snprintf(tmp, PATH_MAX, "%s.new", path) >= PATH_MAX)
+ return EINVAL;
+
+ istmp = 1;
+
+ if (!(f = fopen(tmp, "w"))) {
+ istmp = 0;
+ if (!(f = fopen(path, "w")))
+ return errno;
+ }
+
+ for (i = 0; i < nhosts; i++) {
+ if (fputs(hosts[i].host, f) == EOF || fputc('\n', f) == EOF) {
+ fclose(f);
+ if (istmp)
+ unlink(tmp);
+ return errno;
+ }
+ }
+
+ fclose(f);
+
+ if (istmp && rename(tmp, path) < 0) {
+ unlink(tmp);
+ return errno;
+ }
+
+ if (istmp)
+ unlink(tmp);
+
+ return 0;
+}
+
+static int
+hostsfileSave(dnsmasqHostsfile *hostsfile)
+{
+ int err = hostsfileWrite(hostsfile->path, hostsfile->hosts,
+ hostsfile->nhosts);
+
+ if (err < 0) {
+ virReportSystemError(err, _("cannot write config file '%s'"),
+ hostsfile->path);
+ return -1;
+ }
+
+ return 0;
+}
+
+static int
+hostsfileDelete(dnsmasqHostsfile *hostsfile)
+{
+ if (!virFileExists(hostsfile->path))
+ return 0;
+
+ if (unlink(hostsfile->path) < 0) {
+ virReportSystemError(errno, _("cannot remove config file
'%s'"),
+ hostsfile->path);
+ return -1;
+ }
+
+ return 0;
+}
+
+/**
+ * dnsmasqContextNew:
+ *
+ * Create a new Dnsmasq context
+ *
+ * Returns a pointer to the new structure or NULL in case of error
+ */
+dnsmasqContext *
+dnsmasqContextNew(const char *network_name,
+ const char *config_dir)
+{
+ dnsmasqContext *ctx;
+
+ if (VIR_ALLOC(ctx) < 0)
+ return NULL;
+
+ if (!(ctx->hostsfile = hostsfileNew(network_name, config_dir)))
+ goto error;
+
+ return ctx;
+
+ error:
+ dnsmasqContextFree(ctx);
+ return NULL;
+}
+
+/**
+ * dnsmasqContextFree:
+ * @ctx: pointer to the dnsmasq context
+ *
+ * Free the resources associated with an dnsmasq context
+ */
+void
+dnsmasqContextFree(dnsmasqContext *ctx)
+{
+ if (!ctx)
+ return;
+
+ if (ctx->hostsfile)
+ hostsfileFree(ctx->hostsfile);
+
+ VIR_FREE(ctx);
+}
+
+/**
+ * dnsmasqAddDhcpHost:
+ * @ctx: pointer to the dnsmasq context for each network
+ * @mac: pointer to the string contains mac address of the host
+ * @ip: pointer to the string contains ip address of the host
+ * @name: pointer to the string contains hostname of the host or NULL
+ *
+ * Add dhcp-host entry.
+ */
+void
+dnsmasqAddDhcpHost(dnsmasqContext *ctx,
+ const char *mac,
+ const char *ip,
+ const char *name)
+{
+ if (ctx->hostsfile)
+ hostsfileAdd(ctx->hostsfile, mac, ip, name);
+}
+
+/**
+ * dnsmasqSave:
+ * @ctx: pointer to the dnsmasq context for each network
+ *
+ * Saves all the configurations associated with a context to disk.
+ */
+int
+dnsmasqSave(const dnsmasqContext *ctx)
+{
+ if (ctx->hostsfile)
+ return hostsfileSave(ctx->hostsfile);
+
+ return 0;
+}
+
+
+/**
+ * dnsmasqDelete:
+ * @ctx: pointer to the dnsmasq context for each network
+ *
+ * Delete all the configuration files associated with a context.
+ */
+int
+dnsmasqDelete(const dnsmasqContext *ctx)
+{
+ if (ctx->hostsfile)
+ return hostsfileDelete(ctx->hostsfile);
+
+ return 0;
+}
+
+/**
+ * dnsmasqReload:
+ * @pid: the pid of the target dnsmasq process
+ *
+ * Reloads all the configurations associated to a context
+ */
+int
+dnsmasqReload(pid_t pid)
+{
+ if (kill(pid, SIGHUP) != 0) {
+ virReportSystemError(errno,
+ _("Failed to make dnsmasq (PID: %d) reloading config files.\n"),
+ pid);
+ return -1;
+ }
+
+ return 0;
+}
+
diff --git a/src/util/dnsmasq.h b/src/util/dnsmasq.h
new file mode 100644
index 0000000..987226b
--- /dev/null
+++ b/src/util/dnsmasq.h
@@ -0,0 +1,59 @@
+/*
+ * Copyright (C) 2010 Satoru SATOH <satoru.satoh(a)gmail.com>
+ * Copyright (C) 2007, 2008 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ * based on iptables.h
+ */
+
+#ifndef __DNSMASQ_H__
+#define __DNSMASQ_H__
+
+typedef struct
+{
+ /*
+ * Each entry holds a string,
"<mac_addr>,<hostname>,<ip_addr>" such as
+ * "01:23:45:67:89:0a,foo,10.0.0.3".
+ */
+ char *host;
+
+} dnsmasqDhcpHost;
+
+typedef struct
+{
+ int nhosts;
+ dnsmasqDhcpHost *hosts;
+
+ char path[PATH_MAX]; /* Absolute path of dnsmasq's hostsfile. */
+} dnsmasqHostsfile;
+
+typedef struct
+{
+ dnsmasqHostsfile *hostsfile;
+} dnsmasqContext;
+
+dnsmasqContext * dnsmasqContextNew(const char *network_name,
+ const char *config_dir);
+void dnsmasqContextFree(dnsmasqContext *ctx);
+void dnsmasqAddDhcpHost(dnsmasqContext *ctx,
+ const char *mac,
+ const char *ip,
+ const char *name);
+int dnsmasqSave(const dnsmasqContext *ctx);
+int dnsmasqDelete(const dnsmasqContext *ctx);
+int dnsmasqReload(pid_t pid);
+
+#endif /* __DNSMASQ_H__ */
--
1.6.2.5
12:42 p.m.
New subject: [libvirt] [Resend][PATCH 1/3] Add dnsmasq module
Satoru SATOH wrote:
This patch adds the files implements dnsmasq (hostsfile) module.
Here is some feedback not on the overall structure or utility,
but more on a few implementation details that stood out.
...
diff --git a/src/util/dnsmasq.c b/src/util/dnsmasq.c
...
+static void
+dhcphostFree(dnsmasqDhcpHost *host)
+{
+ VIR_FREE(host->host);
+}
+
+static void
+hostsfileFree(dnsmasqHostsfile *hostsfile)
+{
+ int i;
Making that
unsigned int i;
is slightly better (if possible[*]),
since it's obvious "i" will never need to be signed.
[*] whether it is possible depends on the type of the nhosts
member and the set of gcc warnings enabled. Build with
--enable-compile-warnings=error to check.
+
+ if (hostsfile->hosts) {
+ for (i = 0; i < hostsfile->nhosts; i++)
+ dhcphostFree(&hostsfile->hosts[i]);
+
+ VIR_FREE(hostsfile->hosts);
+
+ hostsfile->nhosts = 0;
+ }
+
+ hostsfile->path[0] = '\0';
+
+ VIR_FREE(hostsfile);
+}
...
+static dnsmasqHostsfile *
+hostsfileNew(const char *name,
+ const char *config_dir)
+{
+ int err;
+ dnsmasqHostsfile *hostsfile;
+
+ if (VIR_ALLOC(hostsfile) < 0)
+ return NULL;
+
+ hostsfile->hosts = NULL;
+ hostsfile->nhosts = 0;
+
+ if ((err = virFileMakePath(config_dir))) {
+ virReportSystemError(err, _("cannot create config directory
'%s'"),
+ config_dir);
+ goto error;
+ }
+
+ if (virFileBuildPath(config_dir, name, ".hostsfile", hostsfile->path,
+ sizeof(hostsfile->path)) < 0) {
+ virReportSystemError(err,
+ _("config filename '%s/%s.%s' is too long"),
+ config_dir, name, ".hostsfile");
+ goto error;
+ }
+
+ return hostsfile;
+
+ error:
+ hostsfileFree(hostsfile);
+ return NULL;
+}
+
+static int
+hostsfileWrite(const char *path, dnsmasqDhcpHost *hosts, int nhosts)
+{
+ char tmp[PATH_MAX];
It's good to avoid use of PATH_MAX.
Declare it like this instead, ...
char *tmp;
+ FILE *f;
+ int istmp;
+ int i;
+
+ if (nhosts == 0 && unlink(path) == 0)
+ return 0;
+
+ if (snprintf(tmp, PATH_MAX, "%s.new", path) >= PATH_MAX)
+ return EINVAL;
... and here, use virAsprintf instead of snprintf.
If the name ends up being too long, you'll catch it when fopen fails.
+ istmp = 1;
+
+ if (!(f = fopen(tmp, "w"))) {
+ istmp = 0;
+ if (!(f = fopen(path, "w")))
+ return errno;
+ }
+
+ for (i = 0; i < nhosts; i++) {
+ if (fputs(hosts[i].host, f) == EOF || fputc('\n', f) == EOF) {
+ fclose(f);
fclose may well clobber the fputs-set errno.
Save it, with this, just before the fclose.
int saved_errno = errno
+ if (istmp)
+ unlink(tmp);
unlink may clobber errno, too.
So here, you should use "return saved_errno;"
+ return errno;
+ }
+ }
+
+ fclose(f);
The fputs calls can succeed, yet fclose's write or close
can still fail. So you must check for fclose failure, too.
+
+ if (istmp && rename(tmp, path) < 0) {
Save errno here too, so unlink can't clobber it.
int saved_errno = errno
+ unlink(tmp);
+ return errno;
and use saved_errno here.
+ }
+
+ if (istmp)
+ unlink(tmp);
+
+ return 0;
+}
9:26 a.m.
New subject: [libvirt] [Resend][PATCH 1/3] Add dnsmasq module
Thanks a lot for your detailed feedback!
On Tue, Apr 20, 2010 at 07:42:45PM +0200, Jim Meyering wrote:
Satoru SATOH wrote:
> This patch adds the files implements dnsmasq (hostsfile) module.
Here is some feedback not on the overall structure or utility,
but more on a few implementation details that stood out.
...
> diff --git a/src/util/dnsmasq.c b/src/util/dnsmasq.c
...
> +static void
> +dhcphostFree(dnsmasqDhcpHost *host)
> +{
> + VIR_FREE(host->host);
> +}
> +
> +static void
> +hostsfileFree(dnsmasqHostsfile *hostsfile)
> +{
> + int i;
Making that
unsigned int i;
is slightly better (if possible[*]),
since it's obvious "i" will never need to be signed.
[*] whether it is possible depends on the type of the nhosts
member and the set of gcc warnings enabled. Build with
--enable-compile-warnings=error to check.
absolutely right.
fixed locally and will post it again.
> +
> + if (hostsfile->hosts) {
> + for (i = 0; i < hostsfile->nhosts; i++)
> + dhcphostFree(&hostsfile->hosts[i]);
> +
> + VIR_FREE(hostsfile->hosts);
> +
> + hostsfile->nhosts = 0;
> + }
> +
> + hostsfile->path[0] = '\0';
> +
> + VIR_FREE(hostsfile);
> +}
...
> +static dnsmasqHostsfile *
> +hostsfileNew(const char *name,
> + const char *config_dir)
> +{
> + int err;
> + dnsmasqHostsfile *hostsfile;
> +
> + if (VIR_ALLOC(hostsfile) < 0)
> + return NULL;
> +
> + hostsfile->hosts = NULL;
> + hostsfile->nhosts = 0;
> +
> + if ((err = virFileMakePath(config_dir))) {
> + virReportSystemError(err, _("cannot create config directory
'%s'"),
> + config_dir);
> + goto error;
> + }
> +
> + if (virFileBuildPath(config_dir, name, ".hostsfile",
hostsfile->path,
> + sizeof(hostsfile->path)) < 0) {
> + virReportSystemError(err,
> + _("config filename '%s/%s.%s' is too long"),
> + config_dir, name, ".hostsfile");
> + goto error;
> + }
> +
> + return hostsfile;
> +
> + error:
> + hostsfileFree(hostsfile);
> + return NULL;
> +}
> +
> +static int
> +hostsfileWrite(const char *path, dnsmasqDhcpHost *hosts, int nhosts)
> +{
> + char tmp[PATH_MAX];
It's good to avoid use of PATH_MAX.
Declare it like this instead, ...
I understand.
char *tmp;
> + FILE *f;
> + int istmp;
> + int i;
> +
> + if (nhosts == 0 && unlink(path) == 0)
> + return 0;
> +
> + if (snprintf(tmp, PATH_MAX, "%s.new", path) >= PATH_MAX)
> + return EINVAL;
... and here, use virAsprintf instead of snprintf.
If the name ends up being too long, you'll catch it when fopen fails.
ok, replaced locally.
> + istmp = 1;
> +
> + if (!(f = fopen(tmp, "w"))) {
> + istmp = 0;
> + if (!(f = fopen(path, "w")))
> + return errno;
> + }
> +
> + for (i = 0; i < nhosts; i++) {
> + if (fputs(hosts[i].host, f) == EOF || fputc('\n', f) == EOF) {
> + fclose(f);
fclose may well clobber the fputs-set errno.
Save it, with this, just before the fclose.
int saved_errno = errno
> + if (istmp)
> + unlink(tmp);
unlink may clobber errno, too.
So here, you should use "return saved_errno;"
> + return errno;
> + }
> + }
> +
> + fclose(f);
The fputs calls can succeed, yet fclose's write or close
can still fail. So you must check for fclose failure, too.
> +
> + if (istmp && rename(tmp, path) < 0) {
Save errno here too, so unlink can't clobber it.
int saved_errno = errno
> + unlink(tmp);
> + return errno;
and use saved_errno here.
done.
> + }
> +
> + if (istmp)
> + unlink(tmp);
> +
> + return 0;
> +}
--
Thanks,
Satoru SATOH
1:06 p.m.
New subject: [libvirt] [Resend][PATCH 1/3] Add dnsmasq module
On 04/18/2010 09:53 PM, Satoru SATOH wrote:
This patch adds the files implements dnsmasq (hostsfile) module.
Signed-off-by: Satoru SATOH <satoru.satoh(a)gmail.com>
In addition to Jim's comments:
---
src/util/dnsmasq.c | 326 ++++++++++++++++++++++++++++++++++++++++++++++++++++
src/util/dnsmasq.h | 59 ++++++++++
2 files changed, 385 insertions(+), 0 deletions(-)
create mode 100644 src/util/dnsmasq.c
create mode 100644 src/util/dnsmasq.h
diff --git a/src/util/dnsmasq.c b/src/util/dnsmasq.c
new file mode 100644
index 0000000..b46654e
--- /dev/null
+++ b/src/util/dnsmasq.c
@@ -0,0 +1,326 @@
+/*
+ * Copyright (C) 2010 Satoru SATOH <satoru.satoh(a)gmail.com>
+ * Copyright (C) 2007-2009 Red Hat, Inc.
You did the right thing in preserving the Red Hat copyright from
src/iptables.c, since that was your template. However, it makes
maintenance easier if you list the Red Hat line first, and extend its
copyright to 2010, since emacs copyright updating hooks only catch the
first copyright line.
+
+#ifdef HAVE_PATHS_H
+#include <paths.h>
+#endif
This fails 'make syntax-check' if you have cppi installed. Indent the
include line:
#ifdef HAVE_PATHS_H
# include <paths.h>
+static int
+hostsfileWrite(const char *path, dnsmasqDhcpHost *hosts, int nhosts)
+{
+ char tmp[PATH_MAX];
+ FILE *f;
+ int istmp;
Make this 'bool istmp', for how you are using it.
+
+typedef struct
+{
+ int nhosts;
+ dnsmasqDhcpHost *hosts;
+
+ char path[PATH_MAX]; /* Absolute path of dnsmasq's hostsfile. */
+} dnsmasqHostsfile;
Jim didn't mention this use of PATH_MAX, but it is worth refactoring.
In particular, PATH_MAX can cause problems if we ever try to port to GNU
Hurd, where there is no limit (I know that the current libvirt code base
already has uses of PATH_MAX, but we shouldn't be adding more).
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
10:55 p.m.
New subject: [libvirt] [Resend][PATCH 2/3] Add build support for dnsmasq module
This patch adds build support for the dnsmasq module.
Signed-off-by: Satoru SATOH <satoru.satoh(a)gmail.com>
---
src/Makefile.am | 1 +
src/libvirt_private.syms | 9 +++++++++
2 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/src/Makefile.am b/src/Makefile.am
index 66dc349..9b3feb0 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -59,6 +59,7 @@ UTIL_SOURCES = \
util/hooks.c util/hooks.h \
util/iptables.c util/iptables.h \
util/ebtables.c util/ebtables.h \
+ util/dnsmasq.c util/dnsmasq.h \
util/json.c util/json.h \
util/logging.c util/logging.h \
util/macvtap.c util/macvtap.h \
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 7950bcd..11791a7 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -326,6 +326,15 @@ iptablesRemoveTcpInput;
iptablesRemoveUdpInput;
+# dnsmasq.h
+dnsmasqContextNew;
+dnsmasqContextFree;
+dnsmasqAddDhcpHost;
+dnsmasqSave;
+dnsmasqDelete;
+dnsmasqReload;
+
+
# libvirt_internal.h
virDrvSupportsFeature;
virDomainMigratePrepare;
--
1.6.2.5
10:58 p.m.
New subject: [libvirt] [Resend][PATCH 3/3] Make dnsmasq started from libvirtd with --dhcp-hostsfile option
This patch makes dnsmasq daemon started from libvirtd with a
--dhcp-hostsfile option instead of --dhcp-host options for each
'//ip/dhcp/host' entries defined in network xml file.
NOTE: I'm not sure where and when to save the dnsmasq hostsfile so that
make it saved in dnsmasq's local statedir when network is defined or
started in the meantime. Especially, I would like to get some advices
for this. (I'm afraid that NETWORK_STATE_DIR is inappropriate as
dnsmasq process will run as nobody and it will not be able to
read the saved hostsfile.)
Signed-off-by: Satoru SATOH <satoru.satoh(a)gmail.com>
---
src/network/bridge_driver.c | 78 +++++++++++++++++++++++++++++++++---------
1 files changed, 61 insertions(+), 17 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 83ab00e..59b0de9 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -57,10 +57,13 @@
#include "iptables.h"
#include "bridge.h"
#include "logging.h"
+#include "dnsmasq.h"
#define NETWORK_PID_DIR LOCAL_STATE_DIR "/run/libvirt/network"
#define NETWORK_STATE_DIR LOCAL_STATE_DIR "/lib/libvirt/network"
+#define DNSMASQ_STATE_DIR LOCAL_STATE_DIR "/lib/dnsmasq"
+
#define VIR_FROM_THIS VIR_FROM_NETWORK
#define networkReportError(code, ...) \
@@ -361,6 +364,29 @@ networkShutdown(void) {
static int
+networkSaveDnsmasqHostsfile(virNetworkObjPtr network,
+ dnsmasqContext *dctx,
+ int force)
+{
+ int r;
+
+ if (! force && virFileExists(dctx->hostsfile->path))
+ return 1;
+
+ for (r = 0 ; r < network->def->nhosts ; r++) {
+ virNetworkDHCPHostDefPtr host = &(network->def->hosts[r]);
+ if ((host->mac) && (host->ip))
+ dnsmasqAddDhcpHost(dctx, host->mac, host->ip, host->name);
+ }
+
+ if (dnsmasqSave(dctx) < 0)
+ return 0;
+
+ return 1;
+}
+
+
+static int
networkBuildDnsmasqArgv(virNetworkObjPtr network,
const char *pidfile,
const char ***argv) {
@@ -401,8 +427,8 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
(2 * network->def->nranges) + /* --dhcp-range 10.0.0.2,10.0.0.254 */
/* --dhcp-lease-max=xxx if needed */
(network->def->nranges ? 0 : 1) +
- /* --dhcp-host 01:23:45:67:89:0a,hostname,10.0.0.3 */
- (2 * network->def->nhosts) +
+ /* --dhcp-hostsfile=/var/lib/dnsmasq/$NAME.hostsfile */
+ (network->def->nhosts > 0 ? 1 : 0) +
/* --enable-tftp --tftp-root /srv/tftp */
(network->def->tftproot ? 3 : 0) +
/* --dhcp-boot pxeboot.img[,,12.34.56.78] */
@@ -473,22 +499,22 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
APPEND_ARG(*argv, i++, buf);
}
- for (r = 0 ; r < network->def->nhosts ; r++) {
- virNetworkDHCPHostDefPtr host = &(network->def->hosts[r]);
- if ((host->mac) && (host->name)) {
- snprintf(buf, sizeof(buf), "%s,%s,%s",
- host->mac, host->name, host->ip);
- } else if (host->mac) {
- snprintf(buf, sizeof(buf), "%s,%s",
- host->mac, host->ip);
- } else if (host->name) {
- snprintf(buf, sizeof(buf), "%s,%s",
- host->name, host->ip);
- } else
- continue;
+ if (network->def->nhosts > 0) {
+ dnsmasqContext *dctx = dnsmasqContextNew(network->def->name,
DNSMASQ_STATE_DIR);
+ char *hostsfileArg;
- APPEND_ARG(*argv, i++, "--dhcp-host");
- APPEND_ARG(*argv, i++, buf);
+ if (dctx == NULL)
+ goto no_memory;
+
+ if (networkSaveDnsmasqHostsfile(network, dctx, 0)) {
+ if (virAsprintf(&hostsfileArg, "--dhcp-hostsfile=%s",
dctx->hostsfile->path) < 0) {
+ dnsmasqContextFree(dctx);
+ goto no_memory;
+ }
+ APPEND_ARG_LIT(*argv, i++, hostsfileArg);
+ }
+
+ dnsmasqContextFree(dctx);
}
if (network->def->tftproot) {
@@ -1294,6 +1320,15 @@ static virNetworkPtr networkDefine(virConnectPtr conn, const char
*xml) {
goto cleanup;
}
+ if (network->def->nhosts > 0) {
+ dnsmasqContext *dctx = dnsmasqContextNew(network->def->name,
DNSMASQ_STATE_DIR);
+ if (dctx == NULL)
+ goto cleanup;
+
+ networkSaveDnsmasqHostsfile(network, dctx, 1);
+ dnsmasqContextFree(dctx);
+ }
+
ret = virGetNetwork(conn, network->def->name, network->def->uuid);
cleanup:
@@ -1329,6 +1364,15 @@ static int networkUndefine(virNetworkPtr net) {
network) < 0)
goto cleanup;
+ if (network->def->nhosts > 0) {
+ dnsmasqContext *dctx = dnsmasqContextNew(network->def->name,
DNSMASQ_STATE_DIR);
+ if (dctx == NULL)
+ goto cleanup;
+
+ dnsmasqDelete(dctx);
+ dnsmasqContextFree(dctx);
+ }
+
virNetworkRemoveInactive(&driver->networks,
network);
network = NULL;
--
1.6.2.5
5328
days inactive
5331
days old
6 comments
3 participants
participants (3)
-
Eric Blake -
Jim Meyering -
Satoru SATOH