rpc: move state stop into virNetDaemon class

Currently the remote daemon code is responsible for calling virStateStop
in a background thread. The virNetDaemon code wants to synchronize with
this during shutdown, however, so the virThreadPtr must be passed over.

Even the limited synchronization done currently, however, is flawed and
to fix this requires the virNetDaemon code to be responsible for calling
virStateStop in a thread more directly.

Thus the logic is moved over into virStateStop via a further callback
to be registered by the remote daemon.

Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms
index c4053fdcb246ee79db652b0ac762f5dfb07cde85..9dcc2792c108c5018ad7a0988fc35274442b6a10 100644 (file)
--- a/src/libvirt_remote.syms
+++ b/src/libvirt_remote.syms
@@ -95,7 +95,7 @@ virNetDaemonQuitExecRestart;
 virNetDaemonRemoveShutdownInhibition;
 virNetDaemonRun;
 virNetDaemonSetLifecycleCallbacks;
-virNetDaemonSetStateStopWorkerThread;
+virNetDaemonStop;
 virNetDaemonUpdateServices;
 
 

diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c
index 9c0025a3bcc6f3bb69f1a950cab4e3f5db28955e..16dc1d86f7c44f29a15ce04b89ce7efb284862f0 100644 (file)
--- a/src/remote/remote_daemon.c
+++ b/src/remote/remote_daemon.c
@@ -514,41 +514,6 @@ static void daemonInhibitCallback(bool inhibit, void *opaque)
 static GDBusConnection *sessionBus;
 static GDBusConnection *systemBus;
 
-static void daemonStopWorker(void *opaque)
-{
-    virNetDaemon *dmn = opaque;
-
-    VIR_DEBUG("Begin stop dmn=%p", dmn);
-
-    ignore_value(virStateStop());
-
-    VIR_DEBUG("Completed stop dmn=%p", dmn);
-
-    /* Exit daemon cleanly */
-    virNetDaemonQuit(dmn);
-}
-
-
-/* We do this in a thread to not block the main loop */
-static void daemonStop(virNetDaemon *dmn)
-{
-    virThread *thr;
-    virObjectRef(dmn);
-
-    thr = g_new0(virThread, 1);
-
-    if (virThreadCreateFull(thr, true,
-                            daemonStopWorker,
-                            "daemon-stop", false, dmn) < 0) {
-        virObjectUnref(dmn);
-        g_free(thr);
-        return;
-    }
-
-    virNetDaemonSetStateStopWorkerThread(dmn, &thr);
-}
-
-
 static GDBusMessage *
 handleSessionMessageFunc(GDBusConnection *connection G_GNUC_UNUSED,
                          GDBusMessage *message,
@@ -562,7 +527,7 @@ handleSessionMessageFunc(GDBusConnection *connection G_GNUC_UNUSED,
     if (virGDBusMessageIsSignal(message,
                                 "org.freedesktop.DBus.Local",
                                 "Disconnected"))
-        daemonStop(dmn);
+        virNetDaemonStop(dmn);
 
     return message;
 }
@@ -581,7 +546,7 @@ handleSystemMessageFunc(GDBusConnection *connection G_GNUC_UNUSED,
 
     VIR_DEBUG("dmn=%p", dmn);
 
-    daemonStop(dmn);
+    virNetDaemonStop(dmn);
 }
 
 
@@ -625,6 +590,7 @@ static void daemonRunStateInit(void *opaque)
     g_atomic_int_set(&driversInitialized, 1);
 
     virNetDaemonSetLifecycleCallbacks(dmn,
+                                      virStateStop,
                                       virStateShutdownPrepare,
                                       virStateShutdownWait);
 

diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c
index 6d20edf28bb991023a8945c882567375cbfb5425..43a75b330f149dd935d18125bb6ff63d3c803834 100644 (file)
--- a/src/rpc/virnetdaemon.c
+++ b/src/rpc/virnetdaemon.c
@@ -65,6 +65,7 @@ struct _virNetDaemon {
     GHashTable *servers;
     virJSONValue *srvObject;
 
+    virNetDaemonLifecycleCallback stopCb;
     virNetDaemonLifecycleCallback shutdownPrepareCb;
     virNetDaemonLifecycleCallback shutdownWaitCb;
     virThread *stateStopThread;
@@ -793,8 +794,10 @@ virNetDaemonRun(virNetDaemon *dmn)
         }
     }
 
