[libvirt] : Re: [PATCH v2] qemu: fix deadlock if createqemuProcessReconnect thread failed

[wang.yechao255 at zte.com.cn]

> On 09/13/2018 10:11 PM, wang.yechao255 at zte.com.cn wrote:
> > I just code review, found there may be problem.
> > 
> > The follow statement in founction qemuProcessReconnectHelper:
> > 
> > "if (virThreadCreate(&thread, false, qemuProcessReconnect, data) < 0) "
> > 
> > may be failed (no one can guarantee 'virThreadCreate' always success).

> Not sure how, but your response isn't threaded with your original
> posting. Makes it difficult to follow.

I'll fix later.

> Posting a few bugs related to qemuProcessReconnectHelper - makes me
> wonder "how" you get yourself into this situation. Is it like the
> libnuma bug and you're creating scripts that have frequent and continual
> libvirtd restarts, which I noted there isn't not really a normal situation.

libnuma bug is reproduced by scripts. But this bug is not, I did not reproduce it.
Only read the code, found may be enter this situation.

> In any case, I think in this situation what we want is to call
> virDomainObjListRemoveLocked instead of virDomainObjListRemove from
> qemuDomainRemoveInactive because all the prerequisites for calling the
> Locked function are true (the obj is locked/reffed and the list lock is
> held).

> Now this is *not* to say qemuDomainRemoveInactive should always call the
> *Locked variant. It means adding a "bool useLocked" to the API and then
> passing true *only* from qemuProcessReconnectHelper and then of course
> calling the virDomainObjListRemoveLocked variant when the bool is true.
> Requires changing all the other callers to pass false.

> Another preferred option:

> Patch 1. Split up qemuDomainRemoveInactive, such that code from "cfg =
> virQEMUDriverGetConfig(driver);" through/including
> "qemuExtDevicesCleanupHost(driver, vm->def);" plus the
> "virObjectUnref(cfg);" is in a static helper
> "qemuDomainRemoveInactiveCommon".

> Patch 2. Create a qemuDomainRemoveInactiveLocked which is a copy of
> qemuDomainRemoveInactive except that instead of calling
> virDomainObjListRemove it calls virDomainObjListRemoveLocked.

> Patch 3. Create a qemuDomainRemoveInactiveJobLocked which copies
> qemuDomainRemoveInactiveJob except of course calling
> virDomainObjListRemoveLocked.

> Patch 4. Modify qemuProcessReconnectHelper to call
> qemuDomainRemoveInactiveJobLocked.

> John

Yes, your suggested pathes are preferred. Thanks, I'll give v3 patch.

---
Best wishes
Wang Yechao