From 2f3054c22a85b06299f4c472ba71cb8760c7a67c Mon Sep 17 00:00:00 2001
From: John Ferlan <jferlan@redhat.com>
Date: Mon, 6 Nov 2017 15:20:55 -0500
Subject: [PATCH] libvirtd: Fix order of cleanup processing

Current cleanup processing is ad-hoc at best - it's led to a couple of
strange and hard to diagnose timing problems and crashes.

So rather than perform cleanup in a somewhat random order, let's
perform cleanup in the exact opposite order of startup.

NB: It is possible that virNetlinkEventServerStart fails and we jump
to cleanup before driversInitialized has been set. That could leave
things inconsistent; however, resolution of that possibility is perhaps
more trouble than it's worth to handle.

Signed-off-by: John Ferlan <jferlan@redhat.com>
---
 daemon/libvirtd.c | 42 +++++++++++++++++++++++++-----------------
 1 file changed, 25 insertions(+), 17 deletions(-)

diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index 73f24915df..5103e8debe 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -1503,15 +1503,33 @@ int main(int argc, char **argv) {
                 0, "shutdown", NULL, NULL);
 
  cleanup:
+    /* Keep cleanup order in inverse order of startup */
+    virNetDaemonClose(dmn);
+
     virNetlinkEventServiceStopAll();
-    virObjectUnref(remoteProgram);
-    virObjectUnref(lxcProgram);
-    virObjectUnref(qemuProgram);
+
+    if (driversInitialized) {
+        /* NB: Possible issue with timing window between driversInitialized
+         * setting if virNetlinkEventServerStart fails */
+        driversInitialized = false;
+        virStateCleanup();
+    }
+
     virObjectUnref(adminProgram);
-    virNetDaemonClose(dmn);
-    virObjectUnref(srv);
     virObjectUnref(srvAdm);
+    virObjectUnref(qemuProgram);
+    virObjectUnref(lxcProgram);
+    virObjectUnref(remoteProgram);
+    virObjectUnref(srv);
+    virObjectUnref(dmn);
+
     virNetlinkShutdown();
+
+    if (pid_file_fd != -1)
+        virPidFileReleasePath(pid_file, pid_file_fd);
+
+    VIR_FREE(run_dir);
+
     if (statuswrite != -1) {
         if (ret != 0) {
             /* Tell parent of daemon what failed */
@@ -1522,25 +1540,15 @@ int main(int argc, char **argv) {
         }
         VIR_FORCE_CLOSE(statuswrite);
     }
-    if (pid_file_fd != -1)
-        virPidFileReleasePath(pid_file, pid_file_fd);
 
     VIR_FREE(sock_file);
     VIR_FREE(sock_file_ro);
     VIR_FREE(sock_file_adm);
+
     VIR_FREE(pid_file);
-    VIR_FREE(remote_config_file);
-    VIR_FREE(run_dir);
 
+    VIR_FREE(remote_config_file);
     daemonConfigFree(config);
 
-    if (driversInitialized) {
-        driversInitialized = false;
-        virStateCleanup();
-    }
-    /* Now that the hypervisor shutdown inhibition functions that use
-     * 'dmn' as a parameter are done, we can finally unref 'dmn' */
-    virObjectUnref(dmn);
-
     return ret;
 }
-- 
2.39.5