Re: [PATCH 21/26] rpc: move state stop into virNetDaemon class

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(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(-)

> 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.