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

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.

+ /* 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 ? + */