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@redhat.com>
---
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;
}
diff --git a/src/rpc/virnetdaemon.h b/src/rpc/virnetdaemon.h
index 31a355adb4..c64f3bdeb1 100644
--- 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 virNetDaemonPreserve(virNetDaemon *dmn);
bool virNetDaemonHasClients(virNetDaemon *dmn);
@@ -84,6 +82,22 @@ bool virNetDaemonHasServer(virNetDaemon *dmn,
typedef int (*virNetDaemonShutdownCallback)(void);
+/*
+ * @preserveCb: preserves any active state
+ * @prepareCb: start shutting down daemon
+ * @waitCb: wait for shutdown completion
+ *
+ * The sequence of operations during shutdown is as follows
+ *
+ * - Listener stops accepting new clients
+ * - Existing clients are closed
+ * - Delay until @preserveCb is complete (if running)
+ * - @prepareCb invoked
+ * - Server worker pool is drained in background
+ * - @waitCb is invoked in background
+ * - Main loop terminates
+ */
void virNetDaemonSetShutdownCallbacks(virNetDaemon *dmn,
+ virNetDaemonShutdownCallback preserveCb,
virNetDaemonShutdownCallback prepareCb,
virNetDaemonShutdownCallback waitCb);
--
2.47.1On Wed, Jan 08, 2025 at 19:42:54 +0000, Daniel P. Berrangé wrote:
> 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@redhat.com>
> ---
> 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/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;
So originally this was passed the 'dmn' pointer ...
> -
> - VIR_DEBUG("Begin stop dmn=%p", dmn);
> -
> - ignore_value(virStateStop());
> -
> - VIR_DEBUG("Completed stop dmn=%p", dmn);
> -
... but it was never touched except for logging ...
> - /* Exit daemon cleanly */
> - virNetDaemonQuit(dmn);
... and then using a self-locking API.
> -}
> -
> -
> -/* We do this in a thread to not block the main loop */
> -static void daemonStop(virNetDaemon *dmn)
> -{
> - virThread *thr;
> - virObjectRef(dmn);
All of the above while having a reference.
> -
> - thr = g_new0(virThread, 1);
> -
> - if (virThreadCreateFull(thr, true,
> - daemonStopWorker,
> - "daemon-stop", false, dmn) < 0) {
So this could make it async without worries ...
> - virObjectUnref(dmn);
> - g_free(thr);
> - return;
> - }
> -
> - virNetDaemonSetStateStopWorkerThread(dmn, &thr);
... and without touching 'dmn' while unlocked.
> -}
> -
> -
> 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)
> }
[Re-organized hunks for explanatory reasons]
>
> +
> +
> +void virNetDaemonPreserve(virNetDaemon *dmn)
> +{
> + VIR_LOCK_GUARD lock = virObjectLockGuard(dmn);
But in the new code, we lock 'dmn' ...
> +
> + if (!dmn->shutdownPreserveCb ||
> + dmn->shutdownPreserveThread)
... so that we can safely access the properties of virNetDaemon ..
> + return;
> +
> + virObjectRef(dmn);
> + dmn->shutdownPreserveThread = g_new0(virThread, 1);
> +
> + if (virThreadCreateFull(dmn->shutdownPreserveThread, true,
> + virNetDaemonPreserveWorker,
> + "daemon-stop", false, dmn) < 0) {
.... but once you make it async here ...
> + virObjectUnref(dmn);
> + g_clear_pointer(&dmn->shutdownPreserveThread, g_free);
> + return;
> + }
... and the context of this function finishes right away, 'dmn' will be
unlocked ...
> +}
> +
> +
>
> +static void virNetDaemonPreserveWorker(void *opaque)
> +{
> + virNetDaemon *dmn = opaque;
> +
> + VIR_DEBUG("Begin stop dmn=%p", dmn);
> +
> + dmn->shutdownPreserveCb();
... but here we access it's props.
> +
> + VIR_DEBUG("Completed stop dmn=%p", dmn);
> +
> + virNetDaemonQuit(dmn);
... now this uses self locking APIs once more.
> + virObjectUnref(dmn);
> +}
> 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;
And this setter technically makes it non-immutable.
While it most likely will not be a problem I don't think we should
access this function pointer outside of a lock. That is unless you
document it as being/make it immutable.
Upcoming patches also add code where virNetDaemonPreserveWorker will
access dmn->quit which definitely isn't immutable so it's likely a more
complete solution will be needed.
On Mon, Feb 03, 2025 at 10:52:23 +0100, Peter Krempa wrote:
> On Wed, Jan 08, 2025 at 19:42:54 +0000, Daniel P. Berrangé wrote:
> > 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@redhat.com>
> > ---
> > 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(-)
[...]
> > 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;
>
>
> And this setter technically makes it non-immutable.
>
> While it most likely will not be a problem I don't think we should
> access this function pointer outside of a lock. That is unless you
> document it as being/make it immutable.
>
> Upcoming patches also add code where virNetDaemonPreserveWorker will
> access dmn->quit which definitely isn't immutable so it's likely a more
> complete solution will be needed.
Disregard this paragraph, I didn't notice the lock guard in the patch
adding the access to dmn->quit.
Thus declaring 'shutdownPreserveCb' to be immutable should be good
enough.
On Wed, Jan 08, 2025 at 19:42:54 +0000, Daniel P. Berrangé wrote:
> 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@redhat.com>
> ---
> 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,
The name 'virStateStop' is starting to be confusing in the context it's
being used now. I suggest you at least document what the expectations
from the hypervisor drivers are? Or better when it's invoked.
> 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;
> + }
> +}
Please use the new function header formatting style.
> +
> +
> 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;
> }
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
© 2016 - 2026 Red Hat, Inc.