[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On 12/23/2010 12:54 PM, Eric Blake wrote:
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?

Yes, I filed two. One about the race, and one about the inability to specify "no pidfile". The "no pidfile" one has already been closed as NOTABUG. The one about the race is still NEW.

https://bugzilla.redhat.com/show_bug.cgi?id=664791
https://bugzilla.redhat.com/show_bug.cgi?id=664783

(I filed them against RHEL6, because I'm not sure where upstream is, but am sure that the RHEL maintainer does).
+    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?

we need to unlink pidfile, but radvd seems to guarantee that pidfile-bin is unlinked when it exits.

I guess we should do it both when we kill radvd, and also when we undefine the network, since there may have been a situation that killed radvd without libvirt's knowledge, in which case an extra will be hanging around.

I'll squash that in as well.

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

Okay.

ACK with those nits addressed.



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]