On Fri, Jan 22, 2021 at 12:45 PM Michal Privoznik <[email protected]> wrote:
> If libvirtd is sent SIGTERM while it is still initializing, it > may crash. The following scenario was observed (using 'stress' to > slow down CPU so much that the window where the problem exists is > bigger): > > 1) The main thread is already executing virNetDaemonRun() and is > in virEventRunDefaultImpl(). > 2) The thread that's supposed to run daemonRunStateInit() is > spawned already, but daemonRunStateInit() is in its very early > stage (in the stack trace I see it's executing > virIdentityGetSystem()). > > If SIGTERM (or any other signal that we don't override handler > for) arrives at this point, the main thread jumps out from > virEventRunDefaultImpl() and enters virStateShutdownPrepare() > (via shutdownPrepareCb which was set earlier). This iterates > through stateShutdownPrepare() callbacks of state drivers and > reaching qemuStateShutdownPrepare() eventually only to > dereference qemu_driver. But since thread 2) has not been > scheduled/not proceeded yet, qemu_driver was not allocated yet. > > Solution is simple - just check if qemu_driver is not NULL. But > doing so only in qemuStateShutdownPrepare() would push the > problem down to virStateShutdownWait(), well > qemuStateShutdownWait(). Therefore, duplicate the trick there > too. > I guess this is a partial solution. Initialization may be in a state when qemu_driver is initialized but qemu_driver->workerPool is still NULL for example. Maybe we'd better delay shutdown until initialization is finished? Nikolay > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1895359#c14 > Signed-off-by > <https://bugzilla.redhat.com/show_bug.cgi?id=1895359#c14Signed-off-by>: > Michal Privoznik <[email protected]> > --- > src/qemu/qemu_driver.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index 027617deef..ca4f366323 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -1072,6 +1072,9 @@ qemuStateStop(void) > static int > qemuStateShutdownPrepare(void) > { > + if (!qemu_driver) > + return 0; > + > virThreadPoolStop(qemu_driver->workerPool); > return 0; > } > @@ -1091,6 +1094,9 @@ qemuDomainObjStopWorkerIter(virDomainObjPtr vm, > static int > qemuStateShutdownWait(void) > { > + if (!qemu_driver) > + return 0; > + > virDomainObjListForEach(qemu_driver->domains, false, > qemuDomainObjStopWorkerIter, NULL); > virThreadPoolDrain(qemu_driver->workerPool); > -- > 2.26.2 > >
