<meta http-equiv="Content-Type" content="text/html; charset=utf-8"><div dir="ltr"><div dir="ltr"><div class="gmail_default" style="font-size:large"><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Jan 22, 2021 at 2:24 PM Michal Privoznik <<a href="mailto:mprivozn@redhat.com">mprivozn@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 1/22/21 12:09 PM, Nikolay Shirokovskiy wrote:<br>
> On Fri, Jan 22, 2021 at 12:45 PM Michal Privoznik <<a href="mailto:mprivozn@redhat.com" target="_blank">mprivozn@redhat.com</a>><br>
> wrote:<br>
> <br>
>> If libvirtd is sent SIGTERM while it is still initializing, it<br>
>> may crash. The following scenario was observed (using 'stress' to<br>
>> slow down CPU so much that the window where the problem exists is<br>
>> bigger):<br>
>><br>
>> 1) The main thread is already executing virNetDaemonRun() and is<br>
>>     in virEventRunDefaultImpl().<br>
>> 2) The thread that's supposed to run daemonRunStateInit() is<br>
>>     spawned already, but daemonRunStateInit() is in its very early<br>
>>     stage (in the stack trace I see it's executing<br>
>>     virIdentityGetSystem()).<br>
>><br>
>> If SIGTERM (or any other signal that we don't override handler<br>
>> for) arrives at this point, the main thread jumps out from<br>
>> virEventRunDefaultImpl() and enters virStateShutdownPrepare()<br>
>> (via shutdownPrepareCb which was set earlier). This iterates<br>
>> through stateShutdownPrepare() callbacks of state drivers and<br>
>> reaching qemuStateShutdownPrepare() eventually only to<br>
>> dereference qemu_driver. But since thread 2) has not been<br>
>> scheduled/not proceeded yet, qemu_driver was not allocated yet.<br>
>><br>
>> Solution is simple - just check if qemu_driver is not NULL. But<br>
>> doing so only in qemuStateShutdownPrepare() would push the<br>
>> problem down to virStateShutdownWait(), well<br>
>> qemuStateShutdownWait(). Therefore, duplicate the trick there<br>
>> too.<br>
>><br>
> <br>
> I guess this is a partial solution. Initialization may be in a state when<br>
> qemu_driver is initialized but qemu_driver->workerPool is still NULL<br>
> for example.<br>
<br>
Yes.<br>
<br>
> <br>
> Maybe we'd better delay shutdown until initialization is finished?<br>
<br>
I'm not exactly sure how to achieve that. Do you have a hint? Also, part <br>
of qemu driver state init is autostarting domains (which may take ages).<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:large">I'm thinking of adding a new variable 'initialized' to virnetdaemon. It can be </div><div class="gmail_default" style="font-size:large">set by call from daemonRunStateInit after initialization is finished.</div><div class="gmail_default" style="font-size:large">And we should call shutdownPrepare/shutdownWait only when both 'quit' and</div><div class="gmail_default" style="font-size:large">'initialized' are true. It will require another pipe pair probably to wake up</div><div class="gmail_default" style="font-size:large">event loop thread when 'initialized' it set to true.</div><div class="gmail_default" style="font-size:large"><br></div><div class="gmail_default" style="font-size:large">As initialization can take a lot of time (autostart as you mentioned) we</div><div class="gmail_default" style="font-size:large">can arm finishTimer right when quit is set without waiting for initialization</div><div class="gmail_default" style="font-size:large">to finish. This way we exit ungracefully just as in other cases when shutdown<br></div><div class="gmail_default" style="font-size:large">finishing takes too much time. Libvirtd has a goal to handle crashes at</div><div class="gmail_default" style="font-size:large">any time so this exit should be fine.</div><div class="gmail_default" style="font-size:large"><br></div><div class="gmail_default" style="font-size:large">Nikolay</div><br></div><div> </div></div></div>