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