On Wed, Jan 14, 2009 at 08:55:46AM +0000, Mark McLoughlin wrote:
> On Tue, 2009-01-13 at 20:49 +0000, Daniel P. Berrange wrote:
> > Currently when we shutdown the virtual networks are all shutdown too.
> > This is less than useful if we're letting guest VMs hang around post
> > shutdown of libvirtd, because it means we're tearing their network
> > connection out from under them. This patch fixes that allowing networks
> > to survive restarts, and be re-detected next time around.
> >
> > When starting a virtual network we write the live config into
> >
> > /var/lib/libvirt/network/$NAME.xml
> >
> > This is because the bridge device name is potentially auto-generated
> > and we need to keep track of that
> >
> > We change dnsmasq args to include an explicit pidfile location
> >
> > /var/run/libvirt/network/$NAME.pid
> >
> > and also tell it to put itself into the background - ie daemonize. This
> > is because we want dnsmasq to survive the daemon.
> >
> > Now, when libvirtd starts up it
> >
> > - Looks for the live config, and if found loads it.
> > - Calls a new method brHasBridge() to see if its desired bridge
> > actually exists (and thus whether the network is running).
> > If it exists,the network is marked active
> > - If DHCP is configured, then reads the dnsmasq PIDfile, and sends
> > kill($PID, 0) to check if the process is actually alive
> >
> > In addition I cleanup the network code to remove the configFile and
> > autostartLink fields in virNetworkObjPtr, so it matches virDomaiObjPtr
> > usage.
> >
> > With all this applied you can now restart the daemon, and virbr0 is
> > left happily running.
>
> It all sounds and looks good to me. Some comments below, but nothing
> major.
Here's an updated patch with all your comments incorporated.
libvirt.spec.in | 31 ++++-
src/Makefile.am | 24 +++-
src/bridge.c | 37 ++++++
src/bridge.h | 2
src/libvirt_bridge.syms | 1
src/libvirt_private.syms | 2
src/network_conf.c | 130 +++++++++++++----------
src/network_conf.h | 18 ++-
src/network_driver.c | 260 +++++++++++++++++++++++++++++++++++------------
9 files changed, 370 insertions(+), 135 deletions(-)
Daniel
diff --git a/libvirt.spec.in b/libvirt.spec.in
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -11,6 +11,7 @@
%define with_python 0%{!?_without_python:1}
%define with_libvirtd 0%{!?_without_libvirtd:1}
%define with_uml 0%{!?_without_uml:1}
+%define with_network 0%{!?_without_network:1}
# Xen is available only on i386 x86_64 ia64
%ifnarch i386 i686 x86_64 ia64
@@ -207,6 +208,10 @@ of recent versions of Linux (and other O
%define _without_uml --without-uml
%endif
+%if ! %{with_network}
+%define _without_network --without-network
+%endif
+
%configure %{?_without_xen} \
%{?_without_qemu} \
%{?_without_openvz} \
@@ -217,6 +222,7 @@ of recent versions of Linux (and other O
%{?_without_python} \
%{?_without_libvirtd} \
%{?_without_uml} \
+ %{?_without_network} \
--with-init-script=redhat \
--with-qemud-pid-file=%{_localstatedir}/run/libvirt_qemud.pid \
--with-remote-file=%{_localstatedir}/run/libvirtd.pid
@@ -232,11 +238,6 @@ rm -f $RPM_BUILD_ROOT%{_libdir}/*.la
rm -f $RPM_BUILD_ROOT%{_libdir}/*.a
rm -f $RPM_BUILD_ROOT%{_libdir}/python*/site-packages/*.la
rm -f $RPM_BUILD_ROOT%{_libdir}/python*/site-packages/*.a
-install -d -m 0755 $RPM_BUILD_ROOT%{_localstatedir}/run/libvirt/
-# Default dir for disk images defined in SELinux policy
-install -d -m 0755 $RPM_BUILD_ROOT%{_localstatedir}/lib/libvirt/images/
-# Default dir for kernel+initrd images defnied in SELinux policy
-install -d -m 0755 $RPM_BUILD_ROOT%{_localstatedir}/lib/libvirt/boot/
%if %{with_qemu}
# We don't want to install /etc/libvirt/qemu/networks in the main %files list
@@ -338,11 +339,31 @@ fi
%endif
%dir %{_localstatedir}/run/libvirt/
+
%dir %{_localstatedir}/lib/libvirt/
%dir %attr(0700, root, root) %{_localstatedir}/lib/libvirt/images/
%dir %attr(0700, root, root) %{_localstatedir}/lib/libvirt/boot/
%if %{with_qemu}
+%dir %{_localstatedir}/run/libvirt/qemu/
+%dir %attr(0700, root, root) %{_localstatedir}/lib/libvirt/qemu/
+%endif
+%if %{with_lxc}
+%dir %{_localstatedir}/run/libvirt/lxc/
+%dir %attr(0700, root, root) %{_localstatedir}/lib/libvirt/lxc/
+%endif
+%if %{with_uml}
+%dir %{_localstatedir}/run/libvirt/uml/
+%dir %attr(0700, root, root) %{_localstatedir}/lib/libvirt/uml/
+%endif
+%if %{with_network}
+%dir %{_localstatedir}/run/libvirt/network/
+%dir %attr(0700, root, root) %{_localstatedir}/lib/libvirt/network/
+%dir %attr(0700, root, root) %{_localstatedir}/lib/libvirt/iptables/filter/
+%dir %attr(0700, root, root) %{_localstatedir}/lib/libvirt/iptables/nat/
+%endif
+
+%if %{with_qemu}
%{_datadir}/augeas/lenses/libvirtd_qemu.aug
%{_datadir}/augeas/lenses/tests/test_libvirtd_qemu.aug
%endif
diff --git a/src/Makefile.am b/src/Makefile.am
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -589,9 +589,29 @@ endif
endif
EXTRA_DIST += $(LXC_CONTROLLER_SOURCES)
-# Create the /var/cache/libvirt directory when installing.
install-exec-local:
- $(MKDIR_P) $(DESTDIR)$(localstatedir)/cache/libvirt
+ $(MKDIR_P) "$(DESTDIR)$(localstatedir)/cache/libvirt"
+ $(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/images"
+ $(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/boot"
+if WITH_QEMU
+ $(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/qemu"
+ $(MKDIR_P) "$(DESTDIR)$(localstatedir)/run/libvirt/qemu"
+endif
+if WITH_LXC
+ $(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/lxc"
+ $(MKDIR_P) "$(DESTDIR)$(localstatedir)/run/libvirt/lxc"
+endif
+if WITH_UML
+ $(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/uml"
+ $(MKDIR_P) "$(DESTDIR)$(localstatedir)/run/libvirt/uml"
+endif
+if WITH_NETWORK
+ $(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/iptables/filter"
+ $(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/iptables/nat"
+ $(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/network"
+ $(MKDIR_P) "$(DESTDIR)$(localstatedir)/run/libvirt/network"
+endif
+
CLEANFILES = *.gcov .libs/*.gcda .libs/*.gcno *.gcno *.gcda
DISTCLEANFILES = $(BUILT_SOURCES)
diff --git a/src/bridge.c b/src/bridge.c
--- a/src/bridge.c
+++ b/src/bridge.c
@@ -163,6 +163,43 @@ int brAddBridge (brControl *ctl ATTRIBUT
}
#endif
+#ifdef SIOCBRDELBR
+int
+brHasBridge(brControl *ctl,
+ const char *name)
+{
+ struct ifreq ifr;
+ int len;
+
+ if (!ctl || !name) {
+ errno = EINVAL;
+ return -1;
+ }
+
+ if ((len = strlen(name)) >= BR_IFNAME_MAXLEN) {
+ errno = EINVAL;
+ return -1;
+ }
+
+ memset(&ifr, 0, sizeof(struct ifreq));
+
+ strncpy(ifr.ifr_name, name, len);
+ ifr.ifr_name[len] = '\0';
+
+ if (ioctl(ctl->fd, SIOCGIFFLAGS, &ifr))
+ return -1;
+
+ return 0;
+}
+#else
+int
+brHasBridge(brControl *ctl,
+ const char *name)
+{
+ return EINVAL;
+}
+#endif
+
/**
* brDeleteBridge:
* @ctl: bridge control pointer
diff --git a/src/bridge.h b/src/bridge.h
--- a/src/bridge.h
+++ b/src/bridge.h
@@ -50,6 +50,8 @@ int brAddBridge (brContr
char **name);
int brDeleteBridge (brControl *ctl,
const char *name);
+int brHasBridge (brControl *ctl,
+ const char *name);
int brAddInterface (brControl *ctl,
const char *bridge,
diff --git a/src/libvirt_bridge.syms b/src/libvirt_bridge.syms
--- a/src/libvirt_bridge.syms
+++ b/src/libvirt_bridge.syms
@@ -9,6 +9,7 @@ brAddBridge;
brAddInterface;
brAddTap;
brDeleteBridge;
+brHasBridge;
brInit;
brSetEnableSTP;
brSetForwardDelay;
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -190,6 +190,7 @@ virFree;
# network_conf.h
virNetworkAssignDef;
+virNetworkConfigFile;
virNetworkDefFormat;
virNetworkDefFree;
virNetworkDefParseFile;
@@ -202,6 +203,7 @@ virNetworkLoadAllConfigs;
virNetworkObjListFree;
virNetworkDefParseNode;
virNetworkRemoveInactive;
+virNetworkSaveConfigXML;
virNetworkSaveConfig;
virNetworkObjLock;
virNetworkObjUnlock;
diff --git a/src/network_conf.c b/src/network_conf.c
--- a/src/network_conf.c
+++ b/src/network_conf.c
@@ -125,9 +125,6 @@ void virNetworkObjFree(virNetworkObjPtr
virNetworkDefFree(net->def);
virNetworkDefFree(net->newDef);
- VIR_FREE(net->configFile);
- VIR_FREE(net->autostartLink);
-
virMutexDestroy(&net->lock);
VIR_FREE(net);
@@ -641,31 +638,17 @@ char *virNetworkDefFormat(virConnectPtr
return NULL;
}
-int virNetworkSaveConfig(virConnectPtr conn,
- const char *configDir,
- const char *autostartDir,
- virNetworkObjPtr net)
+int virNetworkSaveXML(virConnectPtr conn,
+ const char *configDir,
+ virNetworkDefPtr def,
+ const char *xml)
{
- char *xml;
+ char *configFile = NULL;
int fd = -1, ret = -1;
size_t towrite;
int err;
- if (!net->configFile &&
- virAsprintf(&net->configFile, "%s/%s.xml",
- configDir, net->def->name) < 0) {
- virNetworkReportError(conn, VIR_ERR_NO_MEMORY, NULL);
- goto cleanup;
- }
- if (!net->autostartLink &&
- virAsprintf(&net->autostartLink, "%s/%s.xml",
- autostartDir, net->def->name) < 0) {
- virNetworkReportError(conn, VIR_ERR_NO_MEMORY, NULL);
- goto cleanup;
- }
-
- if (!(xml = virNetworkDefFormat(conn,
- net->newDef ? net->newDef : net->def)))
+ if ((configFile = virNetworkConfigFile(conn, configDir, def->name)) == NULL)
goto cleanup;
if ((err = virFileMakePath(configDir))) {
@@ -675,19 +658,12 @@ int virNetworkSaveConfig(virConnectPtr c
goto cleanup;
}
- if ((err = virFileMakePath(autostartDir))) {
- virReportSystemError(conn, err,
- _("cannot create autostart directory '%s'"),
- autostartDir);
- goto cleanup;
- }
-
- if ((fd = open(net->configFile,
+ if ((fd = open(configFile,
O_WRONLY | O_CREAT | O_TRUNC,
S_IRUSR | S_IWUSR )) < 0) {
virReportSystemError(conn, errno,
_("cannot create config file '%s'"),
- net->configFile);
+ configFile);
goto cleanup;
}
@@ -695,48 +671,63 @@ int virNetworkSaveConfig(virConnectPtr c
if (safewrite(fd, xml, towrite) < 0) {
virReportSystemError(conn, errno,
_("cannot write config file '%s'"),
- net->configFile);
+ configFile);
goto cleanup;
}
if (close(fd) < 0) {
virReportSystemError(conn, errno,
_("cannot save config file '%s'"),
- net->configFile);
+ configFile);
goto cleanup;
}
ret = 0;
cleanup:
- VIR_FREE(xml);
if (fd != -1)
close(fd);
+ VIR_FREE(configFile);
+
return ret;
}
+int virNetworkSaveConfig(virConnectPtr conn,
+ const char *configDir,
+ virNetworkDefPtr def)
+{
+ int ret = -1;
+ char *xml;
+
+ if (!(xml = virNetworkDefFormat(conn, def)))
+ goto cleanup;
+
+ if (virNetworkSaveXML(conn, configDir, def, xml))
+ goto cleanup;
+
+ ret = 0;
+cleanup:
+ VIR_FREE(xml);
+ return ret;
+}
+
+
virNetworkObjPtr virNetworkLoadConfig(virConnectPtr conn,
virNetworkObjListPtr nets,
const char *configDir,
const char *autostartDir,
- const char *file)
+ const char *name)
{
char *configFile = NULL, *autostartLink = NULL;
virNetworkDefPtr def = NULL;
virNetworkObjPtr net;
int autostart;
- if (virAsprintf(&configFile, "%s/%s",
- configDir, file) < 0) {
- virNetworkReportError(conn, VIR_ERR_NO_MEMORY, NULL);
+ if ((configFile = virNetworkConfigFile(conn, configDir, name)) == NULL)
goto error;
- }
- if (virAsprintf(&autostartLink, "%s/%s",
- autostartDir, file) < 0) {
- virNetworkReportError(conn, VIR_ERR_NO_MEMORY, NULL);
+ if ((autostartLink = virNetworkConfigFile(conn, autostartDir, name)) == NULL)
goto error;
- }
if ((autostart = virFileLinkPointsTo(autostartLink, configFile)) < 0)
goto error;
@@ -744,7 +735,7 @@ virNetworkObjPtr virNetworkLoadConfig(vi
if (!(def = virNetworkDefParseFile(conn, configFile)))
goto error;
- if (!virFileMatchesNameSuffix(file, def->name, ".xml")) {
+ if (!STREQ(name, def->name)) {
virNetworkReportError(conn, VIR_ERR_INTERNAL_ERROR,
_("Network config filename '%s'"
" does not match network name '%s'"),
@@ -755,10 +746,11 @@ virNetworkObjPtr virNetworkLoadConfig(vi
if (!(net = virNetworkAssignDef(conn, nets, def)))
goto error;
- net->configFile = configFile;
- net->autostartLink = autostartLink;
net->autostart = autostart;
+ VIR_FREE(configFile);
+ VIR_FREE(autostartLink);
+
return net;
error:
@@ -791,7 +783,7 @@ int virNetworkLoadAllConfigs(virConnectP
if (entry->d_name[0] == '.')
continue;
- if (!virFileHasSuffix(entry->d_name, ".xml"))
+ if (!virFileStripSuffix(entry->d_name, ".xml"))
continue;
/* NB: ignoring errors, so one malformed config doesn't
@@ -811,27 +803,51 @@ int virNetworkLoadAllConfigs(virConnectP
}
int virNetworkDeleteConfig(virConnectPtr conn,
+ const char *configDir,
+ const char *autostartDir,
virNetworkObjPtr net)
{
- if (!net->configFile || !net->autostartLink) {
- virNetworkReportError(conn, VIR_ERR_INTERNAL_ERROR,
- _("no config file for %s"), net->def->name);
- return -1;
- }
+ char *configFile = NULL;
+ char *autostartLink = NULL;
+
+ if ((configFile = virNetworkConfigFile(conn, configDir, net->def->name)) == NULL)
+ goto error;
+ if ((autostartLink = virNetworkConfigFile(conn, autostartDir, net->def->name)) == NULL)
+ goto error;
/* Not fatal if this doesn't work */
- unlink(net->autostartLink);
+ unlink(autostartLink);
- if (unlink(net->configFile) < 0) {
+ if (unlink(configFile) < 0) {
virReportSystemError(conn, errno,
_("cannot remove config file '%s'"),
- net->configFile);
- return -1;
+ configFile);
+ goto error;
}
return 0;
+
+error:
+ VIR_FREE(configFile);
+ VIR_FREE(autostartLink);
+ return -1;
}
+char *virNetworkConfigFile(virConnectPtr conn,
+ const char *dir,
+ const char *name)
+{
+ char *ret = NULL;
+
+ if (virAsprintf(&ret, "%s/%s.xml", dir, name) < 0) {
+ virNetworkReportError(conn, VIR_ERR_NO_MEMORY, NULL);
+ return NULL;
+ }
+
+ return ret;
+}
+
+
void virNetworkObjLock(virNetworkObjPtr obj)
{
virMutexLock(&obj->lock);
diff --git a/src/network_conf.h b/src/network_conf.h
--- a/src/network_conf.h
+++ b/src/network_conf.h
@@ -90,9 +90,6 @@ struct _virNetworkObj {
unsigned int autostart : 1;
unsigned int persistent : 1;
- char *configFile; /* Persistent config file path */
- char *autostartLink; /* Symlink path for autostart */
-
virNetworkDefPtr def; /* The current definition */
virNetworkDefPtr newDef; /* New definition to activate at shutdown */
};
@@ -139,10 +136,14 @@ char *virNetworkDefFormat(virConnectPtr
const virNetworkDefPtr def);
+int virNetworkSaveXML(virConnectPtr conn,
+ const char *configDir,
+ virNetworkDefPtr def,
+ const char *xml);
+
int virNetworkSaveConfig(virConnectPtr conn,
const char *configDir,
- const char *autostartDir,
- virNetworkObjPtr net);
+ virNetworkDefPtr def);
virNetworkObjPtr virNetworkLoadConfig(virConnectPtr conn,
virNetworkObjListPtr nets,
@@ -156,8 +157,15 @@ int virNetworkLoadAllConfigs(virConnectP
const char *autostartDir);
int virNetworkDeleteConfig(virConnectPtr conn,
+ const char *configDir,
+ const char *autostartDir,
virNetworkObjPtr net);
+char *virNetworkConfigFile(virConnectPtr conn,
+ const char *dir,
+ const char *name);
+
+
void virNetworkObjLock(virNetworkObjPtr obj);
void virNetworkObjUnlock(virNetworkObjPtr obj);
diff --git a/src/network_driver.c b/src/network_driver.c
--- a/src/network_driver.c
+++ b/src/network_driver.c
@@ -57,6 +57,8 @@
#include "iptables.h"
#include "bridge.h"
+#define NETWORK_PID_DIR LOCAL_STATE_DIR "/run/libvirt/network"
+#define NETWORK_STATE_DIR LOCAL_STATE_DIR "/lib/libvirt/network"
#define VIR_FROM_THIS VIR_FROM_NETWORK
@@ -106,6 +108,64 @@ static struct network_driver *driverStat
static void
+networkFindActiveConfigs(struct network_driver *driver) {
+ unsigned int i;
+
+ for (i = 0 ; i < driver->networks.count ; i++) {
+ virNetworkObjPtr obj = driver->networks.objs[i];
+ virNetworkDefPtr tmp;
+ char *config;
+
+ virNetworkObjLock(obj);
+
+ if ((config = virNetworkConfigFile(NULL,
+ NETWORK_STATE_DIR,
+ obj->def->name)) == NULL) {
+ virNetworkObjUnlock(obj);
+ continue;
+ }
+
+ if (access(config, R_OK) < 0) {
+ VIR_FREE(config);
+ virNetworkObjUnlock(obj);
+ continue;
+ }
+
+ /* Try and load the live config */
+ tmp = virNetworkDefParseFile(NULL, config);
+ VIR_FREE(config);
+ if (tmp) {
+ obj->newDef = obj->def;
+ obj->def = tmp;
+ }
+
+ /* If bridge exists, then mark it active */
+ if (obj->def->bridge &&
+ brHasBridge(driver->brctl, obj->def->bridge) == 0) {
+ obj->active = 1;
+
+ /* Finally try and read dnsmasq pid if any DHCP ranges are set */
+ if (obj->def->nranges &&
+ virFileReadPid(NETWORK_PID_DIR, obj->def->name,
+ &obj->dnsmasqPid) == 0) {
+
+ /* Check its still alive */
+ if (kill(obj->dnsmasqPid, 0) != 0)
+ obj->dnsmasqPid = -1;
+
+ /* XXX ideally we'd check this was actually
+ * the dnsmasq process, not a stale pid file
+ * with someone else's process. But how ?
+ */
+ }
+ }
+
+ virNetworkObjUnlock(obj);
+ }
+}
+
+
+static void
networkAutostartConfigs(struct network_driver *driver) {
unsigned int i;
@@ -132,6 +192,7 @@ static int
networkStartup(void) {
uid_t uid = geteuid();
char *base = NULL;
+ int err;
if (VIR_ALLOC(driverState) < 0)
goto error;
@@ -181,12 +242,26 @@ networkStartup(void) {
VIR_FREE(base);
+ if ((err = brInit(&driverState->brctl))) {
+ virReportSystemError(NULL, err, "%s",
+ _("cannot initialize bridge support"));
+ goto error;
+ }
+
+ if (!(driverState->iptables = iptablesContextNew())) {
+ networkReportError(NULL, NULL, NULL, VIR_ERR_NO_MEMORY,
+ "%s", _("failed to allocate space for IP tables support"));
+ goto error;
+ }
+
+
if (virNetworkLoadAllConfigs(NULL,
&driverState->networks,
driverState->networkConfigDir,
driverState->networkAutostartDir) < 0)
goto error;
+ networkFindActiveConfigs(driverState);
networkAutostartConfigs(driverState);
networkDriverUnlock(driverState);
@@ -269,23 +344,11 @@ networkActive(void) {
*/
static int
networkShutdown(void) {
- unsigned int i;
-
if (!driverState)
return -1;
networkDriverLock(driverState);
- /* shutdown active networks */
- for (i = 0 ; i < driverState->networks.count ; i++) {
- virNetworkObjPtr net = driverState->networks.objs[i];
- virNetworkObjLock(net);
- if (virNetworkIsActive(net))
- networkShutdownNetworkDaemon(NULL, driverState,
- driverState->networks.objs[i]);
- virNetworkObjUnlock(net);
- }
-
/* free inactive networks */
virNetworkObjListFree(&driverState->networks);
@@ -309,23 +372,42 @@ networkShutdown(void) {
static int
networkBuildDnsmasqArgv(virConnectPtr conn,
- virNetworkObjPtr network,
- const char ***argv) {
+ virNetworkObjPtr network,
+ const char *pidfile,
+ const char ***argv) {
int i, len, r;
- char buf[PATH_MAX];
+ char *pidfileArg;
+ char buf[1024];
+
+ /*
+ * NB, be careful about syntax for dnsmasq options in long format
+ *
+ * If the flag has a mandatory argument, it can be given using
+ * either syntax:
+ *
+ * --foo bar
+ * --foo=bar
+ *
+ * If the flag has a optional argument, it *must* be given using
+ * the syntax:
+ *
+ * --foo=bar
+ *
+ * It is hard to determine whether a flag is optional or not,
+ * without reading the dnsmasq source :-( The manpages is not
+ * very explicit on this
+ */
len =
1 + /* dnsmasq */
- 1 + /* --keep-in-foreground */
1 + /* --strict-order */
1 + /* --bind-interfaces */
(network->def->domain?2:0) + /* --domain name */
- 2 + /* --pid-file "" */
+ 2 + /* --pid-file /var/run/libvirt/network/$NAME.pid */
2 + /* --conf-file "" */
/*2 + *//* --interface virbr0 */
2 + /* --except-interface lo */
2 + /* --listen-address 10.0.0.1 */
- 1 + /* --dhcp-leasefile=path */
(2 * network->def->nranges) + /* --dhcp-range 10.0.0.2,10.0.0.254 */
/* --dhcp-host 01:23:45:67:89:0a,hostname,10.0.0.3 */
(2 * network->def->nhosts) +
@@ -339,11 +421,13 @@ networkBuildDnsmasqArgv(virConnectPtr co
goto no_memory; \
} while (0)
+#define APPEND_ARG_LIT(v, n, s) \
+ (v)[(n)] = s
+
i = 0;
APPEND_ARG(*argv, i++, DNSMASQ);
- APPEND_ARG(*argv, i++, "--keep-in-foreground");
/*
* Needed to ensure dnsmasq uses same algorithm for processing
* multiple namedriver entries in /etc/resolv.conf as GLibC.
@@ -356,10 +440,11 @@ networkBuildDnsmasqArgv(virConnectPtr co
APPEND_ARG(*argv, i++, network->def->domain);
}
- APPEND_ARG(*argv, i++, "--pid-file");
- APPEND_ARG(*argv, i++, "");
+ if (virAsprintf(&pidfileArg, "--pid-file=%s", pidfile) < 0)
+ goto no_memory;
+ APPEND_ARG_LIT(*argv, i++, pidfileArg);
- APPEND_ARG(*argv, i++, "--conf-file");
+ APPEND_ARG(*argv, i++, "--conf-file=");
APPEND_ARG(*argv, i++, "");
/*
@@ -377,15 +462,6 @@ networkBuildDnsmasqArgv(virConnectPtr co
APPEND_ARG(*argv, i++, "--except-interface");
APPEND_ARG(*argv, i++, "lo");
- /*
- * NB, dnsmasq command line arg bug means we need to
- * use a single arg '--dhcp-leasefile=path' rather than
- * two separate args in '--dhcp-leasefile path' style
- */
- snprintf(buf, sizeof(buf), "--dhcp-leasefile=%s/lib/libvirt/dhcp-%s.leases",
- LOCAL_STATE_DIR, network->def->name);
- APPEND_ARG(*argv, i++, buf);
-
for (r = 0 ; r < network->def->nranges ; r++) {
snprintf(buf, sizeof(buf), "%s,%s",
network->def->ranges[r].start,
@@ -434,7 +510,10 @@ dhcpStartDhcpDaemon(virConnectPtr conn,
virNetworkObjPtr network)
{
const char **argv;
- int ret, i;
+ char *pidfile;
+ int ret = -1, i, err;
+
+ network->dnsmasqPid = -1;
if (network->def->ipAddress == NULL) {
networkReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
@@ -442,13 +521,49 @@ dhcpStartDhcpDaemon(virConnectPtr conn,
return -1;
}
+ if ((err = virFileMakePath(NETWORK_PID_DIR)) < 0) {
+ virReportSystemError(conn, err,
+ _("cannot create directory %s"),
+ NETWORK_PID_DIR);
+ return -1;
+ }
+ if ((err = virFileMakePath(NETWORK_STATE_DIR)) < 0) {
+ virReportSystemError(conn, err,
+ _("cannot create directory %s"),
+ NETWORK_STATE_DIR);
+ return -1;
+ }
+
+ if (!(pidfile = virFilePid(NETWORK_PID_DIR, network->def->name))) {
+ virReportOOMError(conn);
+ return -1;
+ }
+
argv = NULL;
- if (networkBuildDnsmasqArgv(conn, network, &argv) < 0)
+ if (networkBuildDnsmasqArgv(conn, network, pidfile, &argv) < 0) {
+ VIR_FREE(pidfile);
return -1;
+ }
- ret = virExec(conn, argv, NULL, NULL,
- &network->dnsmasqPid, -1, NULL, NULL, VIR_EXEC_NONBLOCK);
+ if (virRun(conn, argv, NULL) < 0)
+ goto cleanup;
+ /*
+ * There really is no race here - when dnsmasq daemonizes,
+ * its leader process stays around until its child has
+ * actually written its pidfile. So by time virRun exits
+ * it has waitpid'd and guaranteed the proess has started
+ * and written a pid
+ */
+
+ if (virFileReadPid(NETWORK_PID_DIR, network->def->name,
+ &network->dnsmasqPid) < 0)
+ goto cleanup;
+
+ ret = 0;
+
+cleanup:
+ VIR_FREE(pidfile);
for (i = 0; argv[i]; i++)
VIR_FREE(argv[i]);
VIR_FREE(argv);
@@ -554,13 +669,6 @@ networkAddIptablesRules(virConnectPtr co
virNetworkObjPtr network) {
int err;
- if (!driver->iptables && !(driver->iptables = iptablesContextNew())) {
- networkReportError(conn, NULL, NULL, VIR_ERR_NO_MEMORY,
- "%s", _("failed to allocate space for IP tables support"));
- return 0;
- }
-
-
/* allow DHCP requests through to dnsmasq */
if ((err = iptablesAddTcpInput(driver->iptables, network->def->bridge, 67))) {
virReportSystemError(conn, err,
@@ -716,12 +824,6 @@ static int networkStartNetworkDaemon(vir
return -1;
}
- if (!driver->brctl && (err = brInit(&driver->brctl))) {
- virReportSystemError(conn, err, "%s",
- _("cannot initialize bridge support"));
- return -1;
- }
-
if ((err = brAddBridge(driver->brctl, &network->def->bridge))) {
virReportSystemError(conn, err,
_("cannot create bridge '%s'"),
@@ -729,7 +831,6 @@ static int networkStartNetworkDaemon(vir
return -1;
}
-
if (brSetForwardDelay(driver->brctl, network->def->bridge, network->def->delay) < 0)
goto err_delbr;
@@ -774,10 +875,22 @@ static int networkStartNetworkDaemon(vir
dhcpStartDhcpDaemon(conn, network) < 0)
goto err_delbr2;
+
+ /* Persist the live configuration now we have bridge info */
+ if (virNetworkSaveConfig(conn, NETWORK_STATE_DIR, network->def) < 0) {
+ goto err_kill;
+ }
+
network->active = 1;
return 0;
+ err_kill:
+ if (network->dnsmasqPid > 0) {
+ kill(network->dnsmasqPid, SIGTERM);
+ network->dnsmasqPid = -1;
+ }
+
err_delbr2:
networkRemoveIptablesRules(driver, network);
@@ -798,16 +911,24 @@ static int networkStartNetworkDaemon(vir
}
-static int networkShutdownNetworkDaemon(virConnectPtr conn ATTRIBUTE_UNUSED,
- struct network_driver *driver,
- virNetworkObjPtr network) {
+static int networkShutdownNetworkDaemon(virConnectPtr conn,
+ struct network_driver *driver,
+ virNetworkObjPtr network) {
int err;
+ char *stateFile;
networkLog(NETWORK_INFO, _("Shutting down network '%s'\n"), network->def->name);
if (!virNetworkIsActive(network))
return 0;
+ stateFile = virNetworkConfigFile(conn, NETWORK_STATE_DIR, network->def->name);
+ if (!stateFile)
+ return -1;
+
+ unlink(stateFile);
+ VIR_FREE(stateFile);
+
if (network->dnsmasqPid > 0)
kill(network->dnsmasqPid, SIGTERM);
@@ -824,13 +945,10 @@ static int networkShutdownNetworkDaemon(
network->def->bridge, strerror(err));
}
+ /* See if its still alive and really really kill it */
if (network->dnsmasqPid > 0 &&
- waitpid(network->dnsmasqPid, NULL, WNOHANG) != network->dnsmasqPid) {
+ (kill(network->dnsmasqPid, 0) == 0))
kill(network->dnsmasqPid, SIGKILL);
- if (waitpid(network->dnsmasqPid, NULL, 0) != network->dnsmasqPid)
- networkLog(NETWORK_WARN,
- "%s", _("Got unexpected pid for dnsmasq\n"));
- }
network->dnsmasqPid = -1;
network->active = 0;
@@ -1048,8 +1166,7 @@ static virNetworkPtr networkDefine(virCo
if (virNetworkSaveConfig(conn,
driver->networkConfigDir,
- driver->networkAutostartDir,
- network) < 0) {
+ network->newDef ? network->newDef : network->def) < 0) {
virNetworkRemoveInactive(&driver->networks,
network);
network = NULL;
@@ -1086,7 +1203,10 @@ static int networkUndefine(virNetworkPtr
goto cleanup;
}
- if (virNetworkDeleteConfig(net->conn, network) < 0)
+ if (virNetworkDeleteConfig(net->conn,
+ driver->networkConfigDir,
+ driver->networkAutostartDir,
+ network) < 0)
goto cleanup;
virNetworkRemoveInactive(&driver->networks,
@@ -1140,7 +1260,7 @@ static int networkDestroy(virNetworkPtr
}
ret = networkShutdownNetworkDaemon(net->conn, driver, network);
- if (!network->configFile) {
+ if (!network->persistent) {
virNetworkRemoveInactive(&driver->networks,
network);
network = NULL;
@@ -1233,9 +1353,10 @@ cleanup:
}
static int networkSetAutostart(virNetworkPtr net,
- int autostart) {
+ int autostart) {
struct network_driver *driver = net->conn->networkPrivateData;
virNetworkObjPtr network;
+ char *configFile = NULL, *autostartLink = NULL;
int ret = -1;
networkDriverLock(driver);
@@ -1251,6 +1372,11 @@ static int networkSetAutostart(virNetwor
autostart = (autostart != 0);
if (network->autostart != autostart) {
+ if ((configFile = virNetworkConfigFile(net->conn, driver->networkConfigDir, network->def->name)) == NULL)
+ goto cleanup;
+ if ((autostartLink = virNetworkConfigFile(net->conn, driver->networkAutostartDir, network->def->name)) == NULL)
+ goto cleanup;
+
if (autostart) {
int err;
@@ -1261,17 +1387,17 @@ static int networkSetAutostart(virNetwor
goto cleanup;
}
- if (symlink(network->configFile, network->autostartLink) < 0) {
+ if (symlink(configFile, autostartLink) < 0) {
virReportSystemError(net->conn, errno,
_("Failed to create symlink '%s' to '%s'"),
- network->autostartLink, network->configFile);
+ autostartLink, configFile);
goto cleanup;
}
} else {
- if (unlink(network->autostartLink) < 0 && errno != ENOENT && errno != ENOTDIR) {
+ if (unlink(autostartLink) < 0 && errno != ENOENT && errno != ENOTDIR) {
virReportSystemError(net->conn, errno,
_("Failed to delete symlink '%s'"),
- network->autostartLink);
+ autostartLink);
goto cleanup;
}
}
@@ -1281,6 +1407,8 @@ static int networkSetAutostart(virNetwor
ret = 0;
cleanup:
+ VIR_FREE(configFile);
+ VIR_FREE(autostartLink);
if (network)
virNetworkObjUnlock(network);
return ret;
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list