Re: [PATCH 21/26] 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. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/libvirt_remote.syms | 2 +- src/remote/remote_daemon.c | 40 ++------------------------ src/rpc/virnetdaemon.c | 58 ++++++++++++++++++++++++++++---------- src/rpc/virnetdaemon.h | 20 +++++++++++-- 4 files changed, 64 insertions(+), 56 deletions(-) diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms index f0f90815cf..7e87b0bd2a 100644 --- a/src/libvirt_remote.syms +++ b/src/libvirt_remote.syms @@ -90,12 +90,12 @@ virNetDaemonIsPrivileged; virNetDaemonNew; virNetDaemonNewPostExecRestart; virNetDaemonPreExecRestart; +virNetDaemonPreserve; virNetDaemonQuit; virNetDaemonQuitExecRestart; virNetDaemonRemoveShutdownInhibition; virNetDaemonRun; virNetDaemonSetShutdownCallbacks; -virNetDaemonSetStateStopWorkerThread; virNetDaemonUpdateServices; diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c index c4b930cb70..bf91ee5772 100644 --- 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); + virNetDaemonPreserve(dmn); return message; } @@ -581,7 +546,7 @@ handleSystemMessageFunc(GDBusConnection *connection G_GNUC_UNUSED, VIR_DEBUG("dmn=%p", dmn); - daemonStop(dmn); + virNetDaemonPreserve(dmn); } @@ -625,6 +590,7 @@ static void daemonRunStateInit(void *opaque) g_atomic_int_set(&driversInitialized, 1); virNetDaemonSetShutdownCallbacks(dmn, + virStateStop,

virStateShutdownPrepare, virStateShutdownWait); diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c index bb7d2ff9a0..19b19ff834 100644 --- a/src/rpc/virnetdaemon.c +++ b/src/rpc/virnetdaemon.c @@ -65,9 +65,10 @@ struct _virNetDaemon { GHashTable *servers; virJSONValue *srvObject; + virNetDaemonShutdownCallback shutdownPreserveCb; virNetDaemonShutdownCallback shutdownPrepareCb; virNetDaemonShutdownCallback shutdownWaitCb; - virThread *stateStopThread; + virThread *shutdownPreserveThread; int finishTimer; bool quit; bool finished; @@ -107,7 +108,7 @@ virNetDaemonDispose(void *obj) virEventRemoveHandle(dmn->sigwatch); #endif /* !WIN32 */ - g_free(dmn->stateStopThread); + g_free(dmn->shutdownPreserveThread); g_clear_pointer(&dmn->servers, g_hash_table_unref); @@ -705,8 +706,8 @@ daemonShutdownWait(void *opaque) virHashForEach(dmn->servers, daemonServerShutdownWait, NULL); if (!dmn->shutdownWaitCb || dmn->shutdownWaitCb() >= 0) { - if (dmn->stateStopThread) - virThreadJoin(dmn->stateStopThread); + if (dmn->shutdownPreserveThread) + virThreadJoin(dmn->shutdownPreserveThread); graceful = true; } @@ -801,17 +802,6 @@ virNetDaemonRun(virNetDaemon *dmn) } -void -virNetDaemonSetStateStopWorkerThread(virNetDaemon *dmn, - virThread **thr) -{ - 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); -} - - void virNetDaemonQuit(virNetDaemon *dmn) { @@ -833,6 +823,42 @@ virNetDaemonQuitExecRestart(virNetDaemon *dmn) } +static void virNetDaemonPreserveWorker(void *opaque) +{ + virNetDaemon *dmn = opaque; + + VIR_DEBUG("Begin stop dmn=%p", dmn); + + dmn->shutdownPreserveCb(); + + VIR_DEBUG("Completed stop dmn=%p", dmn); + + virNetDaemonQuit(dmn); + virObjectUnref(dmn); +} + + +void virNetDaemonPreserve(virNetDaemon *dmn) +{ + VIR_LOCK_GUARD lock = virObjectLockGuard(dmn); + + if (!dmn->shutdownPreserveCb || + dmn->shutdownPreserveThread) + return; + + virObjectRef(dmn); + dmn->shutdownPreserveThread = g_new0(virThread, 1); + + if (virThreadCreateFull(dmn->shutdownPreserveThread, true, + virNetDaemonPreserveWorker, + "daemon-stop", false, dmn) < 0) { + virObjectUnref(dmn); + g_clear_pointer(&dmn->shutdownPreserveThread, g_free); + return; + } +}

+ + static int daemonServerClose(void *payload, const char *key G_GNUC_UNUSED, @@ -870,11 +896,13 @@ virNetDaemonHasClients(virNetDaemon *dmn) void virNetDaemonSetShutdownCallbacks(virNetDaemon *dmn, + virNetDaemonShutdownCallback preserveCb, virNetDaemonShutdownCallback prepareCb, virNetDaemonShutdownCallback waitCb) { VIR_LOCK_GUARD lock = virObjectLockGuard(dmn); + dmn->shutdownPreserveCb = preserveCb; dmn->shutdownPrepareCb = prepareCb; dmn->shutdownWaitCb = waitCb; }