[libvirt] [PATCH] network: Drop unused networkRestartRadvd

Michal Privoznik posted 1 patch 7 years ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/9a592e4d0bc5ea1ca2900db296ea2217304b25b7.1490774554.git.mprivozn@redhat.com
src/network/bridge_driver.c | 27 ---------------------------
1 file changed, 27 deletions(-)
[libvirt] [PATCH] network: Drop unused networkRestartRadvd
Posted by Michal Privoznik 7 years ago
Since its introduction in 1ce4922e720 this function lived as an
outcast in an #if 0 block.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/network/bridge_driver.c | 27 ---------------------------
 1 file changed, 27 deletions(-)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 5ccd37a..ddd2a3a 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -1980,33 +1980,6 @@ networkRefreshRadvd(virNetworkDriverStatePtr driver,
     return kill(network->radvdPid, SIGHUP);
 }
 
-#if 0
-/* currently unused, so it causes a build error unless we #if it out */
-static int
-networkRestartRadvd(virNetworkObjPtr network)
-{
-    char *radvdpidbase;
-
-    /* if there is a running radvd, kill it */
-    if (network->radvdPid > 0) {
-        /* essentially ignore errors from the following two functions,
-         * since there's really no better recovery to be done than to
-         * just push ahead (and that may be exactly what's needed).
-         */
-        if ((networkKillDaemon(network->radvdPid, "radvd",
-                               network->def->name) >= 0) &&
-            ((radvdpidbase = networkRadvdPidfileBasename(network->def->name))
-             != NULL)) {
-            virPidFileDelete(driver->pidDir, radvdpidbase);
-            VIR_FREE(radvdpidbase);
-        }
-        network->radvdPid = -1;
-    }
-    /* now start radvd if it should be started */
-    return networkStartRadvd(network);
-}
-#endif /* #if 0 */
-
 static int
 networkRefreshDaemonsHelper(virNetworkObjPtr net,
                             void *opaque)
-- 
2.10.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] network: Drop unused networkRestartRadvd
Posted by Laine Stump 7 years ago
On 03/29/2017 04:02 AM, Michal Privoznik wrote:
> Since its introduction in 1ce4922e720 this function lived as an
> outcast in an #if 0 block.

My memory is faint now, but I believe radvd as a whole is now nearly
unused, since dnsmasq gained the same functionality (route
advertisement) soon after we started using radvd, so we added code to
only run radvd if dnsmasq doesn't support RA.

At any rate, I'm guessing that this function was written in expectation
that it might be needed if we had to restart radvd due to a change in
the network's config while the network was still running (that's what we
use networkDhcpDaemonRestart() for), but it looks like it was forgotten,
and the calls were never added in the proper place (basically wherever
networkDhcpDaemonRestart() is called, but only if there is a change in
IPv6 addresses for the network).

(going back to refresh my memory - dnsmasq has supported RA since
version 2.67, which was released in 2013. RHEL6 does still use dnsmasq
2.48, but then any RHEL6 installation has been around for long enough
that if a lack of restarting radvd at appropriate times was a problem,
it would have been noticed a long time ago. RHEL7 also is only at
dnsmasq 2.66, but it appears that the --enable-ra patches have been
backported into the RHEL7 version of dnsmasq, so it won't use radvd)

So, although I think it's a mostly moot point, I still think the proper
solution to this is to start using the function rather than remove it.


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list