Re: [libvirt] PATCH: Allow virtual networks to survive daemon restarts

On Wed, Jan 14, 2009 at 08:55:46AM +0000, Mark McLoughlin wrote: 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 :|