network: reorganize dnsmasq and radvd config file / startup

This patch splits the starting of dnsmasq and radvd into multiple
files, and adds new networkRefreshXX() and networkRestartXX()
functions for each. These new functions are currently commented out
because they won't be used until the next commit, and the compile options
require all static functions to be used.

networkRefreshXX() - rewrites any file-based config for dnsmasq/radvd,
and sends SIGHUP to the process to make it reread its config. If the
program isn't already running, it's just started.

networkRestartXX() - kills the given program, waits for it to exit
(see the comments in the function networkKillDaemon()), then calls
networkStartXX().

This commit is here mostly as a checkpoint to verify no change in
functional behavior after refactoring networkStartXX() functions to
fit in with these new functions.

1 files changed, 282 insertions, 44 deletions

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 8dbb05007..5f7f31e67 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -468,6 +468,68 @@ networkShutdown(void) {
 }
 
 
+#if 0
+/* currently unused, so it causes a build error unless we #if it out */
+/* networkKillDaemon:
+ *
+ * kill the specified pid/name, and wait a bit to make sure it's dead.
+ */
+static int
+networkKillDaemon(pid_t pid, const char *daemonName, const char *networkName)
+{
+    int ii, ret = -1;
+    const char *signame = "TERM";
+
+    /* send SIGTERM, then wait up to 3 seconds for the process to
+     * disappear, send SIGKILL, then wait for up to another 2
+     * seconds. If that fails, log a warning and continue, hoping
+     * for the best.
+     */
+    for (ii = 0; ii < 25; ii++) {
+        int signum = 0;
+        if (ii == 0)
+            signum = SIGTERM;
+        else if (ii == 15) {
+            signum = SIGKILL;
+            signame = "KILL";
+        }
+        if (kill(pid, signum) < 0) {
+            if (errno == ESRCH) {
+                ret = 0;
+            } else {
+                char ebuf[1024];
+                VIR_WARN("Failed to terminate %s process %d "
+                         "for network '%s' with SIG%s: %s",
+                         daemonName, pid, networkName, signame,
+                         virStrerror(errno, ebuf, sizeof(ebuf)));
+            }
+            goto cleanup;
+        }
+        /* NB: since networks have no reference count like
+         * domains, there is no safe way to unlock the network
+         * object temporarily, and so we can't follow the
+         * procedure used by the qemu driver of 1) unlock driver
+         * 2) sleep, 3) add ref to object 4) unlock object, 5)
+         * re-lock driver, 6) re-lock object. We may need to add
+         * that functionality eventually, but for now this
+         * function is rarely used and, at worst, leaving the
+         * network driver locked during this loop of sleeps will
+         * have the effect of holding up any other thread trying
+         * to make modifications to a network for up to 5 seconds;
+         * since modifications to networks are much less common
+         * than modifications to domains, this seems a reasonable
+         * tradeoff in exchange for less code disruption.
+         */
+        usleep(20 * 1000);
+    }
+    VIR_WARN("Timed out waiting after SIG%s to %s process %d "
+             "(network '%s')",
+             signame, daemonName, pid, networkName);
+cleanup:
+    return ret;
+}
+#endif /* #if 0 */
+
 static int
 networkBuildDnsmasqHostsfile(dnsmasqContext *dctx,
                              virNetworkIpDefPtr ipdef,
@@ -777,6 +839,12 @@ networkStartDhcpDaemon(virNetworkObjPtr network)
     int ret = -1;
     dnsmasqContext *dctx = NULL;
 
+    if (!virNetworkDefGetIpByIndex(network->def, AF_UNSPEC, 0)) {
+        /* no IPv6 addresses, so we don't need to run radvd */
+        ret = 0;
+        goto cleanup;
+    }
+
     if (virFileMakePath(NETWORK_PID_DIR) < 0) {
         virReportSystemError(errno,
                              _("cannot create directory %s"),
@@ -840,50 +908,87 @@ cleanup:
     return ret;
 }
 
+#if 0
+/* currently unused, so they cause a build error unless we #if them out */
+/* networkRefreshDhcpDaemon:
+ *  Update dnsmasq config files, then send a SIGHUP so that it rereads
+ *  them.
+ *
+ *  Returns 0 on success, -1 on failure.
+ */
 static int
-networkStartRadvd(virNetworkObjPtr network)
+networkRefreshDhcpDaemon(virNetworkObjPtr network)
 {
-    char *pidfile = NULL;
-    char *radvdpidbase = NULL;
-    virBuffer configbuf = VIR_BUFFER_INITIALIZER;;
-    char *configstr = NULL;
-    char *configfile = NULL;
-    virCommandPtr cmd = NULL;
     int ret = -1, ii;
     virNetworkIpDefPtr ipdef;
+    dnsmasqContext *dctx = NULL;
 
-    network->radvdPid = -1;
+    /* if there's no running dnsmasq, just start it */
+    if (network->dnsmasqPid <= 0 || (kill(network->dnsmasqPid, 0) < 0))
+        return networkStartDhcpDaemon(network);
 
-    if (!virFileIsExecutable(RADVD)) {
-        virReportSystemError(errno,
-                             _("Cannot find %s - "
-                               "Possibly the package isn't installed"),
-                             RADVD);
-        goto cleanup;
+    /* Look for first IPv4 address that has dhcp defined. */
+    /* We support dhcp config on 1 IPv4 interface only. */
+    for (ii = 0;
+         (ipdef = virNetworkDefGetIpByIndex(network->def, AF_INET, ii));
+         ii++) {
+        if (ipdef->nranges || ipdef->nhosts)
+            break;
     }
+    /* If no IPv4 addresses had dhcp info, pick the first (if there were any). */
+    if (!ipdef)
+        ipdef = virNetworkDefGetIpByIndex(network->def, AF_INET, 0);
 
-    if (virFileMakePath(NETWORK_PID_DIR) < 0) {
-        virReportSystemError(errno,
-                             _("cannot create directory %s"),
-                             NETWORK_PID_DIR);
-        goto cleanup;
-    }
-    if (virFileMakePath(RADVD_STATE_DIR) < 0) {
-        virReportSystemError(errno,
-                             _("cannot create directory %s"),
-                             RADVD_STATE_DIR);
-        goto cleanup;
+    if (!ipdef) {
+        /* no <ip> elements, so nothing to do */
+        return 0;
     }
 
-    /* construct pidfile name */
-    if (!(radvdpidbase = networkRadvdPidfileBasename(network->def->name))) {
-        virReportOOMError();
+    if (!(dctx = dnsmasqContextNew(network->def->name, DNSMASQ_STATE_DIR)))
         goto cleanup;
-    }
-    if (!(pidfile = virPidFileBuildPath(NETWORK_PID_DIR, radvdpidbase))) {
-        virReportOOMError();
+
+    if (networkBuildDnsmasqHostsfile(dctx, ipdef, network->def->dns) < 0)
+       goto cleanup;
+
+    if ((ret = dnsmasqSave(dctx)) < 0)
         goto cleanup;
+
+    ret = kill(network->dnsmasqPid, SIGHUP);
+cleanup:
+    dnsmasqContextFree(dctx);
+    return ret;
+}
+
+/* networkRestartDhcpDaemon:
+ *
+ * kill and restart dnsmasq, in order to update any config that is on
+ * the dnsmasq commandline (and any placed in separate config files).
+ *
+ *  Returns 0 on success, -1 on failure.
+ */
+static int
+networkRestartDhcpDaemon(virNetworkObjPtr network)
+{
+    /* if there is a running dnsmasq, kill it */
+    if (network->dnsmasqPid > 0) {
+        networkKillDaemon(network->dnsmasqPid, "dnsmasq",
+                          network->def->name);
+        network->dnsmasqPid = -1;
     }
+    /* now start dnsmasq if it should be started */
+    return networkStartDhcpDaemon(network);
+}
+#endif /* #if 0 */
+
+static int
+networkRadvdConfContents(virNetworkObjPtr network, char **configstr)
+{
+    virBuffer configbuf = VIR_BUFFER_INITIALIZER;;
+    int ret = -1, ii;
+    virNetworkIpDefPtr ipdef;
+    bool v6present = false;
+
+    *configstr = NULL;
 
     /* create radvd config file appropriate for this network */
     virBufferAsprintf(&configbuf, "interface %s\n"
@@ -893,12 +998,15 @@ networkStartRadvd(virNetworkObjPtr network)
                       "  AdvOtherConfigFlag off;\n"
                       "\n",
                       network->def->bridge);
+
+    /* add a section for each IPv6 address in the config */
     for (ii = 0;
          (ipdef = virNetworkDefGetIpByIndex(network->def, AF_INET6, ii));
          ii++) {
         int prefix;
         char *netaddr;
 
+        v6present = true;
         prefix = virNetworkIpDefPrefix(ipdef);
         if (prefix < 0) {
             virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -919,30 +1027,118 @@ networkStartRadvd(virNetworkObjPtr network)
         VIR_FREE(netaddr);
     }
 
-    virBufferAddLit(&configbuf, "};\n");
+    /* only create the string if we found at least one IPv6 address */
+    if (v6present) {
+        virBufferAddLit(&configbuf, "};\n");
 
-    if (virBufferError(&configbuf)) {
-        virReportOOMError();
-        goto cleanup;
+        if (virBufferError(&configbuf)) {
+            virReportOOMError();
+            goto cleanup;
+        }
+        if (!(*configstr = virBufferContentAndReset(&configbuf))) {
+            virReportOOMError();
+            goto cleanup;
+        }
     }
-    if (!(configstr = virBufferContentAndReset(&configbuf))) {
-        virReportOOMError();
+
+    ret = 0;
+cleanup:
+    virBufferFreeAndReset(&configbuf);
+    return ret;
+}
+
+/* write file and return it's name (which must be freed by caller) */
+static int
+networkRadvdConfWrite(virNetworkObjPtr network, char **configFile)
+{
+    int ret = -1;
+    char *configStr = NULL;
+    char *myConfigFile = NULL;
+
+    if (!configFile)
+        configFile = &myConfigFile;
+
+    *configFile = NULL;
+
+    if (networkRadvdConfContents(network, &configStr) < 0)
+        goto cleanup;
+
+    if (!configStr) {
+        ret = 0;
         goto cleanup;
     }
 
     /* construct the filename */
-    if (!(configfile = networkRadvdConfigFileName(network->def->name))) {
+    if (!(*configFile = networkRadvdConfigFileName(network->def->name))) {
         virReportOOMError();
         goto cleanup;
     }
     /* write the file */
-    if (virFileWriteStr(configfile, configstr, 0600) < 0) {
+    if (virFileWriteStr(*configFile, configStr, 0600) < 0) {
         virReportSystemError(errno,
                              _("couldn't write radvd config file '%s'"),
-                             configfile);
+                             *configFile);
+        goto cleanup;
+    }
+
+    ret = 0;
+cleanup:
+    VIR_FREE(configStr);
+    VIR_FREE(myConfigFile);
+    return ret;
+}
+
+static int
+networkStartRadvd(virNetworkObjPtr network)
+{
+    char *pidfile = NULL;
+    char *radvdpidbase = NULL;
+    char *configfile = NULL;
+    virCommandPtr cmd = NULL;
+    int ret = -1;
+
+    network->radvdPid = -1;
+
+    if (!virNetworkDefGetIpByIndex(network->def, AF_INET6, 0)) {
+        /* no IPv6 addresses, so we don't need to run radvd */
+        ret = 0;
+        goto cleanup;
+    }
+
+    if (!virFileIsExecutable(RADVD)) {
+        virReportSystemError(errno,
+                             _("Cannot find %s - "
+                               "Possibly the package isn't installed"),
+                             RADVD);
         goto cleanup;
     }
 
+    if (virFileMakePath(NETWORK_PID_DIR) < 0) {
+        virReportSystemError(errno,
+                             _("cannot create directory %s"),
+                             NETWORK_PID_DIR);
+        goto cleanup;
+    }
+    if (virFileMakePath(RADVD_STATE_DIR) < 0) {
+        virReportSystemError(errno,
+                             _("cannot create directory %s"),
+                             RADVD_STATE_DIR);
+        goto cleanup;
+    }
+
+    /* construct pidfile name */
+    if (!(radvdpidbase = networkRadvdPidfileBasename(network->def->name))) {
+        virReportOOMError();
+        goto cleanup;
+    }
+    if (!(pidfile = virPidFileBuildPath(NETWORK_PID_DIR, radvdpidbase))) {
+        virReportOOMError();
+        goto cleanup;
+    }
+
+    if (networkRadvdConfWrite(network, &configfile) < 0)
+        goto cleanup;
+
     /* prevent radvd from daemonizing itself with "--debug 1", and use
      * a dummy pidfile name - virCommand will create the pidfile we
      * want to use (this is necessary because radvd's internal
@@ -963,21 +1159,63 @@ networkStartRadvd(virNetworkObjPtr network)
     if (virCommandRun(cmd, NULL) < 0)
         goto cleanup;
 
-    if (virPidFileRead(NETWORK_PID_DIR, radvdpidbase,
-                       &network->radvdPid) < 0)
+    if (virPidFileRead(NETWORK_PID_DIR, radvdpidbase, &network->radvdPid) < 0)
         goto cleanup;
 
     ret = 0;
 cleanup:
     virCommandFree(cmd);
     VIR_FREE(configfile);
-    VIR_FREE(configstr);
-    virBufferFreeAndReset(&configbuf);
     VIR_FREE(radvdpidbase);
     VIR_FREE(pidfile);
     return ret;
 }
 
+#if 0
+/* currently unused, so they cause a build error unless we #if them out */
+static int
+networkRefreshRadvd(virNetworkObjPtr network)
+{
+    /* if there's no running radvd, just start it */
+    if (network->radvdPid <= 0 || (kill(network->radvdPid, 0) < 0))
+        return networkStartRadvd(network);
+
+    if (!virNetworkDefGetIpByIndex(network->def, AF_INET6, 0)) {
+        /* no IPv6 addresses, so we don't need to run radvd */
+        return 0;
+    }
+
+    if (networkRadvdConfWrite(network, NULL) < 0)
+        return -1;
+
+    return kill(network->radvdPid, SIGHUP);
+}
+
+static int
+networkRestartRadvd(virNetworkObjPtr network)
+{
+    char *radvdpidbase;
+
+    /* if there is a running radvd, kill it */
+    if (network->radvdPid > 0) {
+        /* essentially ignore errors from the following two functions,
+         * since there's really no better recovery to be done than to
+         * just push ahead (and that may be exactly what's needed).
+         */
+        if ((networkKillDaemon(network->dnsmasqPid, "radvd",
+                               network->def->name) >= 0) &&
+            ((radvdpidbase = networkRadvdPidfileBasename(network->def->name))
+             != NULL)) {
+            virPidFileDelete(NETWORK_PID_DIR, radvdpidbase);
+            VIR_FREE(radvdpidbase);
+        }
+        network->radvdPid = -1;
+    }
+    /* now start radvd if it should be started */
+    return networkStartRadvd(network);
+}
+#endif /* #if 0 */
+
 static int
 networkAddMasqueradingIptablesRules(struct network_driver *driver,
                                     virNetworkObjPtr network,