> On 09/13/2018 10:11 PM, wang.yechao255@xxxxxxxxxx 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
