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