+    VIR_DEBUG("Main loop exited");
     if (dmn->graceful) {
         virThreadJoin(&shutdownThread);
+        VIR_DEBUG("Graceful shutdown complete");
     } else {
         VIR_WARN("Make forcefull daemon shutdown");
         exit(EXIT_FAILURE);
@@ -806,34 +809,61 @@ virNetDaemonRun(virNetDaemon *dmn)
 
 
 void
-virNetDaemonSetStateStopWorkerThread(virNetDaemon *dmn,
-                                     virThread **thr)
+virNetDaemonQuit(virNetDaemon *dmn)
 {
     VIR_LOCK_GUARD lock = virObjectLockGuard(dmn);
 
-    VIR_DEBUG("Setting state stop worker thread on dmn=%p to thr=%p", dmn, thr);
-    dmn->stateStopThread = g_steal_pointer(thr);
+    VIR_DEBUG("Quit requested %p", dmn);
+    dmn->quit = true;
 }
 
 
 void
-virNetDaemonQuit(virNetDaemon *dmn)
+virNetDaemonQuitExecRestart(virNetDaemon *dmn)
 {
     VIR_LOCK_GUARD lock = virObjectLockGuard(dmn);
 
-    VIR_DEBUG("Quit requested %p", dmn);
+    VIR_DEBUG("Exec-restart requested %p", dmn);
     dmn->quit = true;
+    dmn->execRestart = true;
+}
+
+
+static void
+virNetDaemonStopWorker(void *opaque)
+{
+    virNetDaemon *dmn = opaque;
+
+    VIR_DEBUG("Begin stop dmn=%p", dmn);
+
+    dmn->stopCb();
+
+    VIR_DEBUG("Completed stop dmn=%p", dmn);
+
+    virNetDaemonQuit(dmn);
+    virObjectUnref(dmn);
 }
 
 
 void
-virNetDaemonQuitExecRestart(virNetDaemon *dmn)
+virNetDaemonStop(virNetDaemon *dmn)
 {
     VIR_LOCK_GUARD lock = virObjectLockGuard(dmn);
 
-    VIR_DEBUG("Exec-restart requested %p", dmn);
-    dmn->quit = true;
-    dmn->execRestart = true;
+    if (!dmn->stopCb ||
+        dmn->stateStopThread)
+        return;
+
+    virObjectRef(dmn);
+    dmn->stateStopThread = g_new0(virThread, 1);
+
+    if (virThreadCreateFull(dmn->stateStopThread, true,
+                            virNetDaemonStopWorker,
+                            "daemon-stop", false, dmn) < 0) {
+        virObjectUnref(dmn);
+        g_clear_pointer(&dmn->stateStopThread, g_free);
+        return;
+    }
 }
 
 
@@ -874,11 +904,20 @@ virNetDaemonHasClients(virNetDaemon *dmn)
 
 void
 virNetDaemonSetLifecycleCallbacks(virNetDaemon *dmn,
+                                  virNetDaemonLifecycleCallback stopCb,
                                   virNetDaemonLifecycleCallback prepareCb,
                                   virNetDaemonLifecycleCallback waitCb)
 {
     VIR_LOCK_GUARD lock = virObjectLockGuard(dmn);
 
+    VIR_DEBUG("Lifecycle callbacks stop=%p prepare=%p wait=%p",
+              stopCb, prepareCb, waitCb);
+
+    /* Immutable once set */
+    if (dmn->stopCb || dmn->shutdownPrepareCb || dmn->shutdownWaitCb)
+        return;
+
+    dmn->stopCb = stopCb;
     dmn->shutdownPrepareCb = prepareCb;
     dmn->shutdownWaitCb = waitCb;
 }

diff --git a/src/rpc/virnetdaemon.h b/src/rpc/virnetdaemon.h
index 0f4ebb6df701d891078fdd5e51dde3a959972a5d..68c95ade4a4a0da9186bff07c21c887be7bd9114 100644 (file)
--- a/src/rpc/virnetdaemon.h
+++ b/src/rpc/virnetdaemon.h
@@ -66,13 +66,11 @@ int virNetDaemonAddSignalHandler(virNetDaemon *dmn,
 void virNetDaemonUpdateServices(virNetDaemon *dmn,
                                 bool enabled);
 
-void virNetDaemonSetStateStopWorkerThread(virNetDaemon *dmn,
-                                          virThread **thr);
-
 void virNetDaemonRun(virNetDaemon *dmn);
 
 void virNetDaemonQuit(virNetDaemon *dmn);
 void virNetDaemonQuitExecRestart(virNetDaemon *dmn);
+void virNetDaemonStop(virNetDaemon *dmn);
 
 bool virNetDaemonHasClients(virNetDaemon *dmn);
 
@@ -84,6 +82,29 @@ bool virNetDaemonHasServer(virNetDaemon *dmn,
 
 typedef int (*virNetDaemonLifecycleCallback)(void);
 
+/*
+ * @stopCb: preserves any active state on host shutdown / session exit
+ * @prepareCb: start shutting down daemon
+ * @waitCb: wait for shutdown completion
+ *
+ * This method may only be invoked once, the callbacks are immutable
+ * once set.
+ *
+ * On host shutdown (privileged) or session exit (unprivileged)
+ * the @stopCb will be invoked first.
+ *
+ * When the daemon shuts down, the sequence of operations is
+ * as follows
+ *
+ * - Listener stops accepting new clients
+ * - Existing clients are closed
+ * - Delay until @stopCb is complete (if still running)
+ * - @prepareCb invoked
+ * - Server worker pool is drained in background
+ * - @waitCb is invoked in background
+ * - Main loop terminates
+ */
 void virNetDaemonSetLifecycleCallbacks(virNetDaemon *dmn,
+                                       virNetDaemonLifecycleCallback stopCb,
                                        virNetDaemonLifecycleCallback prepareCb,
                                        virNetDaemonLifecycleCallback waitCb);