Re: [libvirt] [PATCHv2 13/13] Run radvd for virtual networks with IPv6 addresses

On 12/22/2010 11:58 AM, Laine Stump wrote: > Running an instance of the router advertisement daemon (radvd) allows > guests using the virtual network to automatically acquire an IPv6 > address and default route. Note that acquiring an address only works > for networks with a prefix length of exactly 64 - radvd is still run > in other circumstances, and still advertises routes, but autoconf will > not work because it requires exactly 64 bits of address info from the > network prefix. > > This patch avoids a race condition with the pidfile by manually > daemonizing radvd rather than allowing it to daemonize itself, then > creating our own pidfile (in addition to radvd's own file, which is > unnecessary). This is accomplished by exec'ing it with "--debug 1" in > the commandline, and using virCommand's features to fork, create a > pidfile, and detach from the newly forked process. As a reminder, have you filed a bug report against radvd about their anti-social daemonizing behavior?

> + cmd = virCommandNewArgList(RADVD, "--debug", "1", > + "--config", configfile, > + "--pidfile", NULL); > + virCommandAddArgFormat(cmd, "%s-bin", pidfile); > + > + virCommandSetPidFile(cmd, pidfile); > + virCommandDaemonize(cmd); > + > + if (virCommandRun(cmd, NULL)< 0) > + goto cleanup; > + > + if (virFileReadPid(NETWORK_PID_DIR, radvdpidbase, > +&network->radvdPid)< 0) > + goto cleanup; > + > + ret = 0; > +cleanup: > + virCommandFree(cmd); > + VIR_FREE(configfile); > + VIR_FREE(configstr); > + virBufferFreeAndReset(&configbuf); > + VIR_FREE(radvdpidbase); > + VIR_FREE(pidfile); Should we also unlink() pidfile or pidfile-bin at any point?

> static int > -networkEnableIpForwarding(void) > +networkEnableIpForwarding(int enableIPv4, int enableIPv6) s/int/bool/

ACK with those nits addressed.