On 04/01/2011 06:45 AM, Michal Novotny wrote:
Hi,
this is the patch to introduce the internal infrastructure for
additional hosts for network bridge driver using the addnhosts*
API functions.
This is necessary for next part of the patch to support DNS
hosts definition in the network XML description.
Michal
Signed-off-by: Michal Novotny<minovotn(a)redhat.com>
---
src/libvirt_private.syms | 1 +
src/network/bridge_driver.c | 3 +
src/util/dnsmasq.c | 266 ++++++++++++++++++++++++++++++++++++++++++-
src/util/dnsmasq.h | 22 ++++-
4 files changed, 287 insertions(+), 5 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 65a86d3..73c3f77 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -186,6 +186,7 @@ virUnrefStream;
# dnsmasq.h
dnsmasqAddDhcpHost;
+dnsmasqAddHost;
dnsmasqContextFree;
dnsmasqContextNew;
dnsmasqDelete;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 41b14f9..4ad3143 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -589,6 +589,9 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
if (dctx->hostsfile->nhosts)
virCommandAddArgPair(cmd, "--dhcp-hostsfile",
dctx->hostsfile->path);
+ if (dctx->addnhostsfile->nhosts)
+ virCommandAddArgPair(cmd, "--addn-hosts",
+ dctx->addnhostsfile->path);
dnsmasqContextFree(dctx);
}
diff --git a/src/util/dnsmasq.c b/src/util/dnsmasq.c
index be230e1..fee3b90 100644
--- a/src/util/dnsmasq.c
+++ b/src/util/dnsmasq.c
@@ -48,6 +48,7 @@
#define VIR_FROM_THIS VIR_FROM_NETWORK
#define DNSMASQ_HOSTSFILE_SUFFIX "hostsfile"
+#define DNSMASQ_ADDNHOSTSFILE_SUFFIX "addnhosts"
static void
dhcphostFree(dnsmasqDhcpHost *host)
@@ -56,6 +57,231 @@ dhcphostFree(dnsmasqDhcpHost *host)
}
static void
+addnhostFree(dnsmasqAddnHost *host)
+{
+ VIR_FREE(host->hostnames);
+ VIR_FREE(host->ip);
+}
+
+static void
+addnhostsFree(dnsmasqAddnHostsfile *addnhostsfile)
+{
+ unsigned int i;
+
+ if (addnhostsfile->hosts) {
+ for (i = 0; i< addnhostsfile->nhosts; i++)
+ addnhostFree(&addnhostsfile->hosts[i]);
+
+ VIR_FREE(addnhostsfile->hosts);
+
+ addnhostsfile->nhosts = 0;
+ }
+
+ VIR_FREE(addnhostsfile->path);
+
+ VIR_FREE(addnhostsfile);
+}
+
+static int
+addnhostsAdd(dnsmasqAddnHostsfile *addnhostsfile,
+ virSocketAddr *ip,
+ const char *name)
+{
+ char *ipstr = NULL;
+ int idx = -1;
+ int i;
+
+ if (!(ipstr = virSocketFormatAddr(ip)))
+ return -1;
+
+ for (i = 0; i< addnhostsfile->nhosts; i++) {
+ if (STREQ((const char *)addnhostsfile->hosts[i].ip, (const char *)ipstr)) {
+ idx = i;
+ break;
+ }
+ }
+
+ if (idx< 0) {
+ if (VIR_REALLOC_N(addnhostsfile->hosts, addnhostsfile->nhosts + 1)<
0)
+ goto alloc_error;
+
+ idx = addnhostsfile->nhosts;
+ if (VIR_ALLOC(addnhostsfile->hosts[idx].hostnames)< 0)
+ goto alloc_error;
+
+ if (virAsprintf(&addnhostsfile->hosts[idx].ip, "%s", ipstr)<
0)
+ goto alloc_error;
+
+ addnhostsfile->hosts[idx].nhostnames = 0;
+ addnhostsfile->nhosts++;
+ }
+
+ if (VIR_REALLOC_N(addnhostsfile->hosts[idx].hostnames,
addnhostsfile->hosts[idx].nhostnames + 1)< 0)
+ goto alloc_error;
+
+ if
(virAsprintf(&addnhostsfile->hosts[idx].hostnames[addnhostsfile->hosts[idx].nhostnames],
"%s", name)< 0)
+ goto alloc_error;
+
+ VIR_FREE(ipstr);
+
+ addnhostsfile->hosts[idx].nhostnames++;
+
+ return 0;
+
+ alloc_error:
+ virReportOOMError();
+ VIR_FREE(ipstr);
+ return -1;
+}
+
+static dnsmasqAddnHostsfile *
+addnhostsNew(const char *name,
+ const char *config_dir)
+{
+ int err;
+ dnsmasqAddnHostsfile *addnhostsfile;
+
+ if (VIR_ALLOC(addnhostsfile)< 0) {
+ virReportOOMError();
+ return NULL;
+ }
+
+ addnhostsfile->hosts = NULL;
+ addnhostsfile->nhosts = 0;
+
+ if (virAsprintf(&addnhostsfile->path, "%s/%s.%s", config_dir,
name,
+ DNSMASQ_ADDNHOSTSFILE_SUFFIX)< 0) {
+ virReportOOMError();
+ goto error;
+ }
+
+ if ((err = virFileMakePath(config_dir))) {
+ virReportSystemError(err, _("cannot create config directory
'%s'"),
+ config_dir);
+ goto error;
+ }
+
+ return addnhostsfile;
+
+ error:
+ addnhostsFree(addnhostsfile);
+ return NULL;
+}
+
+static int
+addnhostsWrite(const char *path,
+ dnsmasqAddnHost *hosts,
+ unsigned int nhosts)
+{
+ char *tmp;
+ FILE *f;
+ bool istmp = true;
+ unsigned int i, ii;
+ int rc = 0;
+
+ if (nhosts == 0)
+ return rc;
+
+ if (virAsprintf(&tmp, "%s.new", path)< 0)
+ return ENOMEM;
Oooh! Figuring that you had cut-pasted this code from the existing
hostsFileWrite(), and knowing that in general we like to return -errno
on failure (although there are still some exceptions), I went to look at
the original code and found a bug! (hostsWriteFile() is returning errno
on failure, but at least one caller is checking for ret < 0).
I'm sending in a patch for that momentarily, and you should base the
next revision of your code on that.
+
+ if (!(f = fopen(tmp, "w"))) {
+ istmp = false;
+ if (!(f = fopen(path, "w"))) {
+ rc = errno;
+ goto cleanup;
+ }
+ }
+
+ for (i = 0; i< nhosts; i++) {
+ if (fputs(hosts[i].ip, f) == EOF || fputc('\t', f) == EOF) {
+ rc = errno;
+ VIR_FORCE_FCLOSE(f);
+
+ if (istmp)
+ unlink(tmp);
+
+ goto cleanup;
+ }
+
+ for (ii = 0; ii< hosts[i].nhostnames; ii++) {
+ if (fputs(hosts[i].hostnames[ii], f) == EOF || fputc('\t', f) ==
EOF) {
+ rc = errno;
+ VIR_FORCE_FCLOSE(f);
+
+ if (istmp)
+ unlink(tmp);
+
+ goto cleanup;
+ }
+ }
+
+ if (fputc('\n', f) == EOF) {
+ rc = errno;
+ VIR_FORCE_FCLOSE(f);
+
+ if (istmp)
+ unlink(tmp);
+
+ goto cleanup;
+ }
+ }
+
+ if (VIR_FCLOSE(f) == EOF) {
+ rc = errno;
+ goto cleanup;
+ }
+
+ if (istmp) {
+ if (rename(tmp, path)< 0) {
+ rc = errno;
+ unlink(tmp);
+ goto cleanup;
+ }
+
+ if (unlink(tmp)< 0) {
+ rc = errno;
+ goto cleanup;
+ }
+ }
+
+ cleanup:
+ VIR_FREE(tmp);
+
+ return rc;
+}
+
+static int
+addnhostsSave(dnsmasqAddnHostsfile *addnhostsfile)
+{
+ int err = addnhostsWrite(addnhostsfile->path, addnhostsfile->hosts,
+ addnhostsfile->nhosts);
+
+ if (err< 0) {
+ virReportSystemError(err, _("cannot write config file '%s'"),
+ addnhostsfile->path);
+ return -1;
+ }
+
+ return 0;
+}
+
+static int
+addnhostsDelete(dnsmasqAddnHostsfile *addnhostsfile)
+{
+ if (!virFileExists(addnhostsfile->path))
+ return 0;
+
+ if (unlink(addnhostsfile->path)< 0) {
+ virReportSystemError(errno, _("cannot remove config file
'%s'"),
+ addnhostsfile->path);
+ return -1;
+ }
+
+ return 0;
+}
Except for the type of the arg, this function is identical to
hostsfileDelete. How about changing hostsfileDelete() to take a "const
char *file", then changing its name to something more generic, and
calling the same function from both places?
+
+static void
hostsfileFree(dnsmasqHostsfile *hostsfile)
{
unsigned int i;
@@ -255,6 +481,8 @@ dnsmasqContextNew(const char *network_name,
if (!(ctx->hostsfile = hostsfileNew(network_name, config_dir)))
goto error;
+ if (!(ctx->addnhostsfile = addnhostsNew(network_name, config_dir)))
+ goto error;
return ctx;
@@ -277,6 +505,8 @@ dnsmasqContextFree(dnsmasqContext *ctx)
if (ctx->hostsfile)
hostsfileFree(ctx->hostsfile);
+ if (ctx->addnhostsfile)
+ addnhostsFree(ctx->addnhostsfile);
VIR_FREE(ctx);
}
@@ -300,6 +530,24 @@ dnsmasqAddDhcpHost(dnsmasqContext *ctx,
hostsfileAdd(ctx->hostsfile, mac, ip, name);
}
+/*
+ * dnsmasqAddHost:
+ * @ctx: pointer to the dnsmasq context for each network
+ * @ip: pointer to the socket address contains ip of the host
+ * @name: pointer to the string contains hostname of the host
+ *
+ * Add additional host entry.
+ */
+
+void
+dnsmasqAddHost(dnsmasqContext *ctx,
+ virSocketAddr *ip,
+ const char *name)
+{
+ if (ctx->addnhostsfile)
+ addnhostsAdd(ctx->addnhostsfile, ip, name);
+}
+
/**
* dnsmasqSave:
* @ctx: pointer to the dnsmasq context for each network
@@ -309,10 +557,15 @@ dnsmasqAddDhcpHost(dnsmasqContext *ctx,
int
dnsmasqSave(const dnsmasqContext *ctx)
{
+ int ret1 = 0;
+ int ret2 = 0;
+
if (ctx->hostsfile)
- return hostsfileSave(ctx->hostsfile);
+ ret1 = hostsfileSave(ctx->hostsfile);
+ if (ctx->addnhostsfile)
+ ret2 = addnhostsSave(ctx->addnhostsfile);
- return 0;
+ return ((ret1 == 0)&& (ret2 == 0)) ? 0 : -1;
Does it really need to be this complicated? Or can you just have a
single "ret", and skip the 2nd save if the first save fails?
}
@@ -325,10 +578,15 @@ dnsmasqSave(const dnsmasqContext *ctx)
int
dnsmasqDelete(const dnsmasqContext *ctx)
{
+ int ret1 = 0;
+ int ret2 = 0;
+
if (ctx->hostsfile)
- return hostsfileDelete(ctx->hostsfile);
+ ret1 = hostsfileDelete(ctx->hostsfile);
+ if (ctx->addnhostsfile)
+ ret2 = addnhostsDelete(ctx->addnhostsfile);
- return 0;
+ return ((ret1 == 0)&& (ret2 == 0)) ? 0 : -1;
I think the "try 2nd even if 1st fails" method *is* appropriate here though.
}
/**
diff --git a/src/util/dnsmasq.h b/src/util/dnsmasq.h
index 02a961f..3f6320a 100644
--- a/src/util/dnsmasq.h
+++ b/src/util/dnsmasq.h
@@ -44,7 +44,24 @@ typedef struct
typedef struct
{
- dnsmasqHostsfile *hostsfile;
+ unsigned int nhostnames;
+ char *ip;
+ char **hostnames;
+
+} dnsmasqAddnHost;
+
+typedef struct
+{
+ unsigned int nhosts;
+ dnsmasqAddnHost *hosts;
+
+ char *path; /* Absolute path of dnsmasq's hostsfile. */
+} dnsmasqAddnHostsfile;
+
+typedef struct
+{
+ dnsmasqHostsfile *hostsfile;
+ dnsmasqAddnHostsfile *addnhostsfile;
} dnsmasqContext;
dnsmasqContext * dnsmasqContextNew(const char *network_name,
@@ -54,6 +71,9 @@ void dnsmasqAddDhcpHost(dnsmasqContext *ctx,
const char *mac,
virSocketAddr *ip,
const char *name);
+void dnsmasqAddHost(dnsmasqContext *ctx,
+ virSocketAddr *ip,
+ const char *name);
int dnsmasqSave(const dnsmasqContext *ctx);
int dnsmasqDelete(const dnsmasqContext *ctx);
int dnsmasqReload(pid_t pid);