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

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.

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, SIOCGIFMTU, &ifr)) + return -1;

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, @@ -58,6 +60,7 @@ int brDeleteInterface (brContr const char *bridge, const char *iface); +

@@ -695,48 +671,64 @@ 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)))

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_LIB_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_LIB_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);

@@ -377,15 +443,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);

@@ -442,13 +502,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_LIB_DIR)) < 0) { + virReportSystemError(conn, err, + _("cannot create directory %s"), + NETWORK_LIB_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 guarenteed the proess has started

+ * and writtena pid

@@ -798,16 +892,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 *configFile; networkLog(NETWORK_INFO, _("Shutting down network '%s'\n"), network->def->name); if (!virNetworkIsActive(network)) return 0; + configFile = virNetworkConfigFile(conn, NETWORK_LIB_DIR, network->def->name); + if (!configFile) + return -1; + + unlink(configFile); + VIR_FREE(configFile);

if (network->dnsmasqPid > 0) kill(network->dnsmasqPid, SIGTERM); @@ -824,13 +926,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")); - }