On Wed, Jan 14, 2009 at 12:44:50PM +0000, Daniel P. Berrange wrote:
> 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.
Sounds a real user improvement, patch looks fine but I'm concerned
about this:
> + /* 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 ?
> + */
OS specific but check in configure about the location of dnsmasq
export it as DNSMASQ_PATH, then compare to file pointed by /proc/pid/exe
#ifdef __linux__
char *pidpath;
virAsprintf(&pidpath, "/proc/%d/exe", obj->dnsmasqPid);
if (virFileLinkPointsTo(pidpath, DNSMASQ_PATH) == 0))
obj->dnsmasqPid = -1;
VIR_FREE(pidpath);
#endif
I'm sure one can find a Solaris equivalent.
Patch looks fine to me,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list