On Thu, Dec 09, 2021 at 15:43:55 +0100, Michal Privoznik wrote:
The initialization of drivers happens in a separate thread. However, the main thread continues initialization and sets shutdown callbacks (virStateShutdownPrepare() and virStateShutdownWait()) even though the driver init thread is still running. This is dangerous because if the daemon decides to quit early (e.g. because SIGINT was delivered) the shutdownPrepare and shutdownWait callback are called over partially init drivers.
Set callbacks only after all drivers were initialized.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/218 Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2027400
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/remote/remote_daemon.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c index de43a54c2e..4e10f3ad23 100644 --- a/src/remote/remote_daemon.c +++ b/src/remote/remote_daemon.c @@ -626,6 +626,10 @@ static void daemonRunStateInit(void *opaque)
driversInitialized = true;
+ virNetDaemonSetShutdownCallbacks(dmn, + virStateShutdownPrepare, + virStateShutdownWait); +
Okay so this placement ensures that the state shutdown code will only ever be executed if the state was already initialized ...
/* Tie the non-privileged daemons to the session/shutdown lifecycle */ if (!virNetDaemonIsPrivileged(dmn)) {
@@ -1214,9 +1218,6 @@ int main(int argc, char **argv) { #endif
/* Run event loop. */ - virNetDaemonSetShutdownCallbacks(dmn, - virStateShutdownPrepare, - virStateShutdownWait);
... which wasn't always true here.
virNetDaemonRun(dmn);
ret = 0;
So at this point it's still possible that the daemon will quit without the callbacks being called, but tat was possible even before. Based on the above and the fact that I wasn't able to reproduce the crash: Reviewed-by: Peter Krempa <pkrempa@redhat.com